From edcfa679dcf6c60ad570b419fbd6b41fd9729588 Mon Sep 17 00:00:00 2001 From: Pranoy Dutta Date: Fri, 17 Apr 2026 03:50:38 +0000 Subject: [PATCH 1/3] fix(diagnostics_channel): make hasSubscribers a getter property Node.js' `Channel.hasSubscribers` and `TracingChannel.hasSubscribers` as being boolean getter properties, not functions. https://nodejs.org/dist/latest-v20.x/docs/api/diagnostics_channel.html#channelhassubscribers This is a breaking change to fix this incompatibility with Node. --- src/node/diagnostics_channel.ts | 11 +++++ src/workerd/api/node/diagnostics-channel.h | 2 +- .../node/tests/diagnostics-channel-test.js | 46 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/node/diagnostics_channel.ts b/src/node/diagnostics_channel.ts index 4446e82f572..d34b716abc3 100644 --- a/src/node/diagnostics_channel.ts +++ b/src/node/diagnostics_channel.ts @@ -109,6 +109,17 @@ export class TracingChannel { return this[kError]; } + get hasSubscribers(): boolean { + return ( + this[kStart]?.hasSubscribers || + this[kEnd]?.hasSubscribers || + this[kAsyncStart]?.hasSubscribers || + this[kAsyncEnd]?.hasSubscribers || + this[kError]?.hasSubscribers || + false + ); + } + subscribe(subscriptions: TracingChannelSubscriptions): void { if (subscriptions.start !== undefined) this[kStart]?.subscribe(subscriptions.start); diff --git a/src/workerd/api/node/diagnostics-channel.h b/src/workerd/api/node/diagnostics-channel.h index 8d04628fc44..35f4b9bab15 100644 --- a/src/workerd/api/node/diagnostics-channel.h +++ b/src/workerd/api/node/diagnostics-channel.h @@ -32,7 +32,7 @@ class Channel: public jsg::Object { jsg::Arguments args); JSG_RESOURCE_TYPE(Channel) { - JSG_METHOD(hasSubscribers); + JSG_READONLY_PROTOTYPE_PROPERTY(hasSubscribers, hasSubscribers); JSG_METHOD(publish); JSG_METHOD(subscribe); JSG_METHOD(unsubscribe); diff --git a/src/workerd/api/node/tests/diagnostics-channel-test.js b/src/workerd/api/node/tests/diagnostics-channel-test.js index f57712fef8b..36c028ccbbc 100644 --- a/src/workerd/api/node/tests/diagnostics-channel-test.js +++ b/src/workerd/api/node/tests/diagnostics-channel-test.js @@ -178,3 +178,49 @@ export const DiagChannelUnbindDuringRunStores = { strictEqual(transformCallCount, 1); }, }; + +export const test_channel_hasSubscribers_is_a_getter = { + async test() { + const ch = channel('getter-test'); + + // It is a boolean, not a function (matches Node.js). + strictEqual(typeof ch.hasSubscribers, 'boolean'); + strictEqual(ch.hasSubscribers, false); + + const listener = () => {}; + ch.subscribe(listener); + strictEqual(ch.hasSubscribers, true); + + ch.unsubscribe(listener); + strictEqual(ch.hasSubscribers, false); + + // Defined on the prototype as a read-only getter. + const desc = Object.getOwnPropertyDescriptor( + Object.getPrototypeOf(ch), + 'hasSubscribers' + ); + ok(desc); + strictEqual(typeof desc.get, 'function'); + strictEqual(desc.set, undefined); + strictEqual(desc.value, undefined); + }, +}; + +export const test_tracingChannel_hasSubscribers_is_a_getter = { + async test() { + const tc = tracingChannel('tracing-getter-test'); + + strictEqual(typeof tc.hasSubscribers, 'boolean'); + strictEqual(tc.hasSubscribers, false); + + const listener = () => {}; + + // Each sub-channel independently flips the aggregate getter. + for (const sub of ['start', 'end', 'asyncStart', 'asyncEnd', 'error']) { + tc[sub].subscribe(listener); + strictEqual(tc.hasSubscribers, true, `via ${sub}`); + tc[sub].unsubscribe(listener); + strictEqual(tc.hasSubscribers, false, `after unsubscribing ${sub}`); + } + }, +}; From ec915b22e46da4e46a076df04bc6b16a4b63cab7 Mon Sep 17 00:00:00 2001 From: Pranoy Dutta Date: Mon, 20 Apr 2026 15:24:30 -0500 Subject: [PATCH 2/3] Add compat flag diagnosticsChannelHasSubscribersGetter --- src/node/diagnostics_channel.ts | 52 ++++++++++++-- src/workerd/api/node/diagnostics-channel.h | 9 ++- src/workerd/api/node/tests/BUILD.bazel | 6 ++ .../tests/diagnostics-channel-getter-test.js | 67 +++++++++++++++++++ .../diagnostics-channel-getter-test.wd-test | 18 +++++ .../node/tests/diagnostics-channel-test.js | 39 ++++++----- .../tests/diagnostics-channel-test.wd-test | 10 ++- src/workerd/io/compatibility-date.capnp | 16 ++++- 8 files changed, 188 insertions(+), 29 deletions(-) create mode 100644 src/workerd/api/node/tests/diagnostics-channel-getter-test.js create mode 100644 src/workerd/api/node/tests/diagnostics-channel-getter-test.wd-test diff --git a/src/node/diagnostics_channel.ts b/src/node/diagnostics_channel.ts index d34b716abc3..ffb4fb72a30 100644 --- a/src/node/diagnostics_channel.ts +++ b/src/node/diagnostics_channel.ts @@ -34,6 +34,9 @@ import { ERR_INVALID_ARG_TYPE } from 'node-internal:internal_errors'; import { validateObject } from 'node-internal:validators'; +const hasSubscribersGetter = + !!Cloudflare.compatibilityFlags['diagnostics_channel_has_subscribers_getter']; + export const { Channel } = diagnosticsChannel; export function hasSubscribers(name: string | symbol): boolean { @@ -109,14 +112,27 @@ export class TracingChannel { return this[kError]; } - get hasSubscribers(): boolean { + // `hasSubscribers` is installed on the prototype below, gated by the + // `diagnostics_channel_has_subscribers_getter` compatibility flag. When the + // flag is enabled it's a getter property (matching Node.js); when disabled + // it's a method (preserving the original workerd behavior). The underlying + // implementation lives in `_hasSubscribers` and branches on the flag because + // the inner `Channel.hasSubscribers` follows the same flag. + _hasSubscribers(): boolean { + const channelHas = (c?: ChannelType): boolean => { + if (c == null) return false; + // When the flag is on, `hasSubscribers` is a boolean getter on the + // inner Channel; otherwise it's a method that must be invoked. + return hasSubscribersGetter + ? (c.hasSubscribers as unknown as boolean) + : (c.hasSubscribers as unknown as () => boolean).call(c); + }; return ( - this[kStart]?.hasSubscribers || - this[kEnd]?.hasSubscribers || - this[kAsyncStart]?.hasSubscribers || - this[kAsyncEnd]?.hasSubscribers || - this[kError]?.hasSubscribers || - false + channelHas(this[kStart]) || + channelHas(this[kEnd]) || + channelHas(this[kAsyncStart]) || + channelHas(this[kAsyncEnd]) || + channelHas(this[kError]) ); } @@ -276,6 +292,28 @@ export class TracingChannel { } } +// Install `hasSubscribers` on TracingChannel.prototype gated by the compat +// flag: getter when enabled (Node.js-compatible), method when disabled +// (preserves legacy workerd behavior). +if (hasSubscribersGetter) { + Object.defineProperty(TracingChannel.prototype, 'hasSubscribers', { + get(this: TracingChannel): boolean { + return this._hasSubscribers(); + }, + configurable: true, + enumerable: false, + }); +} else { + Object.defineProperty(TracingChannel.prototype, 'hasSubscribers', { + value: function hasSubscribers(this: TracingChannel): boolean { + return this._hasSubscribers(); + }, + writable: true, + configurable: true, + enumerable: false, + }); +} + function validateChannel(channel: unknown, name: string): ChannelType { if (!(channel instanceof Channel)) { throw new ERR_INVALID_ARG_TYPE(name, 'Channel', channel); diff --git a/src/workerd/api/node/diagnostics-channel.h b/src/workerd/api/node/diagnostics-channel.h index 35f4b9bab15..a6efec4bbfa 100644 --- a/src/workerd/api/node/diagnostics-channel.h +++ b/src/workerd/api/node/diagnostics-channel.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -31,8 +32,12 @@ class Channel: public jsg::Object { jsg::Optional> maybeReceiver, jsg::Arguments args); - JSG_RESOURCE_TYPE(Channel) { - JSG_READONLY_PROTOTYPE_PROPERTY(hasSubscribers, hasSubscribers); + JSG_RESOURCE_TYPE(Channel, CompatibilityFlags::Reader flags) { + if (flags.getDiagnosticsChannelHasSubscribersGetter()) { + JSG_READONLY_PROTOTYPE_PROPERTY(hasSubscribers, hasSubscribers); + } else { + JSG_METHOD(hasSubscribers); + } JSG_METHOD(publish); JSG_METHOD(subscribe); JSG_METHOD(unsubscribe); diff --git a/src/workerd/api/node/tests/BUILD.bazel b/src/workerd/api/node/tests/BUILD.bazel index 63f002d02aa..a2e66e18718 100644 --- a/src/workerd/api/node/tests/BUILD.bazel +++ b/src/workerd/api/node/tests/BUILD.bazel @@ -148,6 +148,12 @@ wd_test( data = ["diagnostics-channel-test.js"], ) +wd_test( + src = "diagnostics-channel-getter-test.wd-test", + args = ["--experimental"], + data = ["diagnostics-channel-getter-test.js"], +) + wd_test( src = "mimetype-test.wd-test", args = ["--experimental"], diff --git a/src/workerd/api/node/tests/diagnostics-channel-getter-test.js b/src/workerd/api/node/tests/diagnostics-channel-getter-test.js new file mode 100644 index 00000000000..8176125405a --- /dev/null +++ b/src/workerd/api/node/tests/diagnostics-channel-getter-test.js @@ -0,0 +1,67 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 +// +// Tests the `hasSubscribers` getter behavior of `Channel` and `TracingChannel` +// when the `diagnostics_channel_has_subscribers_getter` compat flag is enabled. +// Legacy method behavior is covered by `diagnostics-channel-test`. + +import { ok, strictEqual } from 'node:assert'; + +import { channel, tracingChannel } from 'node:diagnostics_channel'; + +export const test_channel_hasSubscribers_is_a_getter = { + async test() { + const ch = channel('getter-test'); + + // It is a boolean, not a function (matches Node.js). + strictEqual(typeof ch.hasSubscribers, 'boolean'); + strictEqual(ch.hasSubscribers, false); + + const listener = () => {}; + ch.subscribe(listener); + strictEqual(ch.hasSubscribers, true); + + ch.unsubscribe(listener); + strictEqual(ch.hasSubscribers, false); + + // Defined on the prototype as a read-only getter. + const desc = Object.getOwnPropertyDescriptor( + Object.getPrototypeOf(ch), + 'hasSubscribers' + ); + ok(desc); + strictEqual(typeof desc.get, 'function'); + strictEqual(desc.set, undefined); + strictEqual(desc.value, undefined); + }, +}; + +export const test_tracingChannel_hasSubscribers_is_a_getter = { + async test() { + const tc = tracingChannel('tracing-getter-test'); + + strictEqual(typeof tc.hasSubscribers, 'boolean'); + strictEqual(tc.hasSubscribers, false); + + const listener = () => {}; + + // Each sub-channel independently flips the aggregate getter. + for (const sub of ['start', 'end', 'asyncStart', 'asyncEnd', 'error']) { + tc[sub].subscribe(listener); + strictEqual(tc.hasSubscribers, true, `via ${sub}`); + tc[sub].unsubscribe(listener); + strictEqual(tc.hasSubscribers, false, `after unsubscribing ${sub}`); + } + + // TracingChannel.hasSubscribers should also be a prototype getter. + const desc = Object.getOwnPropertyDescriptor( + Object.getPrototypeOf(tc), + 'hasSubscribers' + ); + ok(desc); + strictEqual(typeof desc.get, 'function'); + strictEqual(desc.set, undefined); + strictEqual(desc.value, undefined); + }, +}; diff --git a/src/workerd/api/node/tests/diagnostics-channel-getter-test.wd-test b/src/workerd/api/node/tests/diagnostics-channel-getter-test.wd-test new file mode 100644 index 00000000000..004e8625cf0 --- /dev/null +++ b/src/workerd/api/node/tests/diagnostics-channel-getter-test.wd-test @@ -0,0 +1,18 @@ +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [ + ( name = "diagnostics-channel-getter-test", + worker = ( + modules = [ + (name = "worker", esModule = embed "diagnostics-channel-getter-test.js") + ], + compatibilityFlags = [ + "nodejs_compat", + "nodejs_compat_v2", + "diagnostics_channel_has_subscribers_getter", + ] + ) + ), + ], +); diff --git a/src/workerd/api/node/tests/diagnostics-channel-test.js b/src/workerd/api/node/tests/diagnostics-channel-test.js index 36c028ccbbc..4586b08c88d 100644 --- a/src/workerd/api/node/tests/diagnostics-channel-test.js +++ b/src/workerd/api/node/tests/diagnostics-channel-test.js @@ -179,48 +179,53 @@ export const DiagChannelUnbindDuringRunStores = { }, }; -export const test_channel_hasSubscribers_is_a_getter = { +// Legacy-method behavior: when the `diagnostics_channel_has_subscribers_getter` +// compat flag is NOT enabled, `Channel.hasSubscribers` and +// `TracingChannel.hasSubscribers` are registered as methods. The associated +// `.wd-test` opts out of the flag explicitly so that this behavior is exercised +// under both the default and `@all-compat-flags` test variants. +export const test_channel_hasSubscribers_is_a_method = { async test() { - const ch = channel('getter-test'); + const ch = channel('method-test'); - // It is a boolean, not a function (matches Node.js). - strictEqual(typeof ch.hasSubscribers, 'boolean'); - strictEqual(ch.hasSubscribers, false); + // It is a method, not a boolean property. + strictEqual(typeof ch.hasSubscribers, 'function'); + strictEqual(ch.hasSubscribers(), false); const listener = () => {}; ch.subscribe(listener); - strictEqual(ch.hasSubscribers, true); + strictEqual(ch.hasSubscribers(), true); ch.unsubscribe(listener); - strictEqual(ch.hasSubscribers, false); + strictEqual(ch.hasSubscribers(), false); - // Defined on the prototype as a read-only getter. + // Defined on the prototype as a method (value descriptor, no getter). const desc = Object.getOwnPropertyDescriptor( Object.getPrototypeOf(ch), 'hasSubscribers' ); ok(desc); - strictEqual(typeof desc.get, 'function'); + strictEqual(typeof desc.value, 'function'); + strictEqual(desc.get, undefined); strictEqual(desc.set, undefined); - strictEqual(desc.value, undefined); }, }; -export const test_tracingChannel_hasSubscribers_is_a_getter = { +export const test_tracingChannel_hasSubscribers_is_a_method = { async test() { - const tc = tracingChannel('tracing-getter-test'); + const tc = tracingChannel('tracing-method-test'); - strictEqual(typeof tc.hasSubscribers, 'boolean'); - strictEqual(tc.hasSubscribers, false); + strictEqual(typeof tc.hasSubscribers, 'function'); + strictEqual(tc.hasSubscribers(), false); const listener = () => {}; - // Each sub-channel independently flips the aggregate getter. + // Each sub-channel independently flips the aggregate method result. for (const sub of ['start', 'end', 'asyncStart', 'asyncEnd', 'error']) { tc[sub].subscribe(listener); - strictEqual(tc.hasSubscribers, true, `via ${sub}`); + strictEqual(tc.hasSubscribers(), true, `via ${sub}`); tc[sub].unsubscribe(listener); - strictEqual(tc.hasSubscribers, false, `after unsubscribing ${sub}`); + strictEqual(tc.hasSubscribers(), false, `after unsubscribing ${sub}`); } }, }; diff --git a/src/workerd/api/node/tests/diagnostics-channel-test.wd-test b/src/workerd/api/node/tests/diagnostics-channel-test.wd-test index e1c18ae9484..7a9b756f1d0 100644 --- a/src/workerd/api/node/tests/diagnostics-channel-test.wd-test +++ b/src/workerd/api/node/tests/diagnostics-channel-test.wd-test @@ -7,7 +7,15 @@ const unitTests :Workerd.Config = ( modules = [ (name = "worker", esModule = embed "diagnostics-channel-test.js") ], - compatibilityFlags = ["nodejs_compat", "nodejs_compat_v2"] + compatibilityFlags = [ + "nodejs_compat", + "nodejs_compat_v2", + # Explicitly opt out of the `hasSubscribers` getter so this test file + # always exercises the legacy method behavior, even under + # `@all-compat-flags`. The getter behavior is covered by + # `diagnostics-channel-getter-test`. + "no_diagnostics_channel_has_subscribers_getter", + ] ) ), ], diff --git a/src/workerd/io/compatibility-date.capnp b/src/workerd/io/compatibility-date.capnp index d3998c0794b..34fa070ff3b 100644 --- a/src/workerd/io/compatibility-date.capnp +++ b/src/workerd/io/compatibility-date.capnp @@ -1504,8 +1504,20 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef { # `ctx.version` is much more well defined. The behaviour of this flag will change in the future. noResizableArrayBufferInBlob @173 :Bool - $compatEnableFlag("no_resizable_array_buffer_in_blob") - $compatDisableFlag("resizable_array_buffer_in_blob"); + $compatEnableFlag("no_resizable_array_buffer_in_blob") + $compatDisableFlag("resizable_array_buffer_in_blob"); # When enabled, creating a Blob with a resizable ArrayBuffer will throw a TypeError, matching # expected spec behavior. + + diagnosticsChannelHasSubscribersGetter @174 :Bool + $compatEnableFlag("diagnostics_channel_has_subscribers_getter") + $compatDisableFlag("no_diagnostics_channel_has_subscribers_getter") + $compatEnableDate("2026-05-01"); + # Node.js' `diagnostics_channel.Channel.hasSubscribers` and + # `TracingChannel.hasSubscribers` are boolean getter properties, not methods. + # Originally, workerd registered `Channel.hasSubscribers` as a method (so users + # had to call `ch.hasSubscribers()` with parentheses). When this flag is + # enabled, `hasSubscribers` becomes a read-only getter that evaluates directly + # to a boolean, matching Node.js behavior. + # See: https://nodejs.org/dist/latest-v20.x/docs/api/diagnostics_channel.html#channelhassubscribers } From 975039bb78ac42c4184b9650525cc0fdaeeed864 Mon Sep 17 00:00:00 2001 From: Pranoy Dutta Date: Tue, 21 Apr 2026 16:39:00 -0500 Subject: [PATCH 3/3] Address comments. change compat date, inline local function --- src/node/diagnostics_channel.ts | 77 ++++++++++++------------- src/workerd/io/compatibility-date.capnp | 2 +- 2 files changed, 37 insertions(+), 42 deletions(-) diff --git a/src/node/diagnostics_channel.ts b/src/node/diagnostics_channel.ts index ffb4fb72a30..d2de5e32103 100644 --- a/src/node/diagnostics_channel.ts +++ b/src/node/diagnostics_channel.ts @@ -115,25 +115,42 @@ export class TracingChannel { // `hasSubscribers` is installed on the prototype below, gated by the // `diagnostics_channel_has_subscribers_getter` compatibility flag. When the // flag is enabled it's a getter property (matching Node.js); when disabled - // it's a method (preserving the original workerd behavior). The underlying - // implementation lives in `_hasSubscribers` and branches on the flag because - // the inner `Channel.hasSubscribers` follows the same flag. - _hasSubscribers(): boolean { - const channelHas = (c?: ChannelType): boolean => { - if (c == null) return false; - // When the flag is on, `hasSubscribers` is a boolean getter on the - // inner Channel; otherwise it's a method that must be invoked. - return hasSubscribersGetter - ? (c.hasSubscribers as unknown as boolean) - : (c.hasSubscribers as unknown as () => boolean).call(c); - }; - return ( - channelHas(this[kStart]) || - channelHas(this[kEnd]) || - channelHas(this[kAsyncStart]) || - channelHas(this[kAsyncEnd]) || - channelHas(this[kError]) - ); + // it's a method (preserving the original workerd behavior). + static { + if (hasSubscribersGetter) { + Object.defineProperty(this.prototype, 'hasSubscribers', { + get(this: TracingChannel): boolean { + return [ + this.start, + this.end, + this.asyncStart, + this.asyncEnd, + this.error, + ].some((c) => c != null && (c.hasSubscribers as unknown as boolean)); + }, + configurable: true, + enumerable: false, + }); + } else { + Object.defineProperty(this.prototype, 'hasSubscribers', { + value: function hasSubscribers(this: TracingChannel): boolean { + return [ + this.start, + this.end, + this.asyncStart, + this.asyncEnd, + this.error, + ].some( + (c) => + c != null && + (c.hasSubscribers as unknown as () => boolean).call(c) + ); + }, + writable: true, + configurable: true, + enumerable: false, + }); + } } subscribe(subscriptions: TracingChannelSubscriptions): void { @@ -292,28 +309,6 @@ export class TracingChannel { } } -// Install `hasSubscribers` on TracingChannel.prototype gated by the compat -// flag: getter when enabled (Node.js-compatible), method when disabled -// (preserves legacy workerd behavior). -if (hasSubscribersGetter) { - Object.defineProperty(TracingChannel.prototype, 'hasSubscribers', { - get(this: TracingChannel): boolean { - return this._hasSubscribers(); - }, - configurable: true, - enumerable: false, - }); -} else { - Object.defineProperty(TracingChannel.prototype, 'hasSubscribers', { - value: function hasSubscribers(this: TracingChannel): boolean { - return this._hasSubscribers(); - }, - writable: true, - configurable: true, - enumerable: false, - }); -} - function validateChannel(channel: unknown, name: string): ChannelType { if (!(channel instanceof Channel)) { throw new ERR_INVALID_ARG_TYPE(name, 'Channel', channel); diff --git a/src/workerd/io/compatibility-date.capnp b/src/workerd/io/compatibility-date.capnp index 34fa070ff3b..b561ccca968 100644 --- a/src/workerd/io/compatibility-date.capnp +++ b/src/workerd/io/compatibility-date.capnp @@ -1512,7 +1512,7 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef { diagnosticsChannelHasSubscribersGetter @174 :Bool $compatEnableFlag("diagnostics_channel_has_subscribers_getter") $compatDisableFlag("no_diagnostics_channel_has_subscribers_getter") - $compatEnableDate("2026-05-01"); + $compatEnableDate("2026-05-19"); # Node.js' `diagnostics_channel.Channel.hasSubscribers` and # `TracingChannel.hasSubscribers` are boolean getter properties, not methods. # Originally, workerd registered `Channel.hasSubscribers` as a method (so users