Skip to content

fix(ext/node): emit diagnostics_channel events for HTTP server - #33908

Merged
bartlomieju merged 1 commit into
mainfrom
fix/diagnostics-channel-http-server
May 7, 2026
Merged

bartlomieju merged 1 commit into
mainfrom
fix/diagnostics-channel-http-server

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

Summary

  • Emit http.server.request.start, http.server.response.created, and http.server.response.finish diagnostics_channel events from node:http server, matching Node.js behavior.
  • Enable 3 node_compat tests: test-diagnostics-channel-http.js, test-diagnostics-channel-http-server-start.js, test-diagnostic-channel-http-response-created.js

Closes #33905

@fibibot fibibot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Substance walk

Clean PR adding the three Node-spec'd node:diagnostics_channel events for node:http server: http.server.request.start, http.server.response.created, http.server.response.finish. Verified each:

response.created at the ServerResponse constructor (_http_server.js:298): payload { request: req, response: this }. Matches Node's documented payload — the response is this (the new ServerResponse) and request is the constructor arg. ✓

request.start at parserOnIncoming (_http_server.js:838): payload { request: req, response: res, socket, server }. Fires after res.assignSocket(socket) and res.on('finish', resOnFinish) are wired but before the handler runs — matches Node's order. ✓

response.finish at resOnFinish (_http_server.js:905): payload { request: req, response: res, socket, server }. Fires at the top of resOnFinish before the OTel span end / _chunkedEncoding reset — same shape as request.start. ✓

Event order through a single request: response.created (from constructor at line 780) → request.start (line 838) → handler runs → response.finish (when res emits 'finish'). Matches Node's lib/_http_server.js sequence.

hasSubscribers gate: standard perf optimization to skip the payload-object construction when nothing's listening. Channel .publish() with no subscribers is a no-op anyway, so this is purely perf — no behavioral divergence from Node, which doesn't gate.

channel() import: uses the post-#33897 lazy-load form core.loadExtScript("ext:deno_node/diagnostics_channel.js") rather than the old ESM import. Correct shape for the new IIFE-based diagnostics_channel module.

Test enablement

tests/node_compat/config.jsonc enables three tests that exercise these channels — test-diagnostic-channel-http-response-created.js, test-diagnostics-channel-http-server-start.js, test-diagnostics-channel-http.js. Strong regression-test coverage; if any of the channel names/payloads were wrong these would fail.

CI

99 success, 14 in_progress, 1 skip, 0 fail so far. Holding at COMMENT pending CI completion. Substance is APPROVE-quality; will re-confirm with APPROVE once the matrix lands clean.

@bartlomieju
bartlomieju enabled auto-merge (squash) May 7, 2026 16:06
@bartlomieju
bartlomieju merged commit 0f4c9de into main May 7, 2026
136 checks passed
@bartlomieju
bartlomieju deleted the fix/diagnostics-channel-http-server branch May 7, 2026 16:20
@isaacs

isaacs commented May 7, 2026

Copy link
Copy Markdown

Wow, impressive turnaround. Thank you so much! <3

@bartlomieju

Copy link
Copy Markdown
Member Author

We aim to please :)

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.

Emit diagnostics_channel events for node:http server

3 participants