From ed08636a19dfc5b26a683ea08bd6cf35c80d5d5b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 18 Jul 2026 09:33:21 +0000 Subject: [PATCH 1/4] diagnostics_channel: add BoundedChannel, withStoreScope; make TracingChannel props non-enumerable Close four gaps against Node.js v26: - Export boundedChannel() and the BoundedChannel class, which bundles start/end Channels with subscribe/unsubscribe/withScope/run helpers. - Add Channel.prototype.withStoreScope(data) on both the inactive and active prototypes. The active variant returns a disposable that enters every bound store via AsyncLocalStorage#withScope, publishes the data, and restores the stores on [Symbol.dispose]. - Back TracingChannel with two private BoundedChannel instances and expose start/end/asyncStart/asyncEnd via prototype accessors, with error defined as a non-enumerable own property. Object.keys(tc) now yields [] as it does on Node.js, so spread/for-in/serialisation of a TracingChannel agree across runtimes. - Add TracingChannel.prototype.hasSubscribers delegating to the underlying windows and error channel. traceSync/tracePromise/traceCallback continue to use the existing runStores path via the new accessors, so the existing node parallel tests are unchanged. Imports Node's test-diagnostics-channel-bounded-channel.js unmodified. --- src/js/node/diagnostics_channel.ts | 273 ++++++++++++++++-- .../diagnostics_channel.test.ts | 194 ++++++++++++- ...est-diagnostics-channel-bounded-channel.js | 105 +++++++ 3 files changed, 539 insertions(+), 33 deletions(-) create mode 100644 test/js/node/test/parallel/test-diagnostics-channel-bounded-channel.js diff --git a/src/js/node/diagnostics_channel.ts b/src/js/node/diagnostics_channel.ts index 4c26ff1017fe..26a52c64c5c2 100644 --- a/src/js/node/diagnostics_channel.ts +++ b/src/js/node/diagnostics_channel.ts @@ -1,7 +1,8 @@ // Hardcoded module "node:diagnostics_channel" -// Reference: https://github.com/nodejs/node/blob/fb47afc335ef78a8cef7eac52b8ee7f045300696/lib/diagnostics_channel.js +// Reference: https://github.com/nodejs/node/blob/v26.3.0/lib/diagnostics_channel.js const { validateFunction } = require("internal/validators"); +const { kEmptyObject } = require("internal/shared"); const SafeMap = Map; const SafeFinalizationRegistry = FinalizationRegistry; @@ -9,8 +10,10 @@ const SafeFinalizationRegistry = FinalizationRegistry; const ArrayPrototypeAt = Array.prototype.at; const ArrayPrototypeIndexOf = Array.prototype.indexOf; const ArrayPrototypeSplice = Array.prototype.splice; +const ObjectDefineProperty = Object.defineProperty; const ObjectGetPrototypeOf = Object.getPrototypeOf; const ObjectSetPrototypeOf = Object.setPrototypeOf; +const SymbolDispose = Symbol.dispose; const SymbolHasInstance = Symbol.hasInstance; const PromiseResolve = Promise.$resolve.bind(Promise); const PromiseReject = Promise.$reject.bind(Promise); @@ -91,6 +94,49 @@ function wrapStoreRun(store, data, next, transform = defaultTransform) { }; } +class RunStoresScope { + #stack; + + constructor(activeChannel, data) { + const stack = new DisposableStack(); + let taken = false; + + try { + if (activeChannel._stores) { + for (const entry of activeChannel._stores.entries()) { + const store = entry[0]; + const transform = entry[1]; + + let newContext = data; + if (transform) { + try { + newContext = transform(data); + } catch (err) { + process.nextTick(() => reportError(err)); + continue; + } + } + + stack.use(store.withScope(newContext)); + } + } + + activeChannel.publish(data); + + this.#stack = stack.move(); + taken = true; + } finally { + if (!taken) stack[SymbolDispose](); + } + } + + [SymbolDispose]() { + this.#stack[SymbolDispose](); + } +} + +const noopDisposable = { __proto__: null, [SymbolDispose]() {} }; + class ActiveChannel { _subscribers; name; @@ -149,6 +195,10 @@ class ActiveChannel { } } + withStoreScope(data) { + return new RunStoresScope(this, data); + } + runStores(data, fn, thisArg, ...args) { let run = () => { this.publish(data); @@ -210,6 +260,10 @@ class Channel { runStores(data, fn, thisArg, ...args) { return fn.$apply(thisArg, args); } + + withStoreScope() { + return noopDisposable; + } } const channels = new WeakRefMap(); @@ -240,7 +294,7 @@ function hasSubscribers(name) { return channel.hasSubscribers; } -const traceEvents = ["start", "end", "asyncStart", "asyncEnd", "error"]; +const boundedEvents = ["start", "end"]; function assertChannel(value, name) { if (!(value instanceof Channel)) { @@ -248,41 +302,67 @@ function assertChannel(value, name) { } } -class TracingChannel { - start; - end; - asyncStart; - asyncEnd; - error; +function channelFromMap(nameOrChannels, name, className) { + if (typeof nameOrChannels === "string") { + return channel(`tracing:${nameOrChannels}:${name}`); + } + + if (typeof nameOrChannels === "object" && nameOrChannels !== null) { + const channel = nameOrChannels[name]; + assertChannel(channel, `nameOrChannels.${name}`); + return channel; + } + + throw $ERR_INVALID_ARG_TYPE("nameOrChannels", ["string", "object", className], nameOrChannels); +} +class BoundedChannelScope { + #context; + #end; + #scope; + + constructor(boundedChannel, context) { + if (!boundedChannel.hasSubscribers) { + return; + } + + const { start, end } = boundedChannel; + this.#context = context; + this.#end = end; + + this.#scope = new RunStoresScope(start, context); + } + + [SymbolDispose]() { + if (!this.#scope) { + return; + } + + this.#end.publish(this.#context); + + this.#scope[SymbolDispose](); + this.#scope = undefined; + } +} + +class BoundedChannel { constructor(nameOrChannels) { - if (typeof nameOrChannels === "string") { - this.start = channel(`tracing:${nameOrChannels}:start`); - this.end = channel(`tracing:${nameOrChannels}:end`); - this.asyncStart = channel(`tracing:${nameOrChannels}:asyncStart`); - this.asyncEnd = channel(`tracing:${nameOrChannels}:asyncEnd`); - this.error = channel(`tracing:${nameOrChannels}:error`); - } else if (typeof nameOrChannels === "object") { - const { start, end, asyncStart, asyncEnd, error } = nameOrChannels; - - assertChannel(start, "nameOrChannels.start"); - assertChannel(end, "nameOrChannels.end"); - assertChannel(asyncStart, "nameOrChannels.asyncStart"); - assertChannel(asyncEnd, "nameOrChannels.asyncEnd"); - assertChannel(error, "nameOrChannels.error"); - - this.start = start; - this.end = end; - this.asyncStart = asyncStart; - this.asyncEnd = asyncEnd; - this.error = error; - } else { - throw $ERR_INVALID_ARG_TYPE("nameOrChannels", ["string, object, or Channel"], nameOrChannels); + for (let i = 0; i < boundedEvents.length; ++i) { + const eventName = boundedEvents[i]; + ObjectDefineProperty(this, eventName, { + __proto__: null, + value: channelFromMap(nameOrChannels, eventName, "BoundedChannel"), + }); } } + get hasSubscribers() { + return this.start?.hasSubscribers || this.end?.hasSubscribers; + } + subscribe(handlers) { - for (const name of traceEvents) { + for (let i = 0; i < boundedEvents.length; ++i) { + const name = boundedEvents[i]; if (!handlers[name]) continue; this[name]?.subscribe(handlers[name]); @@ -292,7 +372,8 @@ class TracingChannel { unsubscribe(handlers) { let done = true; - for (const name of traceEvents) { + for (let i = 0; i < boundedEvents.length; ++i) { + const name = boundedEvents[i]; if (!handlers[name]) continue; if (!this[name]?.unsubscribe(handlers[name])) { @@ -303,6 +384,132 @@ class TracingChannel { return done; } + withScope(context = kEmptyObject) { + return new BoundedChannelScope(this, context); + } + + run(context, fn, thisArg, ...args) { + context ??= {}; + const scope = this.withScope(context); + try { + return fn.$apply(thisArg, args); + } finally { + scope[SymbolDispose](); + } + } +} + +function boundedChannel(nameOrChannels) { + return new BoundedChannel(nameOrChannels); +} + +class TracingChannel { + #callWindow; + #continuationWindow; + + constructor(nameOrChannels) { + if (typeof nameOrChannels === "string") { + this.#callWindow = new BoundedChannel(nameOrChannels); + this.#continuationWindow = new BoundedChannel({ + start: channel(`tracing:${nameOrChannels}:asyncStart`), + end: channel(`tracing:${nameOrChannels}:asyncEnd`), + }); + } else if (typeof nameOrChannels === "object" && nameOrChannels !== null) { + assertChannel(nameOrChannels.start, "nameOrChannels.start"); + assertChannel(nameOrChannels.end, "nameOrChannels.end"); + assertChannel(nameOrChannels.asyncStart, "nameOrChannels.asyncStart"); + assertChannel(nameOrChannels.asyncEnd, "nameOrChannels.asyncEnd"); + + this.#callWindow = new BoundedChannel({ + start: nameOrChannels.start, + end: nameOrChannels.end, + }); + this.#continuationWindow = new BoundedChannel({ + start: nameOrChannels.asyncStart, + end: nameOrChannels.asyncEnd, + }); + } + + ObjectDefineProperty(this, "error", { + __proto__: null, + value: channelFromMap(nameOrChannels, "error", "TracingChannel"), + }); + } + + get start() { + return this.#callWindow.start; + } + + get end() { + return this.#callWindow.end; + } + + get asyncStart() { + return this.#continuationWindow.start; + } + + get asyncEnd() { + return this.#continuationWindow.end; + } + + get hasSubscribers() { + return this.#callWindow.hasSubscribers || this.#continuationWindow.hasSubscribers || this.error?.hasSubscribers; + } + + subscribe(handlers) { + if (handlers.start || handlers.end) { + this.#callWindow.subscribe({ + start: handlers.start, + end: handlers.end, + }); + } + + if (handlers.asyncStart || handlers.asyncEnd) { + this.#continuationWindow.subscribe({ + start: handlers.asyncStart, + end: handlers.asyncEnd, + }); + } + + if (handlers.error) { + this.error.subscribe(handlers.error); + } + } + + unsubscribe(handlers) { + let done = true; + + if (handlers.start || handlers.end) { + if ( + !this.#callWindow.unsubscribe({ + start: handlers.start, + end: handlers.end, + }) + ) { + done = false; + } + } + + if (handlers.asyncStart || handlers.asyncEnd) { + if ( + !this.#continuationWindow.unsubscribe({ + start: handlers.asyncStart, + end: handlers.asyncEnd, + }) + ) { + done = false; + } + } + + if (handlers.error) { + if (!this.error.unsubscribe(handlers.error)) { + done = false; + } + } + + return done; + } + traceSync(fn, context = {}, thisArg, ...args) { const { start, end, error } = this; @@ -410,5 +617,7 @@ export default { subscribe, tracingChannel, unsubscribe, + boundedChannel, Channel, + BoundedChannel, }; diff --git a/test/js/node/diagnostics_channel/diagnostics_channel.test.ts b/test/js/node/diagnostics_channel/diagnostics_channel.test.ts index 37dfd54d7a8f..caa2a73041b2 100644 --- a/test/js/node/diagnostics_channel/diagnostics_channel.test.ts +++ b/test/js/node/diagnostics_channel/diagnostics_channel.test.ts @@ -1,7 +1,7 @@ import { gc } from "bun"; import { beforeEach, describe, expect, mock, test } from "bun:test"; import { AsyncLocalStorage } from "node:async_hooks"; -import { channel, Channel, hasSubscribers, subscribe, unsubscribe } from "node:diagnostics_channel"; +import dc, { channel, Channel, hasSubscribers, subscribe, tracingChannel, unsubscribe } from "node:diagnostics_channel"; describe("Channel", () => { // test-diagnostics-channel-has-subscribers.js @@ -339,10 +339,202 @@ describe("Channel", () => { }); }); +describe("Channel.prototype.withStoreScope", () => { + test("is a function on both inactive and active channels", () => { + const ch = channel("withStoreScope-shape"); + expect(typeof ch.withStoreScope).toBe("function"); + + const disposable = ch.withStoreScope({}); + expect(typeof disposable[Symbol.dispose]).toBe("function"); + disposable[Symbol.dispose](); + + ch.subscribe(() => {}); + expect(typeof ch.withStoreScope).toBe("function"); + }); + + test("enters bound stores for the duration of the scope", () => { + const ch = channel("withStoreScope-stores"); + const store = new AsyncLocalStorage(); + const data = { hello: "world" }; + let published: unknown; + + ch.bindStore(store, d => ({ wrapped: d })); + ch.subscribe(msg => { + published = msg; + }); + + expect(store.getStore()).toBeUndefined(); + { + using scope = ch.withStoreScope(data); + void scope; + expect(store.getStore()).toEqual({ wrapped: data }); + expect(published).toBe(data); + } + expect(store.getStore()).toBeUndefined(); + }); +}); + +describe("BoundedChannel", () => { + test("boundedChannel and BoundedChannel are exported", () => { + expect(typeof dc.boundedChannel).toBe("function"); + expect(typeof dc.BoundedChannel).toBe("function"); + expect(dc.boundedChannel("bc-export")).toBeInstanceOf(dc.BoundedChannel); + }); + + test("creates start/end channels from a name", () => { + const bc = dc.boundedChannel("bc-basic"); + + expect(bc.start.name).toBe("tracing:bc-basic:start"); + expect(bc.end.name).toBe("tracing:bc-basic:end"); + expect(bc.hasSubscribers).toBeFalse(); + expect(typeof bc.subscribe).toBe("function"); + expect(typeof bc.unsubscribe).toBe("function"); + expect(typeof bc.run).toBe("function"); + expect(typeof bc.withScope).toBe("function"); + }); + + test("start/end are non-enumerable own properties", () => { + const bc = dc.boundedChannel("bc-shape"); + expect(Object.keys(bc)).toEqual([]); + expect(Object.getOwnPropertyDescriptor(bc, "start")).toMatchObject({ + enumerable: false, + configurable: false, + writable: false, + }); + }); + + test("accepts explicit channel objects", () => { + const start = channel("bc-custom:start"); + const end = channel("bc-custom:end"); + const bc = dc.boundedChannel({ start, end }); + + expect(bc.start).toBe(start); + expect(bc.end).toBe(end); + }); + + test("subscribe/unsubscribe wires start and end handlers", () => { + const bc = dc.boundedChannel("bc-subscribe"); + const events: Array<{ type: string; message: unknown }> = []; + + const handlers = { + start(message: unknown) { + events.push({ type: "start", message }); + }, + end(message: unknown) { + events.push({ type: "end", message }); + }, + }; + + expect(bc.hasSubscribers).toBeFalse(); + bc.subscribe(handlers); + expect(bc.hasSubscribers).toBeTrue(); + + bc.start.publish({ v: 1 }); + bc.end.publish({ v: 2 }); + + expect(events).toEqual([ + { type: "start", message: { v: 1 } }, + { type: "end", message: { v: 2 } }, + ]); + + expect(bc.unsubscribe(handlers)).toBeTrue(); + expect(bc.hasSubscribers).toBeFalse(); + expect(bc.unsubscribe(handlers)).toBeFalse(); + }); + + test("run publishes start, invokes fn, then publishes end", () => { + const bc = dc.boundedChannel("bc-run"); + const events: string[] = []; + bc.subscribe({ + start: () => events.push("start"), + end: () => events.push("end"), + }); + + const thisArg = { tag: "this" } as const; + const result = bc.run( + { ctx: true }, + function (this: unknown, a: number, b: number) { + events.push("fn"); + expect(this).toBe(thisArg); + return a + b; + }, + thisArg, + 2, + 3, + ); + + expect(result).toBe(5); + expect(events).toEqual(["start", "fn", "end"]); + }); + + test("withScope is a no-op disposable when there are no subscribers", () => { + const bc = dc.boundedChannel("bc-noop"); + const scope = bc.withScope({}); + expect(typeof scope[Symbol.dispose]).toBe("function"); + scope[Symbol.dispose](); + }); +}); + describe("TracingChannel", () => { // Port tests from: // https://github.com/search?q=repo%3Anodejs%2Fnode+test-diagnostics-channel+AND+%2Ftracing%2F&type=code test.todo("TODO"); + + test("has no own enumerable properties", () => { + const tc = tracingChannel("tc-shape"); + expect(Object.keys(tc)).toEqual([]); + expect({ ...tc }).toEqual({}); + }); + + test("exposes start/end/asyncStart/asyncEnd as prototype accessors", () => { + const tc = tracingChannel("tc-accessors"); + const proto = Object.getPrototypeOf(tc); + + for (const name of ["start", "end", "asyncStart", "asyncEnd"] as const) { + expect(Object.getOwnPropertyDescriptor(tc, name)).toBeUndefined(); + const desc = Object.getOwnPropertyDescriptor(proto, name); + expect(desc?.get).toBeFunction(); + } + + expect(Object.getOwnPropertyDescriptor(tc, "error")).toMatchObject({ + enumerable: false, + }); + + expect(tc.start.name).toBe("tracing:tc-accessors:start"); + expect(tc.end.name).toBe("tracing:tc-accessors:end"); + expect(tc.asyncStart.name).toBe("tracing:tc-accessors:asyncStart"); + expect(tc.asyncEnd.name).toBe("tracing:tc-accessors:asyncEnd"); + expect(tc.error.name).toBe("tracing:tc-accessors:error"); + }); + + test("hasSubscribers reflects any of the five channels", () => { + const tc = tracingChannel("tc-hassubs"); + expect(tc.hasSubscribers).toBeFalse(); + + const sub = () => {}; + tc.asyncEnd.subscribe(sub); + expect(tc.hasSubscribers).toBeTrue(); + tc.asyncEnd.unsubscribe(sub); + expect(tc.hasSubscribers).toBeFalse(); + }); + + test("constructed from explicit channel objects", () => { + const chans = { + start: channel("tc-obj:start"), + end: channel("tc-obj:end"), + asyncStart: channel("tc-obj:asyncStart"), + asyncEnd: channel("tc-obj:asyncEnd"), + error: channel("tc-obj:error"), + }; + const tc = tracingChannel(chans); + + expect(tc.start).toBe(chans.start); + expect(tc.end).toBe(chans.end); + expect(tc.asyncStart).toBe(chans.asyncStart); + expect(tc.asyncEnd).toBe(chans.asyncEnd); + expect(tc.error).toBe(chans.error); + expect(Object.keys(tc)).toEqual([]); + }); }); const mocks = new Map(); diff --git a/test/js/node/test/parallel/test-diagnostics-channel-bounded-channel.js b/test/js/node/test/parallel/test-diagnostics-channel-bounded-channel.js new file mode 100644 index 000000000000..90db374a4bf7 --- /dev/null +++ b/test/js/node/test/parallel/test-diagnostics-channel-bounded-channel.js @@ -0,0 +1,105 @@ +'use strict'; +require('../common'); +const assert = require('node:assert'); +const dc = require('node:diagnostics_channel'); + +// Test BoundedChannel exports +{ + assert.strictEqual(typeof dc.boundedChannel, 'function'); + assert.strictEqual(typeof dc.BoundedChannel, 'function'); + + const wc = dc.boundedChannel('test-export'); + assert.ok(wc instanceof dc.BoundedChannel); +} + +// Test basic BoundedChannel creation and properties +{ + const boundedChannel = dc.boundedChannel('test-window-basic'); + + assert.ok(boundedChannel.start); + assert.ok(boundedChannel.end); + + assert.strictEqual(boundedChannel.start.name, 'tracing:test-window-basic:start'); + assert.strictEqual(boundedChannel.end.name, 'tracing:test-window-basic:end'); + + assert.strictEqual(boundedChannel.hasSubscribers, false); + + assert.strictEqual(typeof boundedChannel.subscribe, 'function'); + assert.strictEqual(typeof boundedChannel.unsubscribe, 'function'); + assert.strictEqual(typeof boundedChannel.run, 'function'); + assert.strictEqual(typeof boundedChannel.withScope, 'function'); +} + +// Test BoundedChannel with channel objects +{ + const startChannel = dc.channel('custom:start'); + const endChannel = dc.channel('custom:end'); + + const boundedChannel = dc.boundedChannel({ + start: startChannel, + end: endChannel, + }); + + assert.strictEqual(boundedChannel.start, startChannel); + assert.strictEqual(boundedChannel.end, endChannel); +} + +// Test subscribe/unsubscribe +{ + const boundedChannel = dc.boundedChannel('test-window-subscribe'); + const events = []; + + const handlers = { + start(message) { + events.push({ type: 'start', message }); + }, + end(message) { + events.push({ type: 'end', message }); + }, + }; + + assert.strictEqual(boundedChannel.hasSubscribers, false); + + boundedChannel.subscribe(handlers); + + assert.strictEqual(boundedChannel.hasSubscribers, true); + + // Test that events are received + boundedChannel.start.publish({ test: 'start' }); + boundedChannel.end.publish({ test: 'end' }); + + assert.strictEqual(events.length, 2); + assert.strictEqual(events[0].type, 'start'); + assert.strictEqual(events[0].message.test, 'start'); + assert.strictEqual(events[1].type, 'end'); + assert.strictEqual(events[1].message.test, 'end'); + + // Test unsubscribe + const result = boundedChannel.unsubscribe(handlers); + assert.strictEqual(result, true); + assert.strictEqual(boundedChannel.hasSubscribers, false); + + // Test unsubscribe when not subscribed + const result2 = boundedChannel.unsubscribe(handlers); + assert.strictEqual(result2, false); +} + +// Test partial subscription +{ + const boundedChannel = dc.boundedChannel('test-window-partial'); + const events = []; + + boundedChannel.subscribe({ + start(message) { + events.push('start'); + }, + }); + + assert.strictEqual(boundedChannel.hasSubscribers, true); + + boundedChannel.start.publish({}); + boundedChannel.end.publish({}); + + assert.strictEqual(events.length, 1); + assert.strictEqual(events[0], 'start'); +} From 2319742bf575dae2058ef25f8792484e612d5f07 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 18 Jul 2026 09:40:18 +0000 Subject: [PATCH 2/4] lint: hoist repeated property reads into locals --- src/js/node/diagnostics_channel.ts | 79 ++++++++++++------------------ 1 file changed, 32 insertions(+), 47 deletions(-) diff --git a/src/js/node/diagnostics_channel.ts b/src/js/node/diagnostics_channel.ts index 26a52c64c5c2..f15ed6805c99 100644 --- a/src/js/node/diagnostics_channel.ts +++ b/src/js/node/diagnostics_channel.ts @@ -102,8 +102,9 @@ class RunStoresScope { let taken = false; try { - if (activeChannel._stores) { - for (const entry of activeChannel._stores.entries()) { + const stores = activeChannel._stores; + if (stores) { + for (const entry of stores.entries()) { const store = entry[0]; const transform = entry[1]; @@ -363,9 +364,10 @@ class BoundedChannel { subscribe(handlers) { for (let i = 0; i < boundedEvents.length; ++i) { const name = boundedEvents[i]; - if (!handlers[name]) continue; + const handler = handlers[name]; + if (!handler) continue; - this[name]?.subscribe(handlers[name]); + this[name]?.subscribe(handler); } } @@ -374,9 +376,10 @@ class BoundedChannel { for (let i = 0; i < boundedEvents.length; ++i) { const name = boundedEvents[i]; - if (!handlers[name]) continue; + const handler = handlers[name]; + if (!handler) continue; - if (!this[name]?.unsubscribe(handlers[name])) { + if (!this[name]?.unsubscribe(handler)) { done = false; } } @@ -415,19 +418,14 @@ class TracingChannel { end: channel(`tracing:${nameOrChannels}:asyncEnd`), }); } else if (typeof nameOrChannels === "object" && nameOrChannels !== null) { - assertChannel(nameOrChannels.start, "nameOrChannels.start"); - assertChannel(nameOrChannels.end, "nameOrChannels.end"); - assertChannel(nameOrChannels.asyncStart, "nameOrChannels.asyncStart"); - assertChannel(nameOrChannels.asyncEnd, "nameOrChannels.asyncEnd"); - - this.#callWindow = new BoundedChannel({ - start: nameOrChannels.start, - end: nameOrChannels.end, - }); - this.#continuationWindow = new BoundedChannel({ - start: nameOrChannels.asyncStart, - end: nameOrChannels.asyncEnd, - }); + const { start, end, asyncStart, asyncEnd } = nameOrChannels; + assertChannel(start, "nameOrChannels.start"); + assertChannel(end, "nameOrChannels.end"); + assertChannel(asyncStart, "nameOrChannels.asyncStart"); + assertChannel(asyncEnd, "nameOrChannels.asyncEnd"); + + this.#callWindow = new BoundedChannel({ start, end }); + this.#continuationWindow = new BoundedChannel({ start: asyncStart, end: asyncEnd }); } ObjectDefineProperty(this, "error", { @@ -457,52 +455,39 @@ class TracingChannel { } subscribe(handlers) { - if (handlers.start || handlers.end) { - this.#callWindow.subscribe({ - start: handlers.start, - end: handlers.end, - }); + const { start, end, asyncStart, asyncEnd, error } = handlers; + + if (start || end) { + this.#callWindow.subscribe({ start, end }); } - if (handlers.asyncStart || handlers.asyncEnd) { - this.#continuationWindow.subscribe({ - start: handlers.asyncStart, - end: handlers.asyncEnd, - }); + if (asyncStart || asyncEnd) { + this.#continuationWindow.subscribe({ start: asyncStart, end: asyncEnd }); } - if (handlers.error) { - this.error.subscribe(handlers.error); + if (error) { + this.error.subscribe(error); } } unsubscribe(handlers) { + const { start, end, asyncStart, asyncEnd, error } = handlers; let done = true; - if (handlers.start || handlers.end) { - if ( - !this.#callWindow.unsubscribe({ - start: handlers.start, - end: handlers.end, - }) - ) { + if (start || end) { + if (!this.#callWindow.unsubscribe({ start, end })) { done = false; } } - if (handlers.asyncStart || handlers.asyncEnd) { - if ( - !this.#continuationWindow.unsubscribe({ - start: handlers.asyncStart, - end: handlers.asyncEnd, - }) - ) { + if (asyncStart || asyncEnd) { + if (!this.#continuationWindow.unsubscribe({ start: asyncStart, end: asyncEnd })) { done = false; } } - if (handlers.error) { - if (!this.error.unsubscribe(handlers.error)) { + if (error) { + if (!this.error.unsubscribe(error)) { done = false; } } From eeca4be085dda42c16c56b9263e443c4bac836a9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 18 Jul 2026 09:51:20 +0000 Subject: [PATCH 3/4] review: capture DisposableStack at load, fresh noop disposable, {} default for withScope --- src/js/node/diagnostics_channel.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/js/node/diagnostics_channel.ts b/src/js/node/diagnostics_channel.ts index f15ed6805c99..e8742cb5b8b2 100644 --- a/src/js/node/diagnostics_channel.ts +++ b/src/js/node/diagnostics_channel.ts @@ -2,10 +2,10 @@ // Reference: https://github.com/nodejs/node/blob/v26.3.0/lib/diagnostics_channel.js const { validateFunction } = require("internal/validators"); -const { kEmptyObject } = require("internal/shared"); const SafeMap = Map; const SafeFinalizationRegistry = FinalizationRegistry; +const SafeDisposableStack = DisposableStack; const ArrayPrototypeAt = Array.prototype.at; const ArrayPrototypeIndexOf = Array.prototype.indexOf; @@ -98,7 +98,7 @@ class RunStoresScope { #stack; constructor(activeChannel, data) { - const stack = new DisposableStack(); + const stack = new SafeDisposableStack(); let taken = false; try { @@ -136,8 +136,6 @@ class RunStoresScope { } } -const noopDisposable = { __proto__: null, [SymbolDispose]() {} }; - class ActiveChannel { _subscribers; name; @@ -263,7 +261,7 @@ class Channel { } withStoreScope() { - return noopDisposable; + return { [SymbolDispose]() {} }; } } @@ -387,7 +385,7 @@ class BoundedChannel { return done; } - withScope(context = kEmptyObject) { + withScope(context = {}) { return new BoundedChannelScope(this, context); } From 7da374254d2174b72b95173897646b1037b9f78f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 18 Jul 2026 09:58:21 +0000 Subject: [PATCH 4/4] test: cover BoundedChannel.run error path and all five TracingChannel.hasSubscribers branches --- .../diagnostics_channel.test.ts | 48 +++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/test/js/node/diagnostics_channel/diagnostics_channel.test.ts b/test/js/node/diagnostics_channel/diagnostics_channel.test.ts index caa2a73041b2..6b19519e50bc 100644 --- a/test/js/node/diagnostics_channel/diagnostics_channel.test.ts +++ b/test/js/node/diagnostics_channel/diagnostics_channel.test.ts @@ -467,6 +467,31 @@ describe("BoundedChannel", () => { expect(events).toEqual(["start", "fn", "end"]); }); + test("run still publishes end and restores stores when fn throws", () => { + const bc = dc.boundedChannel("bc-run-throw"); + const store = new AsyncLocalStorage(); + const events: string[] = []; + + bc.start.bindStore(store); + bc.subscribe({ + start: () => events.push("start"), + end: () => events.push("end"), + }); + + const boom = new Error("boom"); + expect(store.getStore()).toBeUndefined(); + expect(() => + bc.run({ ctx: true }, () => { + events.push("fn"); + expect(store.getStore()).toEqual({ ctx: true }); + throw boom; + }), + ).toThrow(boom); + + expect(events).toEqual(["start", "fn", "end"]); + expect(store.getStore()).toBeUndefined(); + }); + test("withScope is a no-op disposable when there are no subscribers", () => { const bc = dc.boundedChannel("bc-noop"); const scope = bc.withScope({}); @@ -507,16 +532,19 @@ describe("TracingChannel", () => { expect(tc.error.name).toBe("tracing:tc-accessors:error"); }); - test("hasSubscribers reflects any of the five channels", () => { - const tc = tracingChannel("tc-hassubs"); - expect(tc.hasSubscribers).toBeFalse(); - - const sub = () => {}; - tc.asyncEnd.subscribe(sub); - expect(tc.hasSubscribers).toBeTrue(); - tc.asyncEnd.unsubscribe(sub); - expect(tc.hasSubscribers).toBeFalse(); - }); + test.each(["start", "end", "asyncStart", "asyncEnd", "error"] as const)( + "hasSubscribers reflects a subscriber on %s", + name => { + const tc = tracingChannel(`tc-hassubs-${name}`); + expect(tc.hasSubscribers).toBeFalse(); + + const sub = () => {}; + tc[name].subscribe(sub); + expect(tc.hasSubscribers).toBeTrue(); + expect(tc[name].unsubscribe(sub)).toBeTrue(); + expect(tc.hasSubscribers).toBeFalse(); + }, + ); test("constructed from explicit channel objects", () => { const chans = {