Repository: nodejs/node
Pull request: sea: allow using inspector command line flags with SEA

The inspector command line flags should be allowed via NODE_OPTIONS, though it should not be consumed when passed directly to the SEA. This patch allows it to consume inspector flags from NODE_OPTIONS and add tests for different behaviors.

Combined with https://github.com/nodejs/node/pull/59560, users can still disable inspector options in NODE_OPTIONS if they set `execArgvExtension` to either `none` or `cli`. They could alos enable it via the special CLI flag `--node-options="--inspect"` if they set it to `cli` in the configuration.

Fixes: https://github.com/nodejs/node/issues/51688

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: src/node_options.cc
patch: @@ -422,10 +422,6 @@ void Parse(
 // TODO(addaleax): Make that unnecessary.
 
 DebugOptionsParser::DebugOptionsParser() {
-#ifndef DISABLE_SINGLE_EXECUTABLE_APPLICATION
-  if (sea::IsSingleExecutable()) return;
-#endif
-
   AddOption("--inspect-port",
             "set host:port for inspector",
             &DebugOptions::host_port,

filename: test/sequential/sequential.status
patch: @@ -61,6 +61,8 @@ test-single-executable-application-disable-experimental-sea-warning: SKIP
 test-single-executable-application-empty: SKIP
 test-single-executable-application-exec-argv: SKIP
 test-single-executable-application-exec-argv-empty: SKIP
+test-single-executable-application-inspect-in-sea-flags: SKIP
+test-single-executable-application-inspect: SKIP
 test-single-executable-application-snapshot: SKIP
 test-single-executable-application-snapshot-and-code-cache: SKIP
 test-single-executable-application-snapshot-worker: SKIP

filename: test/sequential/test-single-executable-application-inspect-in-sea-flags.js
patch: @@ -0,0 +1,69 @@
+'use strict';
+
+// This tests that the debugger flag --inspect passed directly to a single executable
+// application would not be consumed by Node.js but passed to the application
+// instead.
+
+require('../common');
+const assert = require('assert');
+const { writeFileSync, existsSync } = require('fs');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const tmpdir = require('../common/tmpdir');
+const { spawnSyncAndExitWithoutError } = require('../common/child_process');
+
+const {
+  generateSEA,
+  skipIfSingleExecutableIsNotSupported,
+} = require('../common/sea');
+
+skipIfSingleExecutableIsNotSupported();
+
+const configFile = tmpdir.resolve('sea-config.json');
+const seaPrepBlob = tmpdir.resolve('sea-prep.blob');
+const outputFile = tmpdir.resolve(process.platform === 'win32' ? 'sea.exe' : 'sea');
+
+tmpdir.refresh();
+
+writeFileSync(tmpdir.resolve('sea.js'), `console.log(process.argv);`, 'utf-8');
+
+// Create SEA configuration
+writeFileSync(configFile, `
+{
+  "main": "sea.js",
+  "output": "sea-prep.blob"
+}
+`);
+
+// Generate the SEA prep blob
+spawnSyncAndExitWithoutError(
+  process.execPath,
+  ['--experimental-sea-config', 'sea-config.json'],
+  { cwd: tmpdir.path }
+);
+
+assert(existsSync(seaPrepBlob));
+
+// Generate the SEA executable
+generateSEA(outputFile, process.execPath, seaPrepBlob);
+
+// Spawn the SEA with inspect option
+spawnSyncAndAssert(
+  outputFile,
+  ['--inspect=0'],
+  {
+    env: {
+      ...process.env,
+    },
+  },
+  {
+    stdout(data) {
+      assert.match(data, /--inspect=0/);
+      return true;
+    },
+    stderr(data) {
+      assert.doesNotMatch(data, /Debugger listening/);
+      return true;
+    },
+    trim: true,
+  }
+);

filename: test/sequential/test-single-executable-application-inspect.js
patch: @@ -0,0 +1,122 @@
+'use strict';
+
+// This tests the creation of a single executable application that can be
+// debugged using the inspector protocol with NODE_OPTIONS=--inspect-brk=0
+
+require('../common');
+const assert = require('assert');
+const { writeFileSync, existsSync } = require('fs');
+const { spawn } = require('child_process');
+const tmpdir = require('../common/tmpdir');
+const { spawnSyncAndExitWithoutError } = require('../common/child_process');
+
+const {
+  generateSEA,
+  skipIfSingleExecutableIsNotSupported,
+} = require('../common/sea');
+
+skipIfSingleExecutableIsNotSupported();
+
+const configFile = tmpdir.resolve('sea-config.json');
+const seaPrepBlob = tmpdir.resolve('sea-prep.blob');
+const outputFile = tmpdir.resolve(process.platform === 'win32' ? 'sea.exe' : 'sea');
+
+tmpdir.refresh();
+
+// Create a simple hello world script
+writeFileSync(tmpdir.resolve('hello.js'), `console.log('Hello, world!');`, 'utf-8');
+
+// Create SEA configuration
+writeFileSync(configFile, `
+{
+  "main": "hello.js",
+  "output": "sea-prep.blob"
+}
+`);
+
+// Generate the SEA prep blob
+spawnSyncAndExitWithoutError(
+  process.execPath,
+  ['--experimental-sea-config', 'sea-config.json'],
+  { cwd: tmpdir.path }
+);
+
+assert(existsSync(seaPrepBlob));
+
+// Generate the SEA executable
+generateSEA(outputFile, process.execPath, seaPrepBlob);
+
+// Spawn the SEA with inspect option
+const seaProcess = spawn(outputFile, [], {
+  env: {
+    ...process.env,
+    NODE_OPTIONS: '--inspect-brk=0',
+  },
+});
+
+let debuggerUrl = null;
+let seaStderr = '';
+
+seaProcess.stderr.setEncoding('utf8');
+seaProcess.stdout.setEncoding('utf8');
+
+seaProcess.stdout.on('data', (data) => {
+  console.log(`[SEA][STDOUT] ${data}`);
+});
+
+seaProcess.stderr.on('data', (data) => {
+  console.log(`[SEA][STDERR] ${data}`);
+  seaStderr += data;
+
+  // Parse the debugger listening message
+  const match = seaStderr.match(/Debugger listening on ws:\/\/([\d.]+):(\d+)\//);
+  if (match && !debuggerUrl) {
+    const host = match[1];
+    const port = match[2];
+    debuggerUrl = `${host}:${port}`;
+
+    console.log(`Running ${process.execPath} inspect ${debuggerUrl}`);
+    // Once we have the debugger URL, spawn the inspector CLI
+    const inspectorProcess = spawn(process.execPath, ['inspect', debuggerUrl], {
+      stdio: ['pipe', 'pipe', 'pipe'],
+    });
+
+    let inspectorStdout = '';
+    inspectorProcess.stdout.setEncoding('utf8');
+    inspectorProcess.stderr.setEncoding('utf8');
+
+    inspectorProcess.stdout.on('data', (data) => {
+      console.log(`[INSPECT][STDOUT] ${data}`);
+      inspectorStdout += data;
+
+      // Check if we successfully connected
+      const matches = [...inspectorStdout.matchAll(/debug> /g)];
+      if (inspectorStdout.includes(`connecting to ${host}:${port} ... ok`) &&
+          matches.length >= 2) {
+        // We are at the second prompt, which means we can send commands to terminate both now.
+        console.log('Sending .exit command to inspector...');
+        inspectorProcess.stdin.write('.exit\n');
+      }
+    });
+
+    inspectorProcess.stderr.on('data', (data) => {
+      console.log(`[INSPECT][STDERR] ${data}`);
+    });
+
+    inspectorProcess.on('close', (code) => {
+      assert.strictEqual(code, 0, `Inspector process exited with code ${code}.`);
+    });
+
+    inspectorProcess.on('error', (err) => {
+      throw err;
+    });
+  }
+});
+
+seaProcess.on('close', (code) => {
+  assert.strictEqual(code, 0, `SEA process exited with code ${code}.`);
+});
+
+seaProcess.on('error', (err) => {
+  throw err;
+});