Repository navigation
feat(ext/node): expand diagnostics_channel coverage - #34243
Merged
Merged
Conversation
Implements the diagnostics_channel emissions that previously absent Node
compatibility tests rely on:
* `console.{log,info,debug,warn,error}` publish the args to per-method
channels before formatting (test-console-diagnostics-channels).
* `child_process` channel fires for each new `ChildProcess`, and
`tracingChannel('child_process.spawn')` publishes start/end/error with
Node's "end XOR error" semantics. PermissionDenied now maps to async
EACCES when uid/gid aren't requested, matching Node's execve failure
path (test-diagnostics-channel-{child-process,process}).
* `worker_threads` channel fires per `Worker` construction
(test-diagnostics-channel-worker-threads).
* `tracingChannel('module.require')` wraps `Module.prototype.require`
with parentFilename/id context (test-diagnostics-channel-module-
require[-error]).
* `net.client.socket` moves to `Socket.prototype.connect` so all three
client entry points (`net.connect`, `net.createConnection`, and
`new net.Socket().connect`, including `tls.connect` via TLSSocket)
publish exactly once. `tracingChannel('net.server.listen')` publishes
asyncStart/asyncEnd/error around `Server.prototype.listen`
(test-diagnostics-channel-{net,net-client-socket-tls}).
* `http2.client.stream.bodyChunkSent` fires from
`ClientHttp2Stream._write`/`_writev` with writev/data/encoding,
honoring writable's `allBuffers` flag so single-buffer multi-write
callers see the flat Buffer array. `http2.client.stream.bodySent`
fires from `_final` even when no chunks were sent
(test-diagnostics-channel-http2-client-stream-body-*).
Polyfill fixes:
* `Channel.subscribe`/`unsubscribe` now snapshot the subscriber array
via slice/pushApply, and `publish` captures the local reference, so
unsubscribing mid-publish still delivers to the remaining subscribers
(test-diagnostics-channel-sync-unsubscribe).
* `WeakRefMap` finalizer skips deletion when a fresh `WeakRef` for the
key already has a live value, fixing the GC race that swapped the
cached channel instance out from under a live consumer
(test-diagnostics-channel-gc-race-condition).
* `tracingChannel(0)` now reports `["string", "object",
"TracingChannel"]` so the formatted message matches Node's
(test-diagnostics-channel-tracing-channel-args-types).
Closes denoland/orchid#149
Co-Authored-By: Divy Srivastava <[email protected]>
The `net.server.listen` tracingChannel was publishing asyncEnd/error from `_emitListeningNT`/`_emitErrorNT`, both deferred via `nextTick`. test-diagnostics-channel-net subscribes, calls `server.listen()`, and then immediately unsubscribes -- by the time the deferred emit ran, the channel had no subscribers and the `error.publish` was a no-op, showing up on aarch64 debug CI as `Expected exactly 1, actual 0`. Move the publishes inline into `_setupListenHandle` (success path => asyncEnd; bind / handle.listen failure => error) so subscribers see the result before the synchronous return from `Server.prototype.listen`. Matches Node's `lib/net.js`, which publishes from `setupListenHandle` directly rather than from the `nextTick`ed emitter. Co-Authored-By: Divy Srivastava <[email protected]>
littledivy
approved these changes
May 20, 2026
littledivy
added a commit
to crowlKats/deno
that referenced
this pull request
Jun 10, 2026
## Summary
Implements the `diagnostics_channel` (and `tracingChannel`) emissions
that previously-absent Node compat tests rely on, plus fixes a few
preexisting polyfill bugs.
New channel emissions:
- `console.{log,info,debug,warn,error}` publish args to per-method
channels **before** formatting, so subscribers can mutate the args and
see the mutation reflected in output.
- `child_process` channel fires for each new `ChildProcess` (covers
`cluster.fork`). `tracingChannel('child_process.spawn')` publishes start
/ end / error with Node's "end XOR error" semantics. As part of this
`PermissionDenied` now maps to an asynchronous `EACCES` error event on
the child when uid/gid weren't requested (matching Node's `execve`
failure path); the EPERM-sync-throw path is preserved for uid/gid setup
failures.
- `worker_threads` channel fires per `Worker` construction.
- `tracingChannel('module.require')` wraps `Module.prototype.require`
with `{ parentFilename, id }` context.
- `net.client.socket` moved from the `net.connect()` factory to
`Socket.prototype.connect`, so all three client entry points
(`net.connect`, `net.createConnection`, `new net.Socket().connect`)
publish exactly once. TLS sockets (`tls.connect` via `TLSSocket extends
Socket`) now publish too.
- `tracingChannel('net.server.listen')` publishes `asyncStart` at the
top of `Server.prototype.listen` (with the original options object, so
ad-hoc fields survive), `asyncEnd` on `'listening'`, and `error` on
`'error'`.
- `http2.client.stream.bodyChunkSent` fires from
`ClientHttp2Stream._write` / `_writev` with `{ stream, writev, data,
encoding }`, honoring the writable internals' `allBuffers` flag so
single-buffer writers see a flat Buffer array.
`http2.client.stream.bodySent` fires from `_final`, including when no
chunks were sent.
Polyfill fixes shaken out by the new tests:
- `Channel.subscribe`/`unsubscribe` now snapshot the subscriber array
via `slice` + `pushApply`, and `publish` captures the local array
reference, so unsubscribe-during-publish still delivers to the remaining
subscribers (`test-diagnostics-channel-sync-unsubscribe`).
- `WeakRefMap`'s finalizer skips deletion when a fresh `WeakRef` for the
same key already holds a live value, fixing the GC race where a
re-created channel would be evicted out from under a live caller
(`test-diagnostics-channel-gc-race-condition`).
- `tracingChannel(0)` now reports `["string", "object",
"TracingChannel"]` so the formatted message matches Node's
(`test-diagnostics-channel-tracing-channel-args-types`).
Out of scope (left for follow-ups):
- `tracingChannel('module.import')` for dynamic `import()` — needs a
host-side dynamic-import hook in `deno_core`/`cli`, since
`core.compileFunction` doesn't accept a per-script dynamic-import
callback. Tests: `test-diagnostics-channel-module-import[-error]`.
- `test-console-diagnostics-channels` expects `process.stdout.write` to
be hijackable, which requires routing the global `console` through
Node's `Console` instance in Node-compat mode. The channel publishing is
in place; only the global-console wiring is missing.
- `test-diagnostics-channel-http2-client-stream-{created,start}` exposes
a pre-existing http/2 push ordering issue where push `created`/`start`
can outpace the request stream's publish in certain configurations.
Closes denoland/orchid#149
## Test plan
- `cargo test --test node_compat -- diagnostics-channel` — newly-added
entries pass; existing entries unchanged.
- `tools/format.js`
- `tools/lint.js --js`
- CI: verifies the network-bound tests (this worker VM has seccomp
filters that block listen(), so the http2/net body and listen tests
couldn't be verified locally and are added to config.jsonc to run on
CI).
---------
Co-authored-by: divybot <[email protected]>
Co-authored-by: Divy Srivastava <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the
diagnostics_channel(andtracingChannel) emissions that previously-absent Node compat tests rely on, plus fixes a few preexisting polyfill bugs.New channel emissions:
console.{log,info,debug,warn,error}publish args to per-method channels before formatting, so subscribers can mutate the args and see the mutation reflected in output.child_processchannel fires for each newChildProcess(coverscluster.fork).tracingChannel('child_process.spawn')publishes start / end / error with Node's "end XOR error" semantics. As part of thisPermissionDeniednow maps to an asynchronousEACCESerror event on the child when uid/gid weren't requested (matching Node'sexecvefailure path); the EPERM-sync-throw path is preserved for uid/gid setup failures.worker_threadschannel fires perWorkerconstruction.tracingChannel('module.require')wrapsModule.prototype.requirewith{ parentFilename, id }context.net.client.socketmoved from thenet.connect()factory toSocket.prototype.connect, so all three client entry points (net.connect,net.createConnection,new net.Socket().connect) publish exactly once. TLS sockets (tls.connectviaTLSSocket extends Socket) now publish too.tracingChannel('net.server.listen')publishesasyncStartat the top ofServer.prototype.listen(with the original options object, so ad-hoc fields survive),asyncEndon'listening', anderroron'error'.http2.client.stream.bodyChunkSentfires fromClientHttp2Stream._write/_writevwith{ stream, writev, data, encoding }, honoring the writable internals'allBuffersflag so single-buffer writers see a flat Buffer array.http2.client.stream.bodySentfires from_final, including when no chunks were sent.Polyfill fixes shaken out by the new tests:
Channel.subscribe/unsubscribenow snapshot the subscriber array viaslice+pushApply, andpublishcaptures the local array reference, so unsubscribe-during-publish still delivers to the remaining subscribers (test-diagnostics-channel-sync-unsubscribe).WeakRefMap's finalizer skips deletion when a freshWeakReffor the same key already holds a live value, fixing the GC race where a re-created channel would be evicted out from under a live caller (test-diagnostics-channel-gc-race-condition).tracingChannel(0)now reports["string", "object", "TracingChannel"]so the formatted message matches Node's (test-diagnostics-channel-tracing-channel-args-types).Out of scope (left for follow-ups):
tracingChannel('module.import')for dynamicimport()— needs a host-side dynamic-import hook indeno_core/cli, sincecore.compileFunctiondoesn't accept a per-script dynamic-import callback. Tests:test-diagnostics-channel-module-import[-error].test-console-diagnostics-channelsexpectsprocess.stdout.writeto be hijackable, which requires routing the globalconsolethrough Node'sConsoleinstance in Node-compat mode. The channel publishing is in place; only the global-console wiring is missing.test-diagnostics-channel-http2-client-stream-{created,start}exposes a pre-existing http/2 push ordering issue where pushcreated/startcan outpace the request stream's publish in certain configurations.Closes denoland/orchid#149
Test plan
cargo test --test node_compat -- diagnostics-channel— newly-added entries pass; existing entries unchanged.tools/format.jstools/lint.js --js