Skip to content

inspector: Node-shaped inspector.console, CLI-started url()/close(), synchronous Session.post (+19 tests) - #35381

Closed
cirospaciari wants to merge 2 commits into
claude/inspector-cdp-on-clifrom
claude/node-inspector-domains
Closed

inspector: Node-shaped inspector.console, CLI-started url()/close(), synchronous Session.post (+19 tests)#35381
cirospaciari wants to merge 2 commits into
claude/inspector-cdp-on-clifrom
claude/node-inspector-domains

Conversation

@cirospaciari

@cirospaciari cirospaciari commented Jul 24, 2026

Copy link
Copy Markdown
Member

Stacked on #34719 — review that first.

What this does

Closes six more upstream node:inspector test files, and vendors thirteen more
that already passed but had never been added.

Six behaviour fixes, each one required by a vendored test:

  • inspector.console reports to the inspector only. It was a copy of the
    global console, so require('inspector').console.log(x) printed x on
    stdout. In Node it is V8's inspector console: the call becomes a
    Runtime.consoleAPICalled notification for attached sessions and nothing is
    written to stdout or stderr. A new native entry point calls the inspector
    controller's own console client, which is the half of Bun::ConsoleObject
    that does not print. The object now carries Node's key set in Node's order.
  • inspector.url(), close() and waitForDebugger() see a CLI-started
    inspector.
    They only knew about a server started by inspector.open(), so
    under --inspect url() returned undefined and open() did not report
    the port as already taken. The debugger thread now reports the CDP endpoint's
    URL back to the inspected thread for --inspect too, over the same control
    channel inspector.open() already used, so close() can stop it. A worker
    still reports no URL: it cannot start an inspector of its own.
  • Session.post matches Node's calling contract. The callback now runs
    synchronously, before post() returns, and posting on a disconnected session
    throws ERR_INSPECTOR_NOT_CONNECTED even when a callback was given. Both were
    checked against node v26.3.0 rather than inferred.
  • CPU profiles account for time spent outside JS. V8 emits (idle) samples
    for wall-clock time the thread was not running JS; JSC's sampling profiler
    records nothing at all then, so the profile claimed a 1-second sleep took
    15 ms. Gaps longer than one extra sampling period are now reported as (idle)
    samples under (root).
  • process.binding('inspector') exposes isEnabled().
  • NodeTracing.getCategories answers with Node's category list.

Plus one smaller one:

  • --debug and --debug-brk are rejected with Node's DEP0062 message and exit
    code 9 instead of being silently ignored. Only for the runtime commands (the
    ones that accept --inspect), and only when the flag precedes the entrypoint,
    so bun --bun node-gyp build --debug still passes it through.

Upstream tests now vendored (19)

Already passing on the base, never vendored (13):
test-inspector-async-stack-traces-promise-then, test-inspector-break-e,
test-inspector-break-when-eval, test-inspector-debug-brk-flag,
test-inspector-debug-end, test-inspector-exception,
test-inspector-heap-allocation-tracker, test-inspector-module,
test-inspector-multisession-ws, test-inspector-stop-profile-after-done,
test-inspector-wait-for-connection, test-inspector-wait.mjs,
test-inspector-waiting-for-disconnect.

Made to pass by this change (6):
test-inspector-already-activated-cli, test-inspector-console,
test-inspector-enabled, test-inspector-has-idle,
test-inspector-invalid-args, test-inspector-tracing-domain.

All nineteen are byte-identical to v26.3.0. test/expectations.txt is
untouched.

Still failing, and why

Of the 45 upstream inspector files not vendored anywhere, 26 remain out:

  • In-process breakpoints on a line with no source-map entry
    (multisession-js, async-context-brk, bindings). In-process pausing works
    — a debugger statement pauses, and a breakpoint on a statement line hits.
    All three of these set the breakpoint on the } that closes a one-line
    function. That original line has no mapping, so the adapter resolves it to the
    next mapped position, which is past the function, and it never fires. V8
    resolves it to the function's exit hook.
  • Worker inspection (close-worker, exit-worker-in-wait-for-connection,
    exit-worker-in-wait-for-connection2, connect-main-thread,
    async-hook-after-done, worker-target). inspector.open() rejects off the
    main thread, and the debugger-thread state is a process-wide singleton with
    one URL and one control callback. Per-worker servers, connectToMainThread,
    and the NodeWorker/Target domains are all downstream of keying that state
    per inspected thread.
  • vm execution contexts (contexts, scriptparsed-context,
    vm-global-accessors-sideeffects, vm-global-accessors-getter-sideeffect).
    JSC's runtime agent has exactly one execution context and no
    throwOnSideEffect. Note the two vm-global-accessors files exit 0 today
    with their assertion swallowed by Node's own Session warning path, so they
    are deliberately not vendored — they would be vacuous passes over wrong
    output.
  • Async stack traces through Node internals
    (async-stack-traces-set-interval, async-hook-setup-at-inspect-brk). Both
    assert an async frame with url === 'node:internal/process/execution'. Bun's
    internals are not Node's module graph.
  • --inspect-brk pauses in the entry module (esm). Bun prepends a
    debugger statement to the entry; Node pauses at the first statement of the
    first evaluated module, which for an ESM entry is a dependency. Matching it
    means pausing via setPauseOnNextStatement instead, which changes every
    --inspect-brk session.
  • Runtime.evaluate timeout (runtime-evaluate-with-timeout). Nothing
    terminates the evaluation, so the test hangs. A fix needs JSC's watchdog
    around the in-process dispatch plus a mapping from the resulting termination
    to a -32000 Execution was terminated reply; the second half is the unknown.
  • /json/version identity (inspector). The test wants exactly
    {"Browser":"node.js/<version>","Protocol-Version":"1.1"}; the base
    deliberately keeps Bun's fields there for --inspect because debug.bun.sh and
    the VSCode extension identify a Bun target by them. It also needs
    includeCommandLineAPI require() support.
  • Fixture layout (overwrite-config). Its
    --require ./test/fixtures/overwrite-config-preload-module.js is relative to
    the repo root, and the fixture itself does require('../common'). Bun's node
    fixtures live under test/js/node/test/fixtures, so no single placement
    satisfies both.
  • Remaining one-offs: debug-async-hook (times out waiting for disconnect under
    bun test), inspect-brk-node (--inspect-brk-node must break inside the
    bootstrap), port-zero and port-zero-cluster (--inspect-port, and
    process.debugPort reflecting the resolved ephemeral port), strip-types
    (gated on process.config.variables.node_use_amaro, which Bun deliberately
    does not set — Node's amaro path is strip-only and Bun's transpiler is not —
    and which would not be enough anyway: the test then needs
    Debugger.scriptParsed.hasSourceURL for transpiled TypeScript),
    dom-storage (--experimental-storage-inspection).

How this was verified

Every added file was run one process per file the way CI does
(bun run --config=bunfig.node-test.toml <file>), three times each for the six
new ones.

Three existing assertions changed with the behaviour they covered:
inspector-profiler.test.ts asserted inspector.console.log === console.log
and that a disconnected post() with a callback delivers the error to it;
process.test.js asserted process.binding("inspector") throws. All three now
assert the new behaviour, and two cases were added: inspector.console.log
writes nothing to stdout, and the binding exposes isEnabled.

Everything was re-run on base bc6bf60d10 plus this change:
inspector.test.ts 29/29, inspector-profiler.test.ts 45/45,
process.test.js 125/125, cpu-prof.test.ts 9/9, vitest.test.ts 2/2, the 31
inspector tests the base already vendored, and preflight over all 19 added
files — byte-verbatim, one process each, plus a silent-pass check that appends
a throw to each file and confirms it then fails.

@github-actions

Copy link
Copy Markdown
Contributor

Found 4 issues this PR may fix:

  1. support node:inspector #2445 - Improves node:inspector compatibility: reimplements inspector.console, fixes url()/close()/waitForDebugger() for CLI-started inspectors, synchronous Session.post() callbacks, and process.binding('inspector')
  2. Debug run configurations inject --debug-brk into NODE_OPTIONS, causing Bun to error #22880 - Adds explicit handling for --debug/--debug-brk flags with DEP0062 deprecation message, directly addressing the error when these flags are injected via NODE_OPTIONS
  3. Programmatic CPU profiler API #28204 - Enhances programmatic CPU profiling by inserting synthetic (idle) samples for wall-clock time outside JS via inspector.Session
  4. worker_threads + inspector.Session Profiler.stop returns null profile in Bun #28472 - Fixes Session.post() callback semantics (synchronous instead of deferred) and improves CPU profiler behavior, relevant to Profiler.stop returning null

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #2445
Fixes #22880
Fixes #28204
Fixes #28472

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. feat(debugger): add programmatic control for node:inspector support [1/4] #25643 - Implements programmatic debugger control for node:inspector (same foundational inspector support)
  2. feat(inspector): implement node:inspector open/close/url/waitForDebugger [2/4] #25644 - Implements node:inspector open/close/url/waitForDebugger (same CLI-started inspector url()/close() feature)
  3. feat(inspector): implement inspector.Session CDP support [3/4] #25645 - Implements inspector.Session CDP support including Session.post() (same synchronous Session.post callbacks feature)

🤖 Generated with Claude Code

@cirospaciari
cirospaciari force-pushed the claude/node-inspector-domains branch from 9da3669 to 6c8cae2 Compare July 24, 2026 04:42
…/close(), synchronous Session.post callbacks

- inspector.console reports to attached sessions only, with Node's key set,
  instead of aliasing the global console and writing to stdout.
- inspector.url()/close()/waitForDebugger() see a server started by --inspect,
  not only one started by inspector.open().
- Session.post delivers its callback synchronously and throws
  ERR_INSPECTOR_NOT_CONNECTED even when a callback was passed, matching Node.
- CPU profiles account for wall-clock time spent outside JS with (idle) nodes.
- process.binding('inspector').isEnabled().
- NodeTracing.getCategories.
- --debug and --debug-brk are rejected with Node's DEP0062 message and exit 9.
@cirospaciari
cirospaciari force-pushed the claude/node-inspector-domains branch from 6c8cae2 to cdc6e24 Compare July 24, 2026 05:04
@cirospaciari cirospaciari changed the title inspector: Node-shaped inspector.console, CLI-started inspector url()/close(), synchronous Session.post callbacks inspector: Node-shaped inspector.console, CLI-started url()/close(), synchronous Session.post (+19 tests) Jul 24, 2026
@cirospaciari

Copy link
Copy Markdown
Member Author

Superseded by #35396, which combines this branch with claude/node-inspect-debugger (the bun inspect CLI debugger, 31 tests) so there is one review surface instead of two. All 19 vendored tests and every source change from this branch are carried over unchanged, and the merge had no conflicts.

One thing to know: parallel/test-inspector-debug-end.js passes here but fails on the combined branch. The debugger branch replaces process._debugProcess (a no-op stub on this branch) with a real kill(pid, SIGUSR1), and Bun installs no SIGUSR1 handler, so the child kills itself instead of exiting 42. The fix belongs on the debugger branch — nothing in this branch needs to change. Details in #35396.

The branch claude/node-inspector-domains is not deleted.

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