From fa256b6a37e9956bf171fb2c570b02136de4783c Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 29 Aug 2026 14:36:15 +0000 Subject: [PATCH 1/2] lib: have runFile honor permissions and accept URL/Buffer paths Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 24 +++++--- lib/internal/bench_runner/cli.js | 24 ++++++-- test/parallel/test-bench-run-file.js | 91 ++++++++++++++++++++++++++-- 3 files changed, 120 insertions(+), 19 deletions(-) diff --git a/doc/api/bench.md b/doc/api/bench.md index 2eb6eb6a382..96a5aade5a4 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -504,7 +504,7 @@ for await (const { type, data } of run()) { added: REPLACEME --> -* `path` {string} The absolute path of one benchmark module. +* `path` {string|Buffer|URL} The path of one benchmark module. * `options` {Object} * `env` {Object} The child process environment. Property values must be strings or `undefined`. This replaces, rather than extends, the parent @@ -518,18 +518,22 @@ added: REPLACEME * Returns: {BenchmarksStream} Runs exactly one benchmark module in a fresh child process and returns its -object-mode event stream. `path` is not interpreted as a glob. Unless the signal -is aborted or the stream is destroyed before startup, every call uses a new -child. Input discovery, ordering, concurrency, retries, and multi-file -scheduling remain the caller's responsibility. +object-mode event stream. A relative `path` is resolved from the current working +directory when `runFile()` is called. `path` is not interpreted as a glob. +Unless the signal is aborted or the stream is destroyed before startup, every +call uses a new child. Input discovery, ordering, concurrency, retries, and +multi-file scheduling remain the caller's responsibility. + +When the Permission Model is enabled, the caller must have file system read +access to `path` and permission to create child processes. Records use advanced child process serialization, preserving supported structured values such as `bigint` and errors. Child writes to stdout and stderr -become `'bench:diagnostic'` records. A module loading error, abnormal child exit, -or cancellation also emits an error diagnostic and produces a terminal -`'bench:summary'` whose `success` property is `false`; these execution failures -do not error the stream. If module evaluation fails after declaring benchmarks, -those declarations still run before the unsuccessful summary. +become `'bench:diagnostic'` records. A permission failure, module loading error, +abnormal child exit, or cancellation also emits an error diagnostic and produces +a terminal `'bench:summary'` whose `success` property is `false`; these execution +failures do not error the stream. If module evaluation fails after declaring +benchmarks, those declarations still run before the unsuccessful summary. `env`, effective inherited options, and an explicitly provided `execArgv` are copied when `runFile()` is called. The runner removes `NODE_OPTIONS`, replaces diff --git a/lib/internal/bench_runner/cli.js b/lib/internal/bench_runner/cli.js index 4b14493e391..8e3d62257b6 100644 --- a/lib/internal/bench_runner/cli.js +++ b/lib/internal/bench_runner/cli.js @@ -36,10 +36,14 @@ const { StringPrototypeStartsWith, StringPrototypeToUpperCase, SymbolDispose, + uncurryThis, } = primordials; +const { Buffer } = require('buffer'); +const BufferToString = uncurryThis(Buffer.prototype.toString); const { spawn } = require('child_process'); const { createWriteStream, statSync } = require('fs'); const { Glob } = require('internal/fs/glob'); +const { getValidatedPath } = require('internal/fs/utils'); const { BenchmarksStream, } = require('internal/bench_runner/benchmarks_stream'); @@ -53,6 +57,7 @@ const { deserializeError, serializeError } = require('internal/error_serdes'); const { AbortError, codes: { + ERR_ACCESS_DENIED, ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_INVALID_STATE, @@ -64,6 +69,7 @@ const { getOptionValue, getOptionsAsFlagsFromBinding, } = require('internal/options'); +const permission = require('internal/process/permission'); const { TIMEOUT_MAX } = require('internal/timers'); const { kEmptyObject } = require('internal/util'); const { @@ -75,7 +81,7 @@ const { } = require('internal/validators'); const { pathToFileURL } = require('internal/url'); const { pipeline } = require('stream/promises'); -const { isAbsolute, resolve, sep } = require('path'); +const { resolve, sep } = require('path'); const { clearTimeout, setTimeout } = require('timers'); const console = require('internal/console/global'); @@ -758,6 +764,16 @@ async function runChild(path, options, scope, onRecord) { }), }; } + const resource = resolve(options.cwd, path); + if (permission.isEnabled() && + !permission.has('fs.read', resource) && + !permission.isAuditMode()) { + throw new ERR_ACCESS_DENIED( + 'Access to this API has been restricted. Use --allow-fs-read to manage permissions.', + 'FileSystemRead', + resource, + ); + } const child = spawn( options.execPath ?? process.execPath, getChildArgs(path, options), @@ -1116,10 +1132,8 @@ async function runIsolated(files, options, output) { } function runFile(path, options = kEmptyObject) { - validateStringWithoutNullBytes(path, 'path'); - if (!isAbsolute(path)) { - throw new ERR_INVALID_ARG_VALUE('path', path, 'must be an absolute path'); - } + path = getValidatedPath(path); + if (typeof path !== 'string') path = BufferToString(path); const file = resolve(path); validateObject(options, 'options'); const { diff --git a/test/parallel/test-bench-run-file.js b/test/parallel/test-bench-run-file.js index eccbf2e7296..600f1a212ae 100644 --- a/test/parallel/test-bench-run-file.js +++ b/test/parallel/test-bench-run-file.js @@ -10,9 +10,9 @@ const path = require('path'); const { runFile } = require('node:bench'); const fixture = fixtures.path('bench-runner/run-file.cjs'); +const relativeFixture = path.relative(process.cwd(), fixture); assert.throws(() => runFile(null), { code: 'ERR_INVALID_ARG_TYPE' }); -assert.throws(() => runFile('relative.cjs'), { code: 'ERR_INVALID_ARG_VALUE' }); assert.throws(() => runFile(fixture, null), { code: 'ERR_INVALID_ARG_TYPE' }); assert.throws(() => runFile(fixture, { execArgv: null }), { code: 'ERR_INVALID_ARG_TYPE', @@ -50,6 +50,9 @@ assert.throws(() => runFile(fixture, { assert.throws(() => runFile(`${fixture}\0`), { code: 'ERR_INVALID_ARG_VALUE', }); +assert.throws(() => runFile(Buffer.from(`${fixture}\0`)), { + code: 'ERR_INVALID_ARG_VALUE', +}); assert.throws(() => runFile(fixture, { env: null }), { code: 'ERR_INVALID_ARG_TYPE', }); @@ -79,7 +82,7 @@ async function testRunFile() { NODE_CHANNEL_FD: '999', NODE_CHANNEL_SERIALIZATION_MODE: 'json', }; - const stream = runFile(fixture, { env, execArgv }); + const stream = runFile(relativeFixture, { env, execArgv }); execArgv.length = 0; env.NODE_BENCH_RUN_FILE = 'mutated'; const records = await stream.toArray(); @@ -108,8 +111,11 @@ async function testRunFile() { async function testConcurrentCalls() { const [cjsRecords, esmRecords] = await Promise.all([ - runFile(fixtures.path('bench-runner/a.cjs')).toArray(), - runFile(fixtures.path('bench-runner/b.mjs')).toArray(), + runFile(fixtures.fileURL('bench-runner/a.cjs')).toArray(), + runFile(Buffer.from(path.relative( + process.cwd(), + fixtures.path('bench-runner/b.mjs'), + ))).toArray(), ]); const cjsResult = cjsRecords.find( ({ type }) => type === 'bench:complete').data; @@ -119,6 +125,14 @@ async function testConcurrentCalls() { assert.strictEqual(esmResult.name, 'beta'); assert.notStrictEqual(cjsResult.params.pid, esmResult.params.pid); assert.notStrictEqual(cjsResult.runId, esmResult.runId); + assert.strictEqual( + cjsRecords.at(-1).data.file, + fixtures.path('bench-runner/a.cjs'), + ); + assert.strictEqual( + esmRecords.at(-1).data.file, + fixtures.path('bench-runner/b.mjs'), + ); } async function testLoadFailure() { @@ -280,6 +294,74 @@ function testEvalParent() { assert.strictEqual(result.stdout, 'true\n'); } +function testPermissions() { + const fsReadScript = ` + const assert = require('assert'); + const { runFile } = require('node:bench'); + const { pathToFileURL } = require('url'); + const target = ${JSON.stringify(fixture)}; + assert.strictEqual(process.permission.has('child'), true); + assert.strictEqual(process.permission.has('fs.read', target), false); + Promise.all([ + target, + Buffer.from(target), + pathToFileURL(target), + ].map(async (input) => { + const records = await runFile(input).toArray(); + const diagnostic = records.find(({ type, data }) => + type === 'bench:diagnostic' && + data.error?.code === 'ERR_ACCESS_DENIED'); + assert.strictEqual(diagnostic.data.error.permission, 'FileSystemRead'); + assert.strictEqual(diagnostic.data.error.resource, target); + assert.strictEqual(records.at(-1).type, 'bench:summary'); + assert.strictEqual(records.at(-1).data.success, false); + })).catch((error) => { + console.error(error); + process.exitCode = 1; + }); + `; + const result = spawnSync(process.execPath, [ + '--no-warnings', + '--permission', + '--allow-child-process', + '-e', + fsReadScript, + ], { encoding: 'utf8' }); + assert.strictEqual(result.status, 0, result.stderr); + + const childProcessScript = ` + const assert = require('assert'); + const { runFile } = require('node:bench'); + const target = ${JSON.stringify(fixture)}; + assert.strictEqual(process.permission.has('fs.read', target), true); + assert.strictEqual(process.permission.has('child'), false); + runFile(target).toArray().then((records) => { + const diagnostic = records.find(({ type, data }) => + type === 'bench:diagnostic' && + data.error?.code === 'ERR_ACCESS_DENIED'); + assert.strictEqual(diagnostic.data.error.permission, 'ChildProcess'); + assert.strictEqual(diagnostic.data.error.resource, process.execPath); + assert.strictEqual(records.at(-1).type, 'bench:summary'); + assert.strictEqual(records.at(-1).data.success, false); + }).catch((error) => { + console.error(error); + process.exitCode = 1; + }); + `; + const childProcessResult = spawnSync(process.execPath, [ + '--no-warnings', + '--permission', + `--allow-fs-read=${fixture}`, + '-e', + childProcessScript, + ], { encoding: 'utf8' }); + assert.strictEqual( + childProcessResult.status, + 0, + childProcessResult.stderr, + ); +} + async function testPreAborted() { const records = await runFile(fixture, { signal: AbortSignal.abort(new Error('already cancelled')), @@ -315,4 +397,5 @@ async function testDestroy() { await testPreAborted(); await testDestroy(); testEvalParent(); + testPermissions(); })().then(common.mustCall()); From 4358154b9197f7dc07172c509e1ed735d0213a4c Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 29 Aug 2026 15:11:16 +0000 Subject: [PATCH 2/2] lib: improve diagnostic message support Support serializable context.diagnostic messages and optionally listen to diagnostic_channel messages Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 25 +++- doc/api/cli.md | 4 +- doc/node.1 | 1 + lib/internal/bench_runner/benchmark.js | 48 ++++++- lib/internal/bench_runner/cli.js | 5 +- lib/internal/bench_runner/harness.js | 36 ++++++ test/fixtures/bench-runner/diagnostic.cjs | 16 ++- test/parallel/test-bench-cli.js | 9 +- test/parallel/test-bench-context-errors.js | 8 +- .../test-bench-diagnostic-channels.js | 119 ++++++++++++++++++ test/parallel/test-bench-diagnostics.js | 10 +- test/parallel/test-bench-validation.js | 4 + 12 files changed, 253 insertions(+), 32 deletions(-) create mode 100644 test/parallel/test-bench-diagnostic-channels.js diff --git a/doc/api/bench.md b/doc/api/bench.md index 96a5aade5a4..842e8a59264 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -316,6 +316,9 @@ added: REPLACEME * `name` {string} The benchmark name. **Default:** The `name` property of `fn`, or `''` when `fn` has no name. * `options` {Object} + * `diagnosticChannels` {Array} String diagnostics channel names, deduplicated + and inherited from containing suites by union. Symbol values in the array + are silently ignored. **Default:** `[]`. * `only` {boolean} When any benchmark or containing suite has `only` set, benchmarks without `only` in their hierarchy are skipped. **Default:** `false`. @@ -346,6 +349,12 @@ samples, but their samples are discarded. An exception, rejection, timeout, abort, missing timing call, or duplicate timing call stops the current benchmark. Later benchmarks continue to run. +For each warmup and measured callback, the runner subscribes to the configured +diagnostics channels. Each publication queues a context diagnostic whose +`message` is `{ name, message }`, containing the string channel name and the +published message. Subscriptions are removed when the callback settles or is +aborted. + A timeout or abort cannot interrupt synchronous JavaScript and does not forcibly cancel asynchronous work that ignores `context.signal`. @@ -388,6 +397,9 @@ added: REPLACEME * `name` {string} The suite name. **Default:** The `name` property of `fn`, or `''` when `fn` has no name. * `options` {Object} + * `diagnosticChannels` {Array} String diagnostics channel names inherited by + nested suites and benchmarks. Symbol values in the array are silently + ignored. **Default:** `[]`. * `only` {boolean} Selects all benchmarks nested in this suite. **Default:** `false`. * `skip` {boolean|string} Skips all benchmarks nested in this suite. @@ -663,7 +675,8 @@ message transport from the duration. `record()` is mutually exclusive with added: REPLACEME --> -* `message` {string} The diagnostic message. +* `message` {any} A structured-cloneable diagnostic value. With CLI process + isolation, it must also be supported by advanced child process serialization. * `options` {Object} * `level` {string} Either `'info'` or `'warning'`. **Default:** `'info'`. * `detail` {any} Additional structured-cloneable diagnostic data. With CLI @@ -679,10 +692,10 @@ before a callback failure are emitted before the failed `'bench:complete'` event and do not themselves cause the benchmark to fail. If a timeout or abort wins before the callback settles, queued diagnostics might not be emitted. -The message and options are validated, and detail is cloned, synchronously. -Calling `diagnostic()` between `context.start()` and `context.end()` therefore -includes that work in the measured duration. Invalid arguments or an -uncloneable detail violate the sample contract. +The message and detail are cloned synchronously. Options are also validated +synchronously. Calling `diagnostic()` between `context.start()` and +`context.end()` therefore includes that work in the measured duration. Invalid +arguments or an uncloneable message or detail violate the sample contract. ### `context.done()` @@ -751,6 +764,8 @@ isolation, all files share one runner and their plans are emitted before any benchmark executes. Plan data contains the benchmark-scoped identity, location, tags, and parameters described in [benchmark result][], together with: +* `diagnosticChannels` {string\[]} The inherited string channel names + subscribed to during each callback. * `samples` {number} The effective maximum number of measured callback invocations after run-level overrides. * `warmup` {number} The effective number of unreported warmup callback diff --git a/doc/api/cli.md b/doc/api/cli.md index 6e98ef66da9..d74253d7791 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -492,9 +492,7 @@ benchmark runner process. This reduces startup overhead but allows module, heap, and process state to carry between files. User writes to stdout or stderr also share destinations with benchmark reporters in this mode. -The supported modes are `'process'` and `'none'`. Worker-thread isolation is not -a CLI mode. Higher-level tools can implement it using externally measured -samples as described in the [benchmark runner][] documentation. +The supported modes are `'process'` and `'none'`. ### `--bench-name-pattern=pattern` diff --git a/doc/node.1 b/doc/node.1 index d7343690215..225f4097512 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -304,6 +304,7 @@ When \fBmode\fR is \fB'none'\fR, all matching files and benchmarks run serially benchmark runner process. This reduces startup overhead but allows module, heap, and process state to carry between files. User writes to stdout or stderr also share destinations with benchmark reporters in this mode. +The supported modes are \fB'process'\fR and \fB'none'\fR. . .It Fl -bench-name-pattern Ns = Ns Ar pattern Only runs benchmarks whose full hierarchical name matches the JavaScript diff --git a/lib/internal/bench_runner/benchmark.js b/lib/internal/bench_runner/benchmark.js index 93fe3459570..375f8e6b635 100644 --- a/lib/internal/bench_runner/benchmark.js +++ b/lib/internal/bench_runner/benchmark.js @@ -48,6 +48,7 @@ const { structuredClone } = require('internal/worker/js_transferable'); const { bigint: hrtime } = process.hrtime; const kDefaultSamples = 30; const kDefaultWarmup = 0; +const kEmptyDiagnosticChannels = ObjectFreeze([]); const kEmptyNamePath = ObjectFreeze([]); const kEmptyParams = ObjectFreeze({ __proto__: null }); const kEmptyTags = ObjectFreeze([]); @@ -82,6 +83,33 @@ function canonicalizeTags(tags, parentTags = kEmptyTags) { return ObjectFreeze(result); } +function canonicalizeDiagnosticChannels( + diagnosticChannels, + parentDiagnosticChannels = kEmptyDiagnosticChannels, +) { + if (diagnosticChannels === undefined) return parentDiagnosticChannels; + if (!ArrayIsArray(diagnosticChannels)) { + throw new ERR_INVALID_ARG_TYPE( + 'options.diagnosticChannels', 'Array', diagnosticChannels); + } + + const result = ArrayPrototypeSlice(parentDiagnosticChannels); + const seen = new SafeSet(parentDiagnosticChannels); + for (let i = 0; i < diagnosticChannels.length; i++) { + const name = diagnosticChannels[i]; + if (typeof name === 'symbol') continue; + if (typeof name !== 'string') { + throw new ERR_INVALID_ARG_TYPE( + `options.diagnosticChannels[${i}]`, ['string', 'symbol'], name); + } + if (!seen.has(name)) { + seen.add(name); + ArrayPrototypePush(result, name); + } + } + return ObjectFreeze(result); +} + function canonicalizeParams(params) { if (params === undefined) return kEmptyParams; validateObject(params, 'options.params'); @@ -106,15 +134,17 @@ function canonicalizeParams(params) { return ObjectFreeze(result); } -function validateNodeOptions(options, parentTags) { +function validateNodeOptions(options, parentTags, parentDiagnosticChannels) { validateObject(options, 'options'); - const { only = false, skip, tags } = options; + const { diagnosticChannels, only = false, skip, tags } = options; if (typeof only !== 'boolean') { throw new ERR_INVALID_ARG_TYPE('options.only', 'boolean', only); } validateSkip(skip); return { __proto__: null, + diagnosticChannels: canonicalizeDiagnosticChannels( + diagnosticChannels, parentDiagnosticChannels), only, skip, tags: canonicalizeTags(tags, parentTags), @@ -157,7 +187,10 @@ class Suite extends AsyncResource { constructor(harness, parent, name, options, fn, loc, isRoot = false) { super('BenchSuite'); const validated = validateNodeOptions( - options, parent?.tags ?? kEmptyTags); + options, + parent?.tags ?? kEmptyTags, + parent?.diagnosticChannels ?? kEmptyDiagnosticChannels, + ); this.harness = harness; this.parent = parent; @@ -174,6 +207,7 @@ class Suite extends AsyncResource { this.namePath, ]); this.parentId = isRoot || parent.isRoot ? null : parent.suiteId; + this.diagnosticChannels = validated.diagnosticChannels; this.only = validated.only; this.skip = validated.skip; this.tags = validated.tags; @@ -195,7 +229,8 @@ class Suite extends AsyncResource { class Bench extends AsyncResource { constructor(harness, parent, name, options, fn, loc) { super('Benchmark'); - const validated = validateNodeOptions(options, parent.tags); + const validated = validateNodeOptions( + options, parent.tags, parent.diagnosticChannels); const { params, samples = kDefaultSamples, @@ -216,6 +251,7 @@ class Bench extends AsyncResource { this.name = name; this.fn = fn; this.loc = createLocation(loc, harness.entryFile); + this.diagnosticChannels = validated.diagnosticChannels; this.only = validated.only; this.skip = validated.skip; this.tags = validated.tags; @@ -275,7 +311,7 @@ class BenchContext { throw new ERR_INVALID_STATE('benchmark sample is no longer active'); } try { - validateString(message, 'message'); + const clonedMessage = structuredClone(message); validateObject(options, 'options'); const { detail, level = 'info' } = options; validateString(level, 'options.level'); @@ -283,7 +319,7 @@ class BenchContext { throw new ERR_INVALID_ARG_VALUE( 'options.level', level, "must be 'info' or 'warning'"); } - const diagnostic = { __proto__: null, level, message }; + const diagnostic = { __proto__: null, level, message: clonedMessage }; if (detail !== undefined) diagnostic.detail = structuredClone(detail); this.#onDiagnostic(diagnostic); } catch (error) { diff --git a/lib/internal/bench_runner/cli.js b/lib/internal/bench_runner/cli.js index 8e3d62257b6..0cc4dc5210a 100644 --- a/lib/internal/bench_runner/cli.js +++ b/lib/internal/bench_runner/cli.js @@ -440,7 +440,8 @@ function validateRecord(record) { warmup, yieldBetweenSamples, } = record.data; - if (typeof record.data.file !== 'string' || + if (!isStringArray(record.data.diagnosticChannels) || + typeof record.data.file !== 'string' || !NumberIsSafeInteger(record.data.line) || record.data.line < 0 || !NumberIsSafeInteger(record.data.column) || record.data.column < 0 || !isStringArray(record.data.tags) || @@ -471,7 +472,7 @@ function validateRecord(record) { record.data.phase !== 'measurement') || !NumberIsSafeInteger(record.data.index) || record.data.index < 0 || record.data.index > 0xFFFFFFFF || - typeof record.data.message !== 'string' || + !ObjectPrototypeHasOwnProperty(record.data, 'message') || (record.data.level !== 'info' && record.data.level !== 'warning') || typeof record.data.file !== 'string' || !NumberIsSafeInteger(record.data.line) || record.data.line < 0 || diff --git a/lib/internal/bench_runner/harness.js b/lib/internal/bench_runner/harness.js index f3e28bc01b1..c18f439f671 100644 --- a/lib/internal/bench_runner/harness.js +++ b/lib/internal/bench_runner/harness.js @@ -24,6 +24,10 @@ const { const { getCallerLocation } = internalBinding('util'); const { exitCodes: { kGenericUserError } } = internalBinding('errors'); const { AsyncLocalStorage } = require('async_hooks'); +const { + subscribe: subscribeToChannel, + unsubscribe: unsubscribeFromChannel, +} = require('diagnostics_channel'); const { AbortController } = require('internal/abort_controller'); const { AbortError, @@ -406,6 +410,7 @@ class Harness { parentId: benchmark.parentId, name: benchmark.name, namePath: ArrayPrototypeSlice(benchmark.namePath), + diagnosticChannels: ArrayPrototypeSlice(benchmark.diagnosticChannels), file: benchmark.loc.file, line: benchmark.loc.line, column: benchmark.loc.column, @@ -720,9 +725,37 @@ class Harness { const context = new BenchContext( benchmark, signal, phase, index, (diagnostic) => ArrayPrototypePush(diagnostics, diagnostic)); + const channels = benchmark.diagnosticChannels; + let abortSubscription; + let diagnosticError; + let diagnosticFailed = false; + let subscribed = 0; + const onMessage = (message, name) => { + if (signal.aborted || diagnosticFailed) return; + try { + context.diagnostic({ __proto__: null, name, message }); + } catch (error) { + diagnosticError = error; + diagnosticFailed = true; + } + }; + const unsubscribe = () => { + while (subscribed > 0) { + subscribed--; + unsubscribeFromChannel(channels[subscribed], onMessage); + } + }; try { + for (let i = 0; i < channels.length; i++) { + subscribeToChannel(channels[i], onMessage); + subscribed++; + } + if (subscribed > 0) { + abortSubscription = addAbortListener(signal, unsubscribe); + } await this.#invoke( benchmark, benchmark, benchmark.fn, [context]); + if (diagnosticFailed) throw diagnosticError; const { done, sample } = context.finish(); return { __proto__: null, @@ -739,6 +772,9 @@ class Harness { error, failed: true, }; + } finally { + abortSubscription?.[SymbolDispose](); + unsubscribe(); } } diff --git a/test/fixtures/bench-runner/diagnostic.cjs b/test/fixtures/bench-runner/diagnostic.cjs index 018b8a31de4..9e413e943dd 100644 --- a/test/fixtures/bench-runner/diagnostic.cjs +++ b/test/fixtures/bench-runner/diagnostic.cjs @@ -1,11 +1,17 @@ 'use strict'; const { bench } = require('node:bench'); +const { channel } = require('diagnostics_channel'); -bench('diagnostic relay', { samples: 1 }, (b) => { - b.diagnostic('relayed warning', { - detail: { value: 42n }, - level: 'warning', - }); +const channelName = 'node:bench:test:diagnostic'; +const diagnosticChannel = channel(channelName); + +bench('diagnostic relay', { + diagnosticChannels: [channelName], + samples: 1, +}, (b) => { + const message = { value: 42n }; + diagnosticChannel.publish(message); + message.value = 0n; b.record({ duration_ns: 1n, operations: 1 }); }); diff --git a/test/parallel/test-bench-cli.js b/test/parallel/test-bench-cli.js index d0f9629ad9a..21090814f86 100644 --- a/test/parallel/test-bench-cli.js +++ b/test/parallel/test-bench-cli.js @@ -399,11 +399,14 @@ for (const isolation of ['process', 'none']) { ({ type }) => type === 'bench:diagnostic').data; const completion = records.find( ({ type }) => type === 'bench:complete').data; - assert.strictEqual(diagnostic.message, 'relayed warning'); - assert.strictEqual(diagnostic.level, 'warning'); + assert.deepStrictEqual(diagnostic.message, { + name: 'node:bench:test:diagnostic', + message: { value: '42' }, + }); + assert.strictEqual(diagnostic.level, 'info'); assert.strictEqual(diagnostic.phase, 'measurement'); assert.strictEqual(diagnostic.index, 0); - assert.deepStrictEqual(diagnostic.detail, { value: '42' }); + assert.strictEqual(diagnostic.detail, undefined); assert.strictEqual(diagnostic.benchId, completion.benchId); assert.strictEqual(diagnostic.fileRunId, completion.fileRunId); assert.strictEqual(completion.error, undefined); diff --git a/test/parallel/test-bench-context-errors.js b/test/parallel/test-bench-context-errors.js index ad2be4aa127..9ba104443ad 100644 --- a/test/parallel/test-bench-context-errors.js +++ b/test/parallel/test-bench-context-errors.js @@ -59,8 +59,8 @@ runner.bench('reentrant record', { samples: 1 }, (b) => { runner.bench('uncloneable detail', { samples: 1 }, (b) => { b.record({ duration_ns: 1n, operations: 1, detail: () => {} }); }); -runner.bench('invalid diagnostic message', { samples: 1 }, (b) => { - b.diagnostic(1); +runner.bench('uncloneable diagnostic message', { samples: 1 }, (b) => { + b.diagnostic(() => {}); }); runner.bench('invalid diagnostic level', { samples: 1 }, (b) => { b.diagnostic('invalid', { level: 'error' }); @@ -113,8 +113,8 @@ runner.bench('caught diagnostic violation', { samples: 1 }, 'ERR_INVALID_STATE'); assert.strictEqual(byName.get('uncloneable detail').error.name, 'DataCloneError'); - assert.strictEqual(byName.get('invalid diagnostic message').error.code, - 'ERR_INVALID_ARG_TYPE'); + assert.strictEqual(byName.get('uncloneable diagnostic message').error.name, + 'DataCloneError'); assert.strictEqual(byName.get('invalid diagnostic level').error.code, 'ERR_INVALID_ARG_VALUE'); assert.strictEqual(byName.get('uncloneable diagnostic detail').error.name, diff --git a/test/parallel/test-bench-diagnostic-channels.js b/test/parallel/test-bench-diagnostic-channels.js new file mode 100644 index 00000000000..9e1bb2a0702 --- /dev/null +++ b/test/parallel/test-bench-diagnostic-channels.js @@ -0,0 +1,119 @@ +// Flags: --no-warnings +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const dc = require('diagnostics_channel'); +const { createRunner } = require('node:bench'); + +const prefix = `node:bench:test:${process.pid}`; +const inheritedName = `${prefix}:inherited`; +const nestedName = `${prefix}:nested`; +const benchmarkName = `${prefix}:benchmark`; +const unlistedName = `${prefix}:unlisted`; +const symbolName = Symbol(`${prefix}:symbol`); +const inheritedChannel = dc.channel(inheritedName); +const nestedChannel = dc.channel(nestedName); +const benchmarkChannel = dc.channel(benchmarkName); +const unlistedChannel = dc.channel(unlistedName); +const symbolChannel = dc.channel(symbolName); + +function recordSample(context) { + context.record({ duration_ns: 1n, operations: 1 }); +} + +async function testCapture() { + const runner = createRunner({ yieldBetweenSamples: false }); + runner.suite('outer', { + diagnosticChannels: [inheritedName, symbolName, inheritedName], + }, common.mustCall(() => { + runner.suite('inner', { + diagnosticChannels: [nestedName], + }, common.mustCall(() => { + runner.bench('captured', { + diagnosticChannels: [benchmarkName, nestedName], + samples: 1, + }, common.mustCall((context) => { + const message = { value: 1 }; + inheritedChannel.publish(message); + message.value = 0; + nestedChannel.publish({ value: 2 }); + benchmarkChannel.publish({ value: 3 }); + unlistedChannel.publish({ value: 4 }); + symbolChannel.publish({ value: 5 }); + recordSample(context); + })); + })); + })); + runner.bench('not captured', { samples: 1 }, common.mustCall((context) => { + inheritedChannel.publish({ value: 6 }); + recordSample(context); + })); + + const records = await runner.run().toArray(); + const plan = records.find( + ({ type, data }) => type === 'bench:plan' && data.name === 'captured').data; + assert.deepStrictEqual(plan.diagnosticChannels, [ + inheritedName, + nestedName, + benchmarkName, + ]); + const diagnostics = records.filter( + ({ type }) => type === 'bench:diagnostic').map(({ data }) => data); + assert.deepStrictEqual(diagnostics.map(({ message }) => message), [ + { name: inheritedName, message: { value: 1 } }, + { name: nestedName, message: { value: 2 } }, + { name: benchmarkName, message: { value: 3 } }, + ]); + assert(diagnostics.every(({ level }) => level === 'info')); + assert.strictEqual(inheritedChannel.hasSubscribers, false); + assert.strictEqual(nestedChannel.hasSubscribers, false); + assert.strictEqual(benchmarkChannel.hasSubscribers, false); + assert.strictEqual(symbolChannel.hasSubscribers, false); +} + +async function testUncloneableMessage() { + const name = `${prefix}:uncloneable`; + const channel = dc.channel(name); + const runner = createRunner({ yieldBetweenSamples: false }); + runner.bench('uncloneable', { + diagnosticChannels: [name], + samples: 1, + }, common.mustCall((context) => { + channel.publish(() => {}); + recordSample(context); + })); + const records = await runner.run().toArray(); + const result = records.find( + ({ type }) => type === 'bench:complete').data; + assert.strictEqual(result.error.name, 'DataCloneError'); + assert.strictEqual(channel.hasSubscribers, false); +} + +async function testAbortCleanup() { + const name = `${prefix}:abort`; + const channel = dc.channel(name); + const controller = new AbortController(); + const runner = createRunner({ yieldBetweenSamples: false }); + runner.bench('abort', { + diagnosticChannels: [name], + samples: 1, + signal: controller.signal, + }, common.mustCall((context) => { + controller.abort(new Error('stop')); + assert.strictEqual(channel.hasSubscribers, false); + channel.publish({ ignored: true }); + recordSample(context); + })); + const records = await runner.run().toArray(); + assert.strictEqual( + records.some(({ type }) => type === 'bench:diagnostic'), + false, + ); +} + +(async () => { + await testCapture(); + await testUncloneableMessage(); + await testAbortCleanup(); +})().then(common.mustCall()); diff --git a/test/parallel/test-bench-diagnostics.js b/test/parallel/test-bench-diagnostics.js index 9f8facbcb16..9e33fd56323 100644 --- a/test/parallel/test-bench-diagnostics.js +++ b/test/parallel/test-bench-diagnostics.js @@ -99,12 +99,14 @@ async function testAfterEachFailurePrecedence() { }, common.mustCall((b) => { finalContext = b; const detail = { index: b.index }; + const message = { index: b.index, phase: b.phase }; const level = b.phase === 'warmup' ? 'info' : 'warning'; - assert.strictEqual(b.diagnostic(`${b.phase} ${b.index}`, { + assert.strictEqual(b.diagnostic(message, { detail, level, }), undefined); detail.index = -1; + message.index = -1; recordSample(b); }, 3)); const expectedError = new Error('benchmark failed'); @@ -135,9 +137,9 @@ async function testAfterEachFailurePrecedence() { assert.strictEqual(diagnostics.length, 4); assert.strictEqual(namedDiagnostics.length, diagnostics.length); assert.deepStrictEqual(diagnostics.map(({ message }) => message), [ - 'warmup 0', - 'measurement 0', - 'measurement 1', + { index: 0, phase: 'warmup' }, + { index: 0, phase: 'measurement' }, + { index: 1, phase: 'measurement' }, 'before failure', ]); assert.deepStrictEqual(diagnostics.map(({ phase, index, level }) => ({ diff --git a/test/parallel/test-bench-validation.js b/test/parallel/test-bench-validation.js index 7071dd2d915..fa7a579aa60 100644 --- a/test/parallel/test-bench-validation.js +++ b/test/parallel/test-bench-validation.js @@ -34,6 +34,10 @@ assert.throws(() => bench('name', { timeout: -1 }, noop), { code: 'ERR_OUT_OF_RANGE' }); assert.throws(() => bench('name', { signal: {} }, noop), { code: 'ERR_INVALID_ARG_TYPE' }); +assert.throws(() => bench('name', { diagnosticChannels: 'channel' }, noop), + { code: 'ERR_INVALID_ARG_TYPE' }); +assert.throws(() => bench('name', { diagnosticChannels: [1] }, noop), + { code: 'ERR_INVALID_ARG_TYPE' }); assert.throws(() => bench('name', { tags: 'fast' }, noop), { code: 'ERR_INVALID_ARG_TYPE' }); assert.throws(() => bench('name', { tags: [''] }, noop),