node: inspector/debugger v26 compat fixes (+17 tests) - #35493
Closed
cirospaciari wants to merge 11 commits into
Closed
node: inspector/debugger v26 compat fixes (+17 tests)#35493cirospaciari wants to merge 11 commits into
cirospaciari wants to merge 11 commits into
Conversation
- Report the pause that ends a --inspect-brk/--inspect-wait park with reason "Break on start" (V8's label; NODE_INSPECT_RESUME_ON_START and the debugger REPL header branch on it). - Match V8's preview shape for collections: omit `entries` when empty, and elide entries on Map/Set previews nested inside another preview, reporting overflow instead. - When an evaluation throws, fetch the exception's own properties and rebuild exceptionDetails the way V8 shapes it: description embedding the formatted stack, a structured stackTrace, text "Uncaught", and the throwing statement's source-mapped position and scriptId. - Re-set pre-parse breakpoints in two phases (all removals, then all re-adds): JSC resolves distinct requests on an unmapped line forward to one shared pause location, and removing one of them after a re-added breakpoint had resolved there cleared that one too.
Node resolves process.debugPort to the actually-bound port once the inspector server is listening; Bun kept returning the 9229 default for a CLI-started server (--inspect=0). Report the resolved port from the debugger thread when the node-CDP endpoint comes up; inspector.open() already assigns it from JS.
…shape over CDP - Translate programmatic ScriptProfiler tracking (console.profile / console.profileEnd with no client Profiler.start) into V8's Profiler.consoleProfileStarted/consoleProfileFinished events. - Implement HeapProfiler.takeHeapSnapshot for remote sessions: build the V8-format snapshot on the inspected thread via Bun.generateHeapSnapshot and stream it back as addHeapSnapshotChunk events. - Drop the JSC-only line/column/sourceURL/originalLine/originalColumn own properties from error previews (V8 errors do not have them) and list stack before message, matching V8's preview order. - Apply the Break-on-start relabel after the reason switch so it wins over a breakpoint on the first statement, as in V8, while an exception still reports as an exception.
A --inspect process could exit before the debugger thread got around to printing the listening banner: the banner and the node-CDP registration run on the debugger thread, and nothing ordered them before the main thread's first statement, so a short script (-p) raced them. Node binds the inspector synchronously at startup, and tools scrape stderr for the banner before reading script output. Arm the existing startup futex in Debugger::create and wait on it after spawning the debugger thread; the debugger thread already clears it on every exit path once its init (server bind, banner, registration) is done. Also print the banner before reportNodeInspectorServerStarted so it is ordered before inspector.open()'s wakeup on that path too.
…-parse breakpoint pauses - Translate Debugger.setBreakpoint's actualLocation and Debugger.setBreakpointByUrl's locations back to original positions -- clients print them (the debugger REPL's `breakpoints` list showed generated line numbers). - A script starts executing as soon as it parses, so a pass-through pre-parse breakpoint could fire before the backend processed the removal issued during its source-map retranslation; V8 never pauses there (it re-resolves internally), so resume through such a pause without reporting it. - When an evaluation returns an error value with a preview, recover the `stack` property JSC's five-property preview cap crowded out, listing it first as V8 does.
Node's --inspect-port=[host:]port names the port an --inspect listener binds and is reflected by process.debugPort even without a listener (--inspect-port=0 alone reports 0). Bun silently ignored the flag and kept reporting the 9229 default. An explicit --inspect=<port> keeps its own port.
…nded off the re-set line JSC resolves a stale pass-through breakpoint forward, sometimes to the very location its re-set lands on; that pause is the one the client asked for and is reported (under the id the client knows), not resumed. Unconditional suppression made probes miss when the pass-through and re-set positions resolved to the same statement.
The probe report's stderr field must carry only the target's own output; Node targets print nothing but the listening lines the filter already drops, while Bun targets also print the Bun Inspector banner.
Byte-verbatim from upstream, plus the fixtures they use (fixtures/debugger/probe-exits-during-probe.js, probe-multi-statement.js, probe-throwing-getter.js). All pass CI-style against the debug build. parallel/test-debugger-auto-resume.mjs parallel/test-debugger-heap-profiler.js parallel/test-debugger-probe-explicit-column.js parallel/test-debugger-probe-expression-throws.js parallel/test-debugger-probe-failure-process-exit.js parallel/test-debugger-probe-json-preview.js parallel/test-debugger-profile.js parallel/test-debugger-sb-before-load.js parallel/test-inspector-emit-protocol-event-errors.js parallel/test-inspector-emit-protocol-event.js parallel/test-inspector-network-arbitrary-data.js parallel/test-inspector-network-data-received.js parallel/test-inspector-network-data-sent.js parallel/test-inspector-port-zero.js parallel/test-inspector-vm-global-accessors-getter-sideeffect.js parallel/test-inspector-vm-global-accessors-sideeffects.js sequential/test-debugger-debug-brk.js
Contributor
|
Found 4 issues this PR may fix:
🤖 Generated with Claude Code |
Collaborator
|
Updated 5:28 PM PT - Jul 24th, 2026
@cirospaciari, your commit f9bddd5 is building: |
Collaborator
|
Updated 5:30 PM PT - Jul 24th, 2026
@autofix-ci[bot], your commit 7aa5bb8 is building: |
Member
Author
|
🤖 Coalesced into #35396 (its direct parent in the inspector lineage, per review-load consolidation) — merged clean, 77-test preflight green. Closing. |
This was referenced Jul 25, 2026
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.
What
Second-wave
node:inspector/node inspectcompat fixes on top of the inspector/CDP-on-CLI stack (#31823 → #34719 → #35396), vendoring 17 more upstream Node v26.3.0test-inspector-*/test-debugger-*tests that now pass.Verified by running every
test-inspector-*/test-debugger-*file in the tree (133 files: the 116 the base already carries plus these 17) CI-style against the debug build: 94 pass, and every failure is in the drop list below with its blocking cause. Preflight (byte-verbatim check, per-file pass, throw-canary) is clean.Fixes
Each item names the defect; the tests it unlocks are in parentheses. Everything else in the diff is vendored test files.
CDP adapter (
src/js/internal/inspector/cdp.ts)--inspect-brk/--inspect-waitpark is now reported with V8's reasonBreak on start;NODE_INSPECT_RESUME_ON_STARTand the debugger REPL header branch on it (test-debugger-auto-resume).entriesomitted from empty Map/Set previews; entries of a Map/Set nested inside another preview elided tooverflow; JSC-onlyline/column/sourceURL/originalLine/originalColumnown properties dropped from error previews withstacklisted first; andstackrecovered viaRuntime.getPropertieswhen JSC's five-property preview cap crowds it out (test-debugger-probe-json-preview).exceptionDetailsnow carriestext: "Uncaught", a description embedding the formatted stack, a structuredstackTrace,scriptId, and the throwing statement's source-mapped position (test-debugger-probe-expression-throws).Debugger.setBreakpoint/setBreakpointByUrlreplies translate their resolved locations back to original coordinates; the REPL'sbreakpointslist printed generated line numbers.console.profile()/console.profileEnd()(JSC's programmatic ScriptProfiler tracking) are announced asProfiler.consoleProfileStarted/consoleProfileFinished(test-debugger-profile).HeapProfiler.takeHeapSnapshotimplemented for remote sessions by building the V8-format snapshot on the inspected thread (Bun.generateHeapSnapshot("v8")) and streamingaddHeapSnapshotChunkevents (test-debugger-heap-profiler).Native
process.debugPortreflects the actually-bound CLI inspector port (was always 9229), reported from the debugger thread when the node-CDP endpoint comes up (test-inspector-port-zero).--inspectstartup blocks until the debugger thread finishes server startup, so theDebugger listening on ws://…banner is ordered before any script output — Node binds its inspector synchronously and this test scrapes stderr for that line before reading stdout. Implemented with the existing startup futex: armed inDebugger::createbefore the spawn, cleared on every debugger-thread exit path (test-inspector-port-zero; a-p/short script could previously exit before the banner).--inspect-port=[host:]portis parsed: it names the port for an active--inspectlistener and is reflected byprocess.debugPorteven without one —--inspect-port=0alone reports 0 and starts no server. An explicit--inspect=<port>keeps its own port (test-inspector-port-zero).bun inspectprobe modeTest harness
test/common/inspector-helper.jssynced verbatim to v26.3.0 — the base carried a pre-waitUntilcopy (test-inspector-network-data-received / data-sent).Vendored tests (17, byte-verbatim)
See the test commit for the list; fixtures
fixtures/debugger/probe-exits-during-probe.js,probe-multi-statement.js,probe-throwing-getter.jscome along.Evaluated and not vendored — blocking cause per file
inspector.open()in workers is the base stack's deliberateERR_WORKER_UNSUPPORTED_OPERATIONstub; NodeWorker attach/sendMessageToWorker need it): test-inspector-close-worker, -exit-worker-in-wait-for-connection, -exit-worker-in-wait-for-connection2, -connect-main-thread, -async-hook-after-done.async_wrap,inspector) and the SIGUSR1/_debugProcessstub decision (the base PR reverted_debugProcessto a stub because Bun has no SIGUSR1 inspector handler): test-inspector-async-call-stack, -async-call-stack-abort, -async-hook-setup-at-signal, test-debugger-pid.exec a={}; athrows where Node's sloppy wrapper creates a global: test-debugger-object-type-remote-object (its Map/Set preview assertions now pass; this is the remaining blocker).error:style, noNode.js vXtrailer): test-debugger-probe-script-throws, -probe-target-syntax-error.bad option: test-debugger-probe-launch.../..; Bun's CI cwd is the bun repo root, which nests these tests deeper): test-debugger-profile-command, test-inspector-overwrite-config.--inspect-waitinitial pause: the firstnsteps to the last top-level statement instead of the next line (the CLI parks with--inspect-wait+Debugger.pause, and stepping from that pause is mis-anchored in JSC): test-debugger-set-context-line-number, -run-after-quit-restart.Cannot access 'x' before initializationvsx is not definedat break-on-start): test-debugger-watchers.Runtime.awaitPromisenever settles (pre-existing in the base stack — the in-process backend channel has no async reply path): test-inspector-promises.--experimental-webstorage/ Storage domain: test-inspector-dom-storage.internal/deps/undicireal internal module: test-inspector-network-fetch.Runtime.evaluatetimeoutparam (JSC has no evaluate watchdog): test-inspector-runtime-evaluate-with-timeout.cluster.settings.inspectPortexecArgv rewriting): test-inspector-port-zero-cluster, sequential/test-inspector-port-cluster.scriptsmust listnode:internal/*scripts Bun does not have: test-debugger-scripts.node:httpnetwork-inspection instrumentation not implemented (only the manualNetwork.*emit API exists): test-inspector-network-content-type, -network-http, -network-http2, -network-websocket, sequential/test-inspector-network-resource.--inspect-brk-node+node:internal/bootstrap/realmscript: test-inspector-inspect-brk-node.bun inspectprobe failure-path fidelity (JSC's "Can only perform operation while paused." vs V8's "Must be paused or waiting to pause"; disconnect-vs-timeout classification when a probe closes the inspector): test-debugger-probe-failure-resume, -probe-failure-hang-during-evaluate.