From c0ae3b237364f5e6723cfa70784eafd55f466a7e Mon Sep 17 00:00:00 2001
From: Kohei Ueno <kohei.ueno119@gmail.com>
Date: Sun, 12 May 2024 03:48:30 +0900
Subject: [PATCH] inspector: introduce the `--inspect-wait` flag

PR-URL: https://github.com/nodejs/node/pull/52734
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
---
 doc/api/cli.md                        | 16 +++++++++++++++
 doc/api/debugger.md                   | 17 ++++++++++++++--
 doc/node.1                            |  5 +++++
 src/inspector_agent.cc                | 20 +++++++++++--------
 src/node_options.cc                   |  8 ++++++++
 src/node_options.h                    |  6 ++++++
 test/parallel/test-inspector-wait.mjs | 28 +++++++++++++++++++++++++++
 7 files changed, 90 insertions(+), 10 deletions(-)
 create mode 100644 test/parallel/test-inspector-wait.mjs

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 1b14aa45102f92..abf49fe004a95a 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -1272,6 +1272,7 @@ Activate inspector on `host:port`. Default is `127.0.0.1:9229`.
 V8 inspector integration allows tools such as Chrome DevTools and IDEs to debug
 and profile Node.js instances. The tools attach to Node.js instances via a
 tcp port and communicate using the [Chrome DevTools Protocol][].
+See [V8 Inspector integration for Node.js][] for further explanation on Node.js debugger.
 
 <!-- Anchor to make sure old links find a target -->
 
@@ -1302,6 +1303,8 @@ added: v7.6.0
 Activate inspector on `host:port` and break at start of user script.
 Default `host:port` is `127.0.0.1:9229`.
 
+See [V8 Inspector integration for Node.js][] for further explanation on Node.js debugger.
+
 ### `--inspect-port=[host:]port`
 
 <!-- YAML
@@ -1323,6 +1326,17 @@ Specify ways of the inspector web socket url exposure.
 By default inspector websocket url is available in stderr and under `/json/list`
 endpoint on `http://host:port/json/list`.
 
+### `--inspect-wait[=[host:]port]`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+Activate inspector on `host:port` and wait for debugger to be attached.
+Default `host:port` is `127.0.0.1:9229`.
+
+See [V8 Inspector integration for Node.js][] for further explanation on Node.js debugger.
+
 ### `-i`, `--interactive`
 
 <!-- YAML
@@ -2662,6 +2676,7 @@ one is included in the list below.
 * `--inspect-brk`
 * `--inspect-port`, `--debug-port`
 * `--inspect-publish-uid`
+* `--inspect-wait`
 * `--inspect`
 * `--max-http-header-size`
 * `--napi-modules`
@@ -3152,6 +3167,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
 [ScriptCoverage]: https://chromedevtools.github.io/devtools-protocol/tot/Profiler#type-ScriptCoverage
 [ShadowRealm]: https://github.com/tc39/proposal-shadowrealm
 [Source Map]: https://sourcemaps.info/spec.html
+[V8 Inspector integration for Node.js]: debugger.md#v8-inspector-integration-for-nodejs
 [V8 JavaScript code coverage]: https://v8project.blogspot.com/2017/12/javascript-code-coverage.html
 [V8 code cache]: https://v8.dev/blog/code-caching-for-devs
 [`"type"`]: packages.md#type
diff --git a/doc/api/debugger.md b/doc/api/debugger.md
index b280fdef68b8c8..03e9cd9185027e 100644
--- a/doc/api/debugger.md
+++ b/doc/api/debugger.md
@@ -234,8 +234,21 @@ V8 Inspector can be enabled by passing the `--inspect` flag when starting a
 Node.js application. It is also possible to supply a custom port with that flag,
 e.g. `--inspect=9222` will accept DevTools connections on port 9222.
 
-To break on the first line of the application code, pass the `--inspect-brk`
-flag instead of `--inspect`.
+Using the `--inspect` flag will execute the code immediately before debugger is connected.
+This means that the code will start running before you can start debugging, which might
+not be ideal if you want to debug from the very beginning.
+
+In such cases, you have two alternatives:
+
+1. `--inspect-wait` flag: This flag will wait for debugger to be attached before executing the code.
+   This allows you to start debugging right from the beginning of the execution.
+2. `--inspect-brk` flag: Unlike `--inspect`, this flag will break on the first line of the code
+   as soon as debugger is attached. This is useful when you want to debug the code step by step
+   from the very beginning, without any code execution prior to debugging.
+
+So, when deciding between `--inspect`, `--inspect-wait`, and `--inspect-brk`, consider whether you want
+the code to start executing immediately, wait for debugger to be attached before execution,
+or break on the first line for step-by-step debugging.
 
 ```console
 $ node --inspect index.js
diff --git a/doc/node.1 b/doc/node.1
index 9ce115141b6b52..a1f90f8499a50a 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -272,6 +272,11 @@ and
 Default is
 .Sy stderr,http .
 .
+.It Fl -inspect-wait Ns = Ns Ar [host:]port
+Activate inspector on
+.Ar host:port
+and wait for debugger to be attached.
+.
 .It Fl -inspect Ns = Ns Ar [host:]port
 Activate inspector on
 .Ar host:port .
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
index 63c8ae14abb3de..cb82f5239a5638 100644
--- a/src/inspector_agent.cc
+++ b/src/inspector_agent.cc
@@ -743,20 +743,24 @@ bool Agent::Start(const std::string& path,
   }, parent_env_);
 
   bool wait_for_connect = options.wait_for_connect();
+  bool should_break_first_line = options.should_break_first_line();
   if (parent_handle_) {
-    wait_for_connect = parent_handle_->WaitForConnect();
-    parent_handle_->WorkerStarted(client_->getThreadHandle(), wait_for_connect);
+    should_break_first_line = parent_handle_->WaitForConnect();
+    parent_handle_->WorkerStarted(client_->getThreadHandle(),
+                                  should_break_first_line);
   } else if (!options.inspector_enabled || !options.allow_attaching_debugger ||
              !StartIoThread()) {
     return false;
   }
 
-  // Patch the debug options to implement waitForDebuggerOnStart for
-  // the NodeWorker.enable method.
-  if (wait_for_connect) {
-    CHECK(!parent_env_->has_serialized_options());
-    debug_options_.EnableBreakFirstLine();
-    parent_env_->options()->get_debug_options()->EnableBreakFirstLine();
+  if (wait_for_connect || should_break_first_line) {
+    // Patch the debug options to implement waitForDebuggerOnStart for
+    // the NodeWorker.enable method.
+    if (should_break_first_line) {
+      CHECK(!parent_env_->has_serialized_options());
+      debug_options_.EnableBreakFirstLine();
+      parent_env_->options()->get_debug_options()->EnableBreakFirstLine();
+    }
     client_->waitForFrontend();
   }
   return true;
diff --git a/src/node_options.cc b/src/node_options.cc
index 74bb50cd8b5153..699630fecfea8a 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -331,6 +331,14 @@ DebugOptionsParser::DebugOptionsParser() {
   Implies("--inspect-brk-node", "--inspect");
   AddAlias("--inspect-brk-node=", { "--inspect-port", "--inspect-brk-node" });
 
+  AddOption(
+      "--inspect-wait",
+      "activate inspector on host:port and wait for debugger to be attached",
+      &DebugOptions::inspect_wait,
+      kAllowedInEnvvar);
+  Implies("--inspect-wait", "--inspect");
+  AddAlias("--inspect-wait=", {"--inspect-port", "--inspect-wait"});
+
   AddOption("--inspect-publish-uid",
             "comma separated list of destinations for inspector uid"
             "(default: stderr,http)",
diff --git a/src/node_options.h b/src/node_options.h
index cf7f2c81507790..2d9bdba99d706d 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -71,6 +71,8 @@ class DebugOptions : public Options {
   bool allow_attaching_debugger = true;
   // --inspect
   bool inspector_enabled = false;
+  // --inspect-wait
+  bool inspect_wait = false;
   // --debug
   bool deprecated_debug = false;
   // --inspect-brk
@@ -93,6 +95,10 @@ class DebugOptions : public Options {
   }
 
   bool wait_for_connect() const {
+    return break_first_line || break_node_first_line || inspect_wait;
+  }
+
+  bool should_break_first_line() const {
     return break_first_line || break_node_first_line;
   }
 
diff --git a/test/parallel/test-inspector-wait.mjs b/test/parallel/test-inspector-wait.mjs
new file mode 100644
index 00000000000000..9bb28ed22b258d
--- /dev/null
+++ b/test/parallel/test-inspector-wait.mjs
@@ -0,0 +1,28 @@
+import * as common from '../common/index.mjs';
+
+common.skipIfInspectorDisabled();
+
+import assert from 'node:assert';
+import { NodeInstance } from '../common/inspector-helper.js';
+
+
+async function runTests() {
+  const child = new NodeInstance(['--inspect-wait=0'], 'console.log(0);');
+  const session = await child.connectInspectorSession();
+  await session.send({ method: 'NodeRuntime.enable' });
+  await session.waitForNotification('NodeRuntime.waitingForDebugger');
+
+  // The execution should be paused until the debugger is attached
+  while (await child.nextStderrString() !== 'Debugger attached.');
+
+  await session.send({ 'method': 'Runtime.runIfWaitingForDebugger' });
+
+  // Wait for the execution to finish
+  while (await child.nextStderrString() !== 'Waiting for the debugger to disconnect...');
+
+  await session.send({ method: 'NodeRuntime.disable' });
+  session.disconnect();
+  assert.strictEqual((await child.expectShutdown()).exitCode, 0);
+}
+
+runTests().then(common.mustCall());