node: inspector/debugger v26 compat wave 3 — Network instrumentation, cluster inspect-port, in-process awaitPromise (+5 tests) - #35539
Draft
cirospaciari wants to merge 16 commits into
Conversation
…ndows The urlRegex the probe hands to Debugger.setBreakpointByUrl accepted either separator in its leading directory group but pinned the separators inside the suffix itself to "/", so a multi-segment suffix never matched a script URL spelled with backslashes. Gated to win32 because a backslash is a legal filename character on POSIX. Also drops test-inspector-has-idle.js: it fails on six lanes because the (idle) sample node never materialises outside x64 Linux. The (idle) emission itself is kept.
…low size] `bun inspect` adds a CLI debugger client — REPL, probe mode and CDP client — as ~3k lines of new built-in JS plus NodeInspectEntryPoint.cpp. Built-in JS is bundled into the binary, so the growth is proportional: roughly +580 KB on the linux targets and +240 KB elsewhere. The gate compares against canary main and the base already carries its own allowances, so a stacked branch trips on the sum rather than its own delta.
…encoding - pushStream() encoded array header values with ToString, comma-joining them into a single header field; encode one field per element like the request()/respond() encoders (a pushed 'x-h': ['a','b'] now decodes as 'a, b' on the peer, matching Node). - Http2Stream.close(code) ended the writable side with an empty END_STREAM DATA frame before the RST_STREAM, which is a protocol error on a server stream that never sent response HEADERS; nghttp2 peers (real Node clients) failed the whole session with ERR_HTTP2_ERROR. The stream now records the RST before end() runs so _final skips the wire write, and _destroy no longer sends a second RST_STREAM for a close()d stream. - The http2.client.stream.created diagnostics channel published before endStream closed the writable side; Node publishes after, so subscribers observe the final writableEnded.
Ports lib/internal/inspector/network{,_http,_http2}.js from Node v26.3.0,
subscribed to the diagnostics channels the http and http2 clients already
publish. fetch has no undici channels in Bun, so a delegating wrapper around
globalThis.fetch emits the same events while inspection is active.
Instrumentation is enabled by the first session that enables the Network
domain and disabled when the last one goes away, so the clients pay nothing
otherwise (Node gates the same code behind --experimental-network-inspection).
Network.requestWillBeSent and Network.webSocketCreated now carry the
caller's stack in initiator.stack like Node's NetworkAgent
(captureStackTrace at the emit point, raw script paths for CJS files).
Vendors test-inspector-network-http, test-inspector-network-http2 and
test-inspector-network-content-type (all fail without this).
kill(pid, SIGUSR1) with an errno exception on failure, like Node's node_process_methods.cc DebugProcess. 'bun inspect -p <dead pid>' now reports "Target process: N doesn't exist." instead of trying to connect to port 9229. The Windows variant (thread injection through a named mapping) stays a stub. Vendors test-debugger-pid.
Ports isUsingInspector/getInspectPort from lib/internal/util/inspector.js and enables the inspect-port rewriting in createWorkerProcess that was commented out: when the primary runs with --inspect, each forked worker gets --inspect-port=<debugPort + n> appended to its execArgv, and cluster.settings.inspectPort (number or function) overrides the increment. inspectPort: null throws ERR_SOCKET_BAD_PORT like Node. Vendors test-inspector-port-zero-cluster.
… sample the CPU profile at stop - A Runtime.evaluate with awaitPromise on an in-process session settled its backend reply from a promise reaction, but the channel woke the JS side with a posted event-loop task, which loses the race with process exit when nothing else keeps the loop alive (the resolving timer was the last ref). Wake with a microtask instead, matching when V8 hands Node the reply. - The sampling profiler only wakes once per interval, so a profile stopped shortly after starting contained no samples and Profiler.stop returned an empty profile. Take one synthetic sample of the current JS stack at stop, like V8 records the stop call site. Vendors test-inspector-promises.
…into claude/node-v26-inspector-wave3
…null The $ERR_SOCKET_BAD_PORT helper takes a pre-built message, not (name, port, allowZero); passing the tuple produced the message "Port". Now matches Node: "Port should be >= 0 and < 65536. Received null."
…into claude/node-v26-inspector-wave3
process._debugProcess(process.pid) is the documented way to activate the inspector at runtime, and Node's runtime installs a SIGUSR1 handler at startup, so self-signaling is safe there. Bun installs none, so the kill() terminated the calling process — test-inspector-debug-end's testSessionNoCrash does exactly that and regressed. The complete feature (SIGUSR1 handler + activation + Windows path) is already implemented in the open PR #34106; keep the stub until that lands rather than ship the unsafe half.
…rred RST_STREAM frames
Follow-ups to the close(code) wire fix, found by running the vendored http2
suite against it:
- The frame engine routed every non-CANCEL reset to the stream-error
dispatch, so a peer RST_STREAM(NO_ERROR) destroyed the stream as errored
and 'end' never fired; node/nghttp2 deliver a normal close with rstCode 0.
Now dispatched as the same full close the END_STREAM path uses (the
legacy parser already made this distinction).
- The RST_STREAM that close() defers (setImmediate / 'finish') was written
into the native cork with nothing left to flush it, so the peer could
wait forever; flush at the send site like _final does for END_STREAM.
- streamEnd(7) destroyed a cleanly-closed stream whose readable side had
ended but not yet emitted 'end', losing the 'end' event ('close' fired
with readableEnded=false); defer the destroy until 'end' whenever EOF
has not been delivered, on both the client and server handlers.
Collaborator
|
Updated 8:15 PM PT - Jul 24th, 2026
@cirospaciari, your commit 592c075 is building: |
Contributor
|
Found 4 issues this PR may fix:
🤖 Generated with Claude Code |
Collaborator
|
Updated 6:05 PM PT - Aug 3rd, 2026
❌ @robobun, your commit f8b8f84 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 35539That installs a local version of the PR into your bun-35539 --bun |
…n-check validation for inspector tests - cdp.ts: remove unreachable duplicate setBreakpoint/setBreakpointByUrl cases (merge artifact); hoist remote.objectId and pending.method into locals for the bun(no-duplicate-conditional-property-access) rule - network_http.ts: hoist request.path for the same rule - h2-conformance.test.ts: RST_STREAM(NO_ERROR) is now a clean close so the ERR_HTTP2_STREAM_CANCEL line is no longer emitted - inspector.test.ts: the CDP adapter now labels the first pause after a parked target as "Break on start" - JSC InjectedScript scopeChain/evaluateWithScopeExtension have unchecked throw scopes in the prebuilt WebKit; strip BUN_JSC_validateExceptionChecks from the env the debugger/inspector test helpers hand to their spawned targets, and skip validation for the tests that drive an in-process inspector.Session
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
2 participants
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.
What
Third-wave inspector/debugger compat work on top of #35493 (wave 2), vendoring 5 more upstream Node v26.3.0 tests that now pass. Targets
claude/node-v26-inspector-wave2; the branch also merges the currentclaude/node-v26-combined-34719head (the wave-2 coalesce, the Windows probe-suffix fix, and the test-inspector-has-idle removal), so the effective diff over the coalesced lineage is exactly the commits listed below.Fixes
Each item names the defect; the tests it unlocks are in parentheses.
Network-domain instrumentation for the http/http2/fetch clients
lib/internal/inspector/network{,_http,_http2}.jsfrom Node v26.3.0 asinternal/inspector/*modules, subscribed to the diagnostics channels Bun's_http_clientandhttp2already publish. Bun's fetch is native and publishes no undici channels, so a delegating wrapper aroundglobalThis.fetchemits the same events while inspection is active (same pattern as the module's console hooks). Instrumentation is enabled by the first session that enables the Network domain and disabled with the last — the clients pay nothing otherwise; Node gates the identical code behind--experimental-network-inspection(test-inspector-network-http, test-inspector-network-http2, test-inspector-network-content-type).Network.requestWillBeSent/Network.webSocketCreatednow carry the caller's stack ininitiator.stack, like Node's C++ NetworkAgent (captureStackTraceat the emit point; raw script paths, since V8 reports CJS scripts as absolute paths, not file:// URLs). The capture skips through the emitting helper itself because theguardEventParamswrapper exits via a proper tail call and cannot be used as the skip anchor.http2 wire fixes surfaced by test-inspector-network-http2 against real Node peers
pushStream()encoded array header values with ToString, comma-joining them into one field; now one field per element like the request()/respond() encoders.Http2Stream.close(code)sent an empty END_STREAM DATA frame before the RST_STREAM — a protocol error on a server stream that never sent response HEADERS; a real Node client fails the whole session withERR_HTTP2_ERROR errno -505(verified with a node-client/bun-server pair). close() now records the RST beforeend()runs so_finalskips the wire write,_destroyno longer duplicates the RST_STREAM, and the deferred RST send flushes the native cork (nothing else was left to flush it).RST_STREAM(NO_ERROR)to the stream-error dispatch, destroying the stream as errored with no'end'; node/nghttp2 deliver a normal close with rstCode 0. It now dispatches the same full close as END_STREAM (the legacy parser already made this distinction), andstreamEnd(7)defers the destroy until'end'whenever the readable side has not yet delivered EOF, so'end'precedes'close'like Node. Interop forclose(0)andclose(5)verified in all bun/node client-server pairings.http2.client.stream.createddiagnostics channel published beforeendStreamclosed the writable side; Node publishes after, so subscribers observe the finalwritableEnded.In-process session (node:inspector Session)
Runtime.evaluateawaitPromisereply settles from a promise reaction, but the in-process channel woke the JS side with a posted event-loop task, which loses the race with process exit when the resolving timer was the loop's last ref. The wake is now a microtask, matching when V8 hands Node the reply (test-inspector-promises).Profiler.stopnow takes one synthetic sample of the current JS stack, like V8 records the stop call site (test-inspector-promises).cluster inspect-port allocation — enables the commented-out execArgv rewriting in
createWorkerProcesswith a port ofisUsingInspector/getInspectPort: workers forked by an--inspectprimary get--inspect-port=<debugPort + n>appended,cluster.settings.inspectPort(number or function) overrides the increment, andinspectPort: nullthrowsERR_SOCKET_BAD_PORTwith Node's message (test-inspector-port-zero-cluster).process._debugProcess: attempted, then reverted in-branch — a POSIX
kill(pid, SIGUSR1)implementation madeprocess._debugProcess(process.pid)fatal (Bun installs no SIGUSR1 handler; Node's runtime does), regressing the vendored test-inspector-debug-end. The complete feature (handler + activation + Windows path) is already implemented in open PR #34106, so the stub stays until that lands; test-debugger-pid is not vendored here for the same reason.Vendored tests (5, byte-verbatim)
parallel/test-inspector-network-http.js, parallel/test-inspector-network-http2.js, parallel/test-inspector-network-content-type.js, parallel/test-inspector-promises.js, parallel/test-inspector-port-zero-cluster.js — each fails on the base without the fix in its commit.
Evaluated and not vendored — blocking cause per file
/json/versionidentity: test-inspector.js deepStrictEquals{"Browser":"node.js/v26.3.0","Protocol-Version":"1.1"}; Bun's --inspect endpoint deliberately reports a Bun identity that debug.bun.sh and the VSCode extension key on.Runtime.evaluatetimeoutparam (no evaluate watchdog in JSC): test-inspector-runtime-evaluate-with-timeout.How verified
Every vendored test fails on the unfixed base (baseline run in this worktree) and passes CI-style (
BUN_GARBAGE_COLLECTOR_LEVEL=1etc., bunfig.node-test.toml) on the debug build; preflight (byte-verbatim, per-file pass, throw-canary) is clean. Regression sweeps on the final build: all 258 vendored test-http2-* files, all 89 test-cluster-* + test-diagnostics-channel-* files, and the full vendored test-inspector-/test-debugger- suite. Interop for the http2 close() changes verified against real Node peers in both directions.