Skip to content

Commit 1a3a954

Browse files
committed
test: use the real 22.7.0 AsyncContextFrame cutoff
Review nits from #397. AsyncContextFrame landed in 22.7.0, not at the 22 boundary, so several places named the wrong version. Drop `&& satisfies(process.versions.node, '>=22.7.0')` from the four useCPED definitions. It is redundant against isAsyncContextFrameActive(): ACF cannot be active below 22.7.0, so the detection already answers false there. This leaves semver unused in test-get-value-from-map-profiler.ts, so the import goes too. Fix the cutoffs that were expressed as a bare major: the skip gates in test-async-context-frame.ts now use a semver check, and the prose in test-otel-thread-ctx.ts and in the async-context-frame doc comment names 22.7.0. asyncContextFrameHint() had the only user-visible instance of the bug: on Node 22.0 through 22.6 it advised passing --experimental-async-context-frame, a flag those versions do not have. Compared by major/minor rather than semver.satisfies because semver is a devDependency and this module ships. Boundary checked across 20.19.0, 22.6.0, 22.7.0, 22.23.2, 23.5.0 and 24.18.0. 124 passing on macOS, 175 passing / 2 pending in test:docker, unchanged.
1 parent bbbe8a4 commit 1a3a954

7 files changed

Lines changed: 22 additions & 23 deletions

ts/src/async-context-frame.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ let active: boolean | undefined;
4444
* `process.execArgv`, because the two disagree in both directions and each
4545
* combination is reachable today:
4646
*
47-
* - `NODE_OPTIONS=--experimental-async-context-frame` is accepted on Node 22
48-
* and 23 and turns ACF on without appearing in `execArgv`. Inferring "off"
49-
* there makes callers refuse to run in a process that would have worked.
47+
* - `NODE_OPTIONS=--experimental-async-context-frame` is accepted from Node
48+
* 22.7.0 through 23 and turns ACF on without appearing in `execArgv`.
49+
* Inferring "off" there makes callers refuse to run in a process that would
50+
* have worked.
5051
* - `NODE_OPTIONS=--no-async-context-frame` is accepted on Node 24 and turns
5152
* ACF off without appearing in `execArgv`. Inferring "on" there is the worse
5253
* error: the CPED slot is never written, so a writer that starts anyway keeps
@@ -97,8 +98,10 @@ export function isAsyncContextFrameActive(): boolean {
9798
*/
9899
export function asyncContextFrameHint(): string {
99100
const version = process.versions.node;
100-
const major = Number(version.split('.')[0]);
101-
if (major < 22) {
101+
const [major, minor] = version.split('.').map(Number);
102+
// Hand-rolled rather than semver.satisfies: semver is a devDependency, and
103+
// this module ships.
104+
if (major < 22 || (major === 22 && minor < 7)) {
102105
return `Node ${version} does not support it at all; Node 24 and later enable it by default`;
103106
}
104107
if (major < 24) {

ts/test/test-async-context-frame.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import {AsyncLocalStorage} from 'node:async_hooks';
1919
import {fork} from 'node:child_process';
2020
import {join} from 'node:path';
2121

22+
import {satisfies} from 'semver';
23+
2224
import {isAsyncContextFrameActive} from '../src/async-context-frame';
2325

2426
const addon = require('node-gyp-build')(join(__dirname, '..', '..')) as {
@@ -28,6 +30,8 @@ const addon = require('node-gyp-build')(join(__dirname, '..', '..')) as {
2830
const CHILD = join(__dirname, 'async-context-frame-child.js');
2931

3032
const major = Number(process.versions.node.split('.')[0]);
33+
// ACF landed in 22.7.0, so the opt-in routes are gated on that, not on major 22.
34+
const hasAcfSupport = satisfies(process.versions.node, '>=22.7.0');
3135

3236
interface ChildReport {
3337
active: boolean;
@@ -84,7 +88,7 @@ describe('isAsyncContextFrameActive', () => {
8488
});
8589

8690
it('reports it inactive when Node has no support for it', async function () {
87-
if (major >= 22) return this.skip();
91+
if (hasAcfSupport) return this.skip();
8892
const {active} = await probeChild();
8993
assert.equal(active, false);
9094
});
@@ -112,11 +116,11 @@ describe('isAsyncContextFrameActive', () => {
112116
});
113117

114118
it('reports it active when NODE_OPTIONS turns it on', async function () {
115-
// The mirror image, on the other Node line: 22 and 23 accept the flag in
119+
// The mirror image, on the other Node line: 22.7.0 through 23 accept the flag in
116120
// NODE_OPTIONS (24 rejects it outright), again without it reaching execArgv,
117121
// so inferring from execArgv concludes ACF is off when it is on — and the
118122
// caller refuses to run in a process that would have worked.
119-
if (major < 22 || major >= 24) return this.skip();
123+
if (!hasAcfSupport || major >= 24) return this.skip();
120124
const {active, execArgv} = await probeChild({
121125
nodeOptions: '--experimental-async-context-frame',
122126
});

ts/test/test-get-value-from-map-profiler.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,13 @@
2929
import assert from 'assert';
3030
import {join} from 'path';
3131
import {AsyncLocalStorage} from 'async_hooks';
32-
import {satisfies} from 'semver';
3332

3433
import {isAsyncContextFrameActive} from '../src/async-context-frame';
3534

3635
const findBinding = require('node-gyp-build');
3736
const profiler = findBinding(join(__dirname, '..', '..'));
3837

39-
const useCPED =
40-
isAsyncContextFrameActive() && satisfies(process.versions.node, '>=22.7.0');
38+
const useCPED = isAsyncContextFrameActive();
4139

4240
const supportedPlatform =
4341
process.platform === 'darwin' || process.platform === 'linux';

ts/test/test-otel-thread-ctx.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,9 @@ function tcIsTruncated(): boolean {
6262
}
6363

6464
const isLinux = process.platform === 'linux';
65-
// AsyncContextFrame is the writer's discovery substrate: opt-in on Node 22/23
66-
// (via --experimental-async-context-frame) and on by default from Node 24
65+
// AsyncContextFrame is the writer's discovery substrate: opt-in from Node
66+
// 22.7.0 through 23 (via --experimental-async-context-frame) and on by
67+
// default from Node 24
6768
// (disable-able via --no-async-context-frame). The TS layer refuses to install
6869
// the hook when it isn't active, so the entire describe block is skipped then.
6970
// Asks the same question the source side asks, the same way.

ts/test/test-time-profiler.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ import {fork} from 'child_process';
3232

3333
import assert from 'assert';
3434

35-
const useCPED =
36-
isAsyncContextFrameActive() && satisfies(process.versions.node, '>=22.7.0');
35+
const useCPED = isAsyncContextFrameActive();
3736

3837
const collectAsyncId = satisfies(process.versions.node, '>=24.0.0');
3938

ts/test/worker.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@ const DURATION_MILLIS = 1000;
1212
const intervalMicros = 10000;
1313
const withContexts =
1414
process.platform === 'darwin' || process.platform === 'linux';
15-
const useCPED =
16-
withContexts &&
17-
isAsyncContextFrameActive() &&
18-
satisfies(process.versions.node, '>=22.7.0');
15+
const useCPED = withContexts && isAsyncContextFrameActive();
1916
const collectAsyncId =
2017
withContexts && satisfies(process.versions.node, '>=24.0.0');
2118

ts/test/worker2.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,7 @@ const INTERVAL_MICROS = 10000;
1010
const withContexts =
1111
process.platform === 'darwin' || process.platform === 'linux';
1212

13-
const useCPED =
14-
withContexts &&
15-
isAsyncContextFrameActive() &&
16-
satisfies(process.versions.node, '>=22.7.0');
13+
const useCPED = withContexts && isAsyncContextFrameActive();
1714

1815
const collectAsyncId =
1916
withContexts && satisfies(process.versions.node, '>=24.0.0');

0 commit comments

Comments
 (0)