Skip to content

fix(ext/node): validate a diagnostics_channel subscription before activation - #35

Merged
jackspirou merged 1 commit into
nimbus/v2.9.7from
fix/diagnostics-channel-validate-before-activate
Sep 24, 2026
Merged

jackspirou merged 1 commit into
nimbus/v2.9.7from
fix/diagnostics-channel-validate-before-activate

Conversation

@jackspirou

Copy link
Copy Markdown
Contributor

Summary

Channel#subscribe() on an inactive channel called markActive() before it validated the subscription. An invalid subscription threw ERR_INVALID_ARG_TYPE, but the channel stayed active. After that, channel.hasSubscribers and diagnostics_channel.hasSubscribers(name) returned true with no subscriber.

Node.js fixed the same bug in nodejs/node#65313 (issue nodejs/node#65312), shipped in v24.21.0 and v26.8.0. The Node.js 24.21+ and 26.8+ versions of test-diagnostics-channel-pub-sub.js and test-diagnostics-channel-object-channel-pub-sub.js assert this behavior.

Change

  • Channel#subscribe() calls validateFunction(subscription, "subscription") before markActive(this).
  • New regression test tests/unit_node/diagnostics_channel_test.ts. It covers dc.subscribe(name, null) and channel.subscribe(null), and then a valid subscribe and unsubscribe.

No Node.js release depends on the old behavior, so this is a plain bug fix with no version policy.

Tests

  • The new test fails before the fix (AssertionError) and passes after it.
  • unit_node::diagnostics_channel_test: pass.
  • node_compat: test-diagnostics-channel-pub-sub.js and test-diagnostics-channel-object-channel-pub-sub.js pass.
  • dprint and tools/lint.js --js: clean.

…ivation

Channel#subscribe() activated the channel before it validated the
subscription. An invalid subscription threw ERR_INVALID_ARG_TYPE, but the
channel stayed active, so hasSubscribers returned true without a
subscriber. Validate first, as nodejs/node#65313 does.
@jackspirou
jackspirou merged commit abe1cd2 into nimbus/v2.9.7 Sep 24, 2026
1 check 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.

1 participant