Skip to content

feat(ext/node): expand diagnostics_channel coverage - #34243

Merged
littledivy merged 2 commits into
mainfrom
orch/divybot-149
May 20, 2026
Merged

littledivy merged 2 commits into
mainfrom
orch/divybot-149

Conversation

@divybot

@divybot divybot commented May 19, 2026 •

Copy link
Copy Markdown
Contributor

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).

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]>
@divybot divybot changed the title node compat: close diagnostics_channel coverage for absent Node tests feat(ext/node): expand diagnostics_channel coverage May 19, 2026
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
littledivy merged commit c39514d into main May 20, 2026
136 checks passed
@littledivy
littledivy deleted the orch/divybot-149 branch May 20, 2026 03:30
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]>
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.

2 participants