From 821e2675e3b9b6fd2359a829abeff034836c37ab Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 10 Aug 2026 13:46:04 +0200 Subject: [PATCH 1/2] debugger: wait for target startup The inspector can accept a connection before an --inspect-brk target enters its frontend wait. Runtime.runIfWaitingForDebugger can then be handled too early, allowing the target to subsequently block forever. Wait for NodeRuntime.waitingForDebugger before initializing and releasing launched targets. Race the handshake against disconnects and apply it to both interactive and probe startup. Refs: https://github.com/nodejs/node/issues/64116 Assisted-by: codex:gpt-5.6-sol Co-authored-by: Archkon <180910180+Archkon@users.noreply.github.com> Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com> Signed-off-by: Filip Skokan --- lib/internal/debugger/inspect_helpers.js | 50 ++++++- lib/internal/debugger/inspect_probe.js | 12 ++ lib/internal/debugger/inspect_repl.js | 7 +- .../test-debugger-run-restart-init.js | 66 ++++++++- .../test-debugger-wait-for-debugger.js | 137 ++++++++++++++++++ 5 files changed, 269 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-debugger-wait-for-debugger.js diff --git a/lib/internal/debugger/inspect_helpers.js b/lib/internal/debugger/inspect_helpers.js index f83876e96bc0..1605986254af 100644 --- a/lib/internal/debugger/inspect_helpers.js +++ b/lib/internal/debugger/inspect_helpers.js @@ -4,7 +4,9 @@ const { ArrayPrototypePushApply, Number, Promise, + PromiseWithResolvers, RegExpPrototypeExec, + SafePromiseRace, StringPrototypeEndsWith, } = primordials; @@ -18,7 +20,10 @@ const { AbortController, } = require('internal/abort_controller'); -const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes; +const { + ERR_DEBUGGER_ERROR, + ERR_DEBUGGER_STARTUP_ERROR, +} = require('internal/errors').codes; const { exitCodes: { kInvalidCommandLineArgument, @@ -61,6 +66,48 @@ function ensureTrailingNewline(text) { return StringPrototypeEndsWith(text, '\n') ? text : `${text}\n`; } +async function waitForDebugger( + client, + callMethod = (method) => client.callMethod(method), +) { + const { + promise: waitingPromise, + resolve: resolveWaiting, + } = PromiseWithResolvers(); + const { + promise: closedPromise, + reject: rejectClosed, + } = PromiseWithResolvers(); + const onWaiting = () => resolveWaiting(); + const onClose = () => { + rejectClosed(new ERR_DEBUGGER_ERROR( + 'Debugger session ended while waiting for target startup')); + }; + + // The inspector can accept a connection before the target reaches its + // startup wait. Enabling NodeRuntime makes that state observable whether + // the target was already waiting or starts waiting later. + client.once('NodeRuntime.waitingForDebugger', onWaiting); + client.once('close', onClose); + try { + await SafePromiseRace([ + callMethod('NodeRuntime.enable'), + closedPromise, + ]); + await SafePromiseRace([ + waitingPromise, + closedPromise, + ]); + await SafePromiseRace([ + callMethod('NodeRuntime.disable'), + closedPromise, + ]); + } finally { + client.removeListener('NodeRuntime.waitingForDebugger', onWaiting); + client.removeListener('close', onClose); + } +} + function writeInspectUsageAndExit(invokedAs, message, exitCode) { const code = exitCode ?? (message ? kInvalidCommandLineArgument : 0); const out = code === 0 ? process.stdout : process.stderr; @@ -189,5 +236,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort, module.exports = { ensureTrailingNewline, launchChildProcess, + waitForDebugger, writeInspectUsageAndExit, }; diff --git a/lib/internal/debugger/inspect_probe.js b/lib/internal/debugger/inspect_probe.js index b6cac6dc5779..3b77a46f1ba1 100644 --- a/lib/internal/debugger/inspect_probe.js +++ b/lib/internal/debugger/inspect_probe.js @@ -33,6 +33,7 @@ const InspectClient = require('internal/debugger/inspect_client'); const { ensureTrailingNewline, launchChildProcess, + waitForDebugger, } = require('internal/debugger/inspect_helpers'); const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes; @@ -1044,6 +1045,17 @@ class ProbeInspectorSession { this.connected = true; try { + try { + await waitForDebugger( + this.client, + (method) => this.callCdp(method), + ); + } catch (err) { + // A close event may have completed the structured report while the + // readiness helper was rejecting its disconnect race. + if (this.finished) { throw kInspectorFailedSentinel; } + throw err; + } await this.callCdp('Runtime.enable'); await this.callCdp('Debugger.enable'); await this.bindBreakpoints(); diff --git a/lib/internal/debugger/inspect_repl.js b/lib/internal/debugger/inspect_repl.js index 548df089fb14..69ca174dd241 100644 --- a/lib/internal/debugger/inspect_repl.js +++ b/lib/internal/debugger/inspect_repl.js @@ -60,6 +60,7 @@ const { fileURLToPath } = require('internal/url'); const { customInspectSymbol, SideEffectFreeRegExpPrototypeSymbolReplace } = require('internal/util'); const { inspect: utilInspect } = require('internal/util/inspect'); const { isObjectLiteral } = require('internal/repl/utils'); +const { waitForDebugger } = require('internal/debugger/inspect_helpers'); const debuglog = require('internal/util/debuglog').debuglog('inspect'); const SHORTCUTS = { @@ -1204,9 +1205,13 @@ function createRepl(inspector) { } async function initAfterStart() { + const waitForDebuggerOnStart = !!inspector.options?.script; waitForInitialBreakRender = - !!inspector.options?.script && + waitForDebuggerOnStart && process.env.NODE_INSPECT_RESUME_ON_START !== '1'; + if (waitForDebuggerOnStart) { + await waitForDebugger(inspector.client); + } await Runtime.enable(); await Profiler.enable(); await Profiler.setSamplingInterval({ interval: 100 }); diff --git a/test/parallel/test-debugger-run-restart-init.js b/test/parallel/test-debugger-run-restart-init.js index 78f237353baf..b57939135f80 100644 --- a/test/parallel/test-debugger-run-restart-init.js +++ b/test/parallel/test-debugger-run-restart-init.js @@ -79,9 +79,29 @@ async function assertCommandWaitsForInit(repl, command, gate, calls) { const runGate = createGate(); const restartGate = createGate(); const gates = [null, runGate, restartGate]; + const client = new EventEmitter(); + let nodeRuntimeEnableCount = 0; + client.callMethod = common.mustCall(async (method) => { + calls.push(method); + if (method === 'NodeRuntime.enable') { + const emitWaiting = () => { + calls.push('NodeRuntime.waitingForDebugger'); + client.emit('NodeRuntime.waitingForDebugger'); + }; + // Cover notifications arriving both before and after the enable reply. + if (nodeRuntimeEnableCount++ % 2 === 0) { + emitWaiting(); + } else { + setImmediate(emitWaiting); + } + } else { + assert.strictEqual(method, 'NodeRuntime.disable'); + } + }, 6); const inspector = { - client: new EventEmitter(), + client, domainNames: ['Debugger', 'HeapProfiler', 'Profiler', 'Runtime'], + options: { script: 'debugger-target.js' }, stdin: new PassThrough(), stdout: new PassThrough(), run: common.mustCall(async () => { @@ -101,6 +121,29 @@ async function assertCommandWaitsForInit(repl, command, gate, calls) { await assertCommandWaitsForInit(repl, 'run', runGate, calls); await assertCommandWaitsForInit(repl, 'restart', restartGate, calls); + assert.deepStrictEqual( + calls.filter((call) => ( + call === 'NodeRuntime.enable' || + call === 'NodeRuntime.waitingForDebugger' || + call === 'NodeRuntime.disable' || + call === 'Runtime.runIfWaitingForDebugger' + )), + [ + 'NodeRuntime.enable', + 'NodeRuntime.waitingForDebugger', + 'NodeRuntime.disable', + 'Runtime.runIfWaitingForDebugger', + 'NodeRuntime.enable', + 'NodeRuntime.waitingForDebugger', + 'NodeRuntime.disable', + 'Runtime.runIfWaitingForDebugger', + 'NodeRuntime.enable', + 'NodeRuntime.waitingForDebugger', + 'NodeRuntime.disable', + 'Runtime.runIfWaitingForDebugger', + ], + ); + assert.deepStrictEqual( calls.filter((call) => ( call === 'inspector.run' || @@ -116,4 +159,25 @@ async function assertCommandWaitsForInit(repl, command, gate, calls) { ); repl.close(); + + const attachCalls = []; + const attachClient = new EventEmitter(); + attachClient.callMethod = common.mustNotCall(); + const attachInspector = { + client: attachClient, + domainNames: ['Debugger', 'HeapProfiler', 'Profiler', 'Runtime'], + options: {}, + stdin: new PassThrough(), + stdout: new PassThrough(), + suspendReplWhile(fn) { + return fn(); + }, + }; + + for (const domain of attachInspector.domainNames) { + attachInspector[domain] = createAgent(domain, attachCalls, []); + } + + const attachRepl = await createRepl(attachInspector)(); + attachRepl.close(); })().then(common.mustCall()); diff --git a/test/parallel/test-debugger-wait-for-debugger.js b/test/parallel/test-debugger-wait-for-debugger.js new file mode 100644 index 000000000000..b438147ea832 --- /dev/null +++ b/test/parallel/test-debugger-wait-for-debugger.js @@ -0,0 +1,137 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); + +common.skipIfInspectorDisabled(); + +const assert = require('assert'); +const { EventEmitter } = require('events'); +const { + waitForDebugger, +} = require('internal/debugger/inspect_helpers'); + +function assertListenersRemoved(client) { + assert.strictEqual( + client.listenerCount('NodeRuntime.waitingForDebugger'), + 0, + ); + assert.strictEqual(client.listenerCount('close'), 0); +} + +async function testWaitingNotification(beforeEnableReply) { + const client = new EventEmitter(); + const calls = []; + client.callMethod = common.mustCall(async (method) => { + calls.push(method); + const emitWaiting = () => { + client.emit('NodeRuntime.waitingForDebugger'); + }; + if (method === 'NodeRuntime.enable') { + if (beforeEnableReply) { + emitWaiting(); + } else { + setImmediate(emitWaiting); + } + } else { + assert.strictEqual(method, 'NodeRuntime.disable'); + } + }, 2); + + await waitForDebugger(client); + assert.deepStrictEqual(calls, [ + 'NodeRuntime.enable', + 'NodeRuntime.disable', + ]); + assertListenersRemoved(client); +} + +async function testCloseWhileWaiting(beforeEnableReply) { + const client = new EventEmitter(); + client.callMethod = common.mustCall((method) => { + assert.strictEqual(method, 'NodeRuntime.enable'); + setImmediate(() => client.emit('close')); + return beforeEnableReply ? new Promise(() => {}) : Promise.resolve(); + }); + + await assert.rejects( + waitForDebugger(client), + { + code: 'ERR_DEBUGGER_ERROR', + message: 'Debugger session ended while waiting for target startup', + }, + ); + assertListenersRemoved(client); +} + +async function testCloseWhileDisabling() { + const client = new EventEmitter(); + client.callMethod = common.mustCall((method) => { + if (method === 'NodeRuntime.enable') { + client.emit('NodeRuntime.waitingForDebugger'); + return Promise.resolve(); + } + assert.strictEqual(method, 'NodeRuntime.disable'); + setImmediate(() => client.emit('close')); + return new Promise(() => {}); + }, 2); + + await assert.rejects( + waitForDebugger(client), + { + code: 'ERR_DEBUGGER_ERROR', + message: 'Debugger session ended while waiting for target startup', + }, + ); + assertListenersRemoved(client); +} + +async function testEnableFailure() { + const client = new EventEmitter(); + const expected = new Error('NodeRuntime.enable failed'); + client.callMethod = common.mustCall(async (method) => { + assert.strictEqual(method, 'NodeRuntime.enable'); + throw expected; + }); + + await assert.rejects( + waitForDebugger(client), + (error) => { + assert.strictEqual(error, expected); + return true; + }, + ); + assertListenersRemoved(client); +} + +async function testDisableFailure() { + const client = new EventEmitter(); + const expected = new Error('NodeRuntime.disable failed'); + client.callMethod = common.mustCall(async (method) => { + if (method === 'NodeRuntime.enable') { + client.emit('NodeRuntime.waitingForDebugger'); + return; + } + assert.strictEqual(method, 'NodeRuntime.disable'); + throw expected; + }, 2); + + await assert.rejects( + waitForDebugger(client), + (error) => { + assert.strictEqual(error, expected); + return true; + }, + ); + assertListenersRemoved(client); +} + +(async () => { + await testWaitingNotification(true); + await testWaitingNotification(false); + await testCloseWhileWaiting(true); + await testCloseWhileWaiting(false); + await testCloseWhileDisabling(); + await testEnableFailure(); + await testDisableFailure(); +})().then(common.mustCall()); From 18ce20f1dbb8be376d73ce8fb1c80f25f26b870b Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sat, 15 Aug 2026 15:19:22 +0300 Subject: [PATCH 2/2] fixup! debugger: wait for target startup --- .../test-debugger-probe-startup-disconnect.js | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 test/parallel/test-debugger-probe-startup-disconnect.js diff --git a/test/parallel/test-debugger-probe-startup-disconnect.js b/test/parallel/test-debugger-probe-startup-disconnect.js new file mode 100644 index 000000000000..68c8432dc552 --- /dev/null +++ b/test/parallel/test-debugger-probe-startup-disconnect.js @@ -0,0 +1,56 @@ +// Flags: --expose-internals +// This tests that a disconnect while probe mode is waiting for target startup +// is reported as a structured probe failure instead of an internal error. +'use strict'; + +const common = require('../common'); +common.skipIfInspectorDisabled(); + +const assert = require('assert'); +const { EventEmitter } = require('events'); +const { assertProbeJson } = require('../common/debugger-probe'); +const { ProbeInspectorSession } = require('internal/debugger/inspect_probe'); + +const probe = { + expr: 'value', + target: { suffix: 'probe-target.js', line: 1 }, +}; +const client = new EventEmitter(); +client.connect = common.mustCall(); +client.callMethod = common.mustCall((method) => { + assert.strictEqual(method, 'NodeRuntime.enable'); + setImmediate(() => client.emit('close')); + return new Promise(() => {}); +}); +client.reset = common.mustCall(); + +const session = new ProbeInspectorSession({ + childArgv: ['-e', ''], + host: '127.0.0.1', + port: 0, + probes: [probe], + skipPortPreflight: true, +}); +session.client = client; + +session.run().then(common.mustCall(({ code, report }) => { + assert.strictEqual(code, 1); + assertProbeJson(report, { + v: 2, + probes: [probe], + results: [{ + event: 'error', + pending: [0], + error: { + code: 'probe_failure', + message: + 'Inspector connection lost before probes started before probes: ' + + 'probe-target.js:1. The target startup may have torn down the ' + + 'inspector. If startup does not touch the inspector, this is likely ' + + 'a Node.js bug. Please file an issue.', + stderr: '', + details: { lastCdpMethod: 'NodeRuntime.enable' }, + }, + }], + }); +}));