debugger: wait for target startup

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,
 };

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();

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 });

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' },
+      },
+    }],
+  });
+}));

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());