Repository navigation
fix(ext/node): emit diagnostics_channel events for HTTP server - #33908
Conversation
fibibot
left a comment
There was a problem hiding this comment.
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.
|
Wow, impressive turnaround. Thank you so much! <3 |
|
We aim to please :) |
Summary
http.server.request.start,http.server.response.created, andhttp.server.response.finishdiagnostics_channel events fromnode:httpserver, matching Node.js behavior.test-diagnostics-channel-http.js,test-diagnostics-channel-http-server-start.js,test-diagnostic-channel-http-response-created.jsCloses #33905