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

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