feat: node inspect argv parser, allow exception state to be set from …

…command line
nodejs · iloveitaly · Jun 11, 2023 · Jul 28, 2023 · Jul 28, 2023 · 836df085e4c2741fbd7ff1872cdae527e7b7bf7c
commit 836df085e4c2741fbd7ff1872cdae527e7b7bf7c
diff --git a/lib/internal/debugger/inspect.js b/lib/internal/debugger/inspect.js
@@ -32,10 +32,11 @@ const {
   AbortController,
 } = require('internal/abort_controller');
 
-const { 0: InspectClient, 1: createRepl } =
+const { 0: InspectClient, 1: createRepl, 2: parseArguments } =
     [
       require('internal/debugger/inspect_client'),
       require('internal/debugger/inspect_repl'),
+      require('internal/debugger/util/argument_parser')
     ];
 
 const debuglog = util.debuglog('inspect');
@@ -286,50 +287,6 @@ class NodeInspector {
   }
 }
 
-function parseArgv(args) {
-  const target = ArrayPrototypeShift(args);
-  let host = '127.0.0.1';
-  let port = 9229;
-  let isRemote = false;
-  let script = target;
-  let scriptArgs = args;
-
-  const hostMatch = RegExpPrototypeExec(/^([^:]+):(\d+)$/, target);
-  const portMatch = RegExpPrototypeExec(/^--port=(\d+)$/, target);
-
-  if (hostMatch) {
-    // Connecting to remote debugger
-    host = hostMatch[1];
-    port = Number(hostMatch[2]);
-    isRemote = true;
-    script = null;
-  } else if (portMatch) {
-    // Start on custom port
-    port = Number(portMatch[1]);
-    script = args[0];
-    scriptArgs = ArrayPrototypeSlice(args, 1);
-  } else if (args.length === 1 && RegExpPrototypeExec(/^\d+$/, args[0]) !== null &&
-             target === '-p') {
-    // Start debugger against a given pid
-    const pid = Number(args[0]);
-    try {
-      process._debugProcess(pid);
-    } catch (e) {
-      if (e.code === 'ESRCH') {
-        process.stderr.write(`Target process: ${pid} doesn't exist.\n`);
-        process.exit(kGenericUserError);
-      }
-      throw e;
-    }
-    script = null;
-    isRemote = true;
-  }
-
-  return {
-    host, port, isRemote, script, scriptArgs,
-  };
-}
-
 function startInspect(argv = ArrayPrototypeSlice(process.argv, 2),
                       stdin = process.stdin,
                       stdout = process.stdout) {
@@ -343,7 +300,7 @@ function startInspect(argv = ArrayPrototypeSlice(process.argv, 2),
     process.exit(kInvalidCommandLineArgument);
   }
 
-  const options = parseArgv(argv);
+  const options = parseArguments(argv);
   const inspector = new NodeInspector(options, stdin, stdout);
 
   stdin.resume();

diff --git a/lib/internal/debugger/inspect_repl.js b/lib/internal/debugger/inspect_repl.js
@@ -361,7 +361,7 @@ function aliasProperties(target, mapping) {
 }
 
 function createRepl(inspector) {
-  const { Debugger, HeapProfiler, Profiler, Runtime } = inspector;
+  const { Debugger, HeapProfiler, Profiler, Runtime, options: commandLineOptions } = inspector;
 
   let repl;
 
@@ -370,7 +370,7 @@ function createRepl(inspector) {
   const watchedExpressions = [];
   const knownBreakpoints = [];
   let heapSnapshotPromise = null;
-  let pauseOnExceptionState = process.env.NODE_INSPECT_PAUSE_ON_EXCEPTION_STATE || 'none';
+  let pauseOnExceptionState = commandLineOptions.pauseOnExceptionState || 'none';
   let lastCommand;
 
   // Things we need to reset when the app restarts
@@ -883,7 +883,7 @@ function createRepl(inspector) {
   }
 
   Debugger.on('paused', ({ callFrames, reason /* , hitBreakpoints */ }) => {
-    if (process.env.NODE_INSPECT_RESUME_ON_START === '1' &&
+    if ((process.env.NODE_INSPECT_RESUME_ON_START === '1' || commandLineOptions.inspectResumeOnStart === true) &&
         reason === 'Break on start') {
       debuglog('Paused on start, but NODE_INSPECT_RESUME_ON_START' +
               ' environment variable is set to 1, resuming');

diff --git a/lib/internal/debugger/util/argument_parser.js b/lib/internal/debugger/util/argument_parser.js
@@ -0,0 +1,111 @@
+const {
+  ArrayPrototypeShift,
+  ArrayPrototypeSplice,
+  ArrayPrototypeIncludes,
+  StringPrototypeSplit,
+  RegExpPrototypeExec,
+} = primordials;
+
+function parseBoolean(value) {
+  return value === 'true' || value === '1' || value === 'yes';
+}
+
+function validatePauseOnExceptionState(value) {
+  const validStates = ['uncaught', 'none', 'all'];
+  if (!ArrayPrototypeIncludes(validStates, value)) {
+    throw new Error(`Invalid state passed for pauseOnExceptionState: ${value}. Must be one of 'uncaught', 'none', or 'all'.`);
+  }
+  return value;
+}
+
+function parseArguments(argv) {
+  const legacyArguments = processLegacyArgs(argv)
+
+  let options = {
+    pauseOnExceptionState: undefined,
+    inspectResumeOnStart: undefined
+  }
+
+  // `NODE_INSPECT_OPTIONS` is parsed first and can be overwritten by command line arguments
+
+  if (process.env.NODE_INSPECT_OPTIONS) {
+    const envOptions = StringPrototypeSplit(process.env.NODE_INSPECT_OPTIONS, ' ');
+    for (let i = 0; i < envOptions.length; i++) {
+      switch (envOptions[i]) {
+        case '--pause-on-exception-state':
+          options.pauseOnExceptionState = validatePauseOnExceptionState(envOptions[++i]);
+          break;
+        case '--inspect-resume-on-start':
+          options.inspectResumeOnStart = parseBoolean(envOptions[++i]);
+          break;
+      }
+    }
+  }
+
+  for (let i = 0; i < argv.length;) {
+    switch (argv[i]) {
+      case '--pause-on-exception-state':
+        options.pauseOnExceptionState = validatePauseOnExceptionState(argv[i+1]);
+        ArrayPrototypeSplice(argv, i, 2);
+        break;
+      case '--inspect-resume-on-start':
+        options.inspectResumeOnStart = parseBoolean(argv[i+1]);
+        ArrayPrototypeSplice(argv, i, 2);
+        break;
+      default:
+        i++;
+        break;
+    }
+  }
+
+  return {...options, ...legacyArguments};
+}
+
+// the legacy `node inspect` options assumed the first argument was the target
+// to avoid breaking existing scripts, we maintain this behavior
+
+function processLegacyArgs(args) {
+  const target = ArrayPrototypeShift(args);
+  let host = '127.0.0.1';
+  let port = 9229;
+  let isRemote = false;
+  let script = target;
+  let scriptArgs = args;
+
+  const hostMatch = RegExpPrototypeExec(/^([^:]+):(\d+)$/, target);
+  const portMatch = RegExpPrototypeExec(/^--port=(\d+)$/, target);
+
+  if (hostMatch) {
+    // Connecting to remote debugger
+    host = hostMatch[1];
+    port = Number(hostMatch[2]);
+    isRemote = true;
+    script = null;
+  } else if (portMatch) {
+    // Start on custom port
+    port = Number(portMatch[1]);
+    script = args[0];
+    scriptArgs = ArrayPrototypeSlice(args, 1);
+  } else if (args.length === 1 && RegExpPrototypeExec(/^\d+$/, args[0]) !== null &&
+             target === '-p') {
+    // Start debugger against a given pid
+    const pid = Number(args[0]);
+    try {
+      process._debugProcess(pid);
+    } catch (e) {
+      if (e.code === 'ESRCH') {
+        process.stderr.write(`Target process: ${pid} doesn't exist.\n`);
+        process.exit(kGenericUserError);
+      }
+      throw e;
+    }
+    script = null;
+    isRemote = true;
+  }
+
+  return {
+    host, port, isRemote, script, scriptArgs,
+  };
+}
+
+module.exports = parseArguments;