Skip to content

fix(diagnostics_channel): make channel.hasSubscribers a getter property#6601

Merged
jasnell merged 3 commits intocloudflare:mainfrom
prydt:main
Apr 22, 2026
Merged

fix(diagnostics_channel): make channel.hasSubscribers a getter property#6601
jasnell merged 3 commits intocloudflare:mainfrom
prydt:main

Conversation

@prydt
Copy link
Copy Markdown
Contributor

@prydt prydt commented Apr 17, 2026

Node.js' Channel.hasSubscribers and TracingChannel.hasSubscribers as being boolean getter properties, not functions. In workerd, they are currently implemented as functions which means that if anyone tries to do something like

if(channel.hasSubscribers) // always truthy

https://nodejs.org/dist/latest-v20.x/docs/api/diagnostics_channel.html#channelhassubscribers

This should make workerd's behavior match Node.js for diagnostics_channel. I've added a couple tests here but I'm open to suggestions for ways to improve this if needed.

Here is a worker demonstrating the issue: https://diagnostics-channel-hassubscribers-bug.pranoydll.workers.dev/

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.
@prydt prydt requested review from a team as code owners April 17, 2026 03:56
Copy link
Copy Markdown
Contributor

@guybedford guybedford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for posting the fix.

Copy link
Copy Markdown
Collaborator

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fortunately, this is going to need a compat flag in order to land

@prydt
Copy link
Copy Markdown
Contributor Author

prydt commented Apr 20, 2026

Fortunately, this is going to need a compact flag in order to land

That makes sense. I'll add a compat flag and report back. Thanks!

@prydt
Copy link
Copy Markdown
Contributor Author

prydt commented Apr 20, 2026

Added the compat flag and opened a docs PR. Set a placeholder compat date of 2026-05-01, but that might be too early so feel free to suggest a date you think is more reasonable.

(The add-compat-flag SKILL was extremely useful!)

@prydt prydt requested a review from jasnell April 20, 2026 20:38
Comment thread src/workerd/io/compatibility-date.capnp Outdated
Comment thread src/node/diagnostics_channel.ts Outdated
@prydt prydt requested a review from jasnell April 22, 2026 15:17
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.57%. Comparing base (d1e13ad) to head (975039b).
⚠️ Report is 78 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6601      +/-   ##
==========================================
- Coverage   70.85%   66.57%   -4.29%     
==========================================
  Files         438      405      -33     
  Lines      123644   117527    -6117     
  Branches    19455    19381      -74     
==========================================
- Hits        87608    78239    -9369     
- Misses      24509    27716    +3207     
- Partials    11527    11572      +45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jasnell
Copy link
Copy Markdown
Collaborator

jasnell commented Apr 22, 2026

Manual run of the internal build job is clear. Doc PR opened. Merging.

@jasnell jasnell merged commit 93a6311 into cloudflare:main Apr 22, 2026
24 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants