Increment inspector port for spawned subcommands (#991)

* Increment inspector port for spawned subcommands

* Add tests for --inspect port incrementing

* Skip --inspect tests for node 6

* Check for --inspect before regex matches

* Add #991 to CHANGELOG

* Add inspector port to README

* Switch debugging link to english

* Lint

CHANGELOG.md

@@ -15,6 +15,7 @@
     * can now define both `--foo` and `--no-foo`
     * custom event listeners: `--no-foo` on cli now emits `option:no-foo` (previously `option:foo`)
     * default value: defining `--no-foo` after defining `--foo` leaves the default value unchanged (previously set it to false)
+  * Increment inspector port for spawned subcommands (#991)
   * Change docs for `.command` to contrast action handler vs git-style executable. TypeScript now uses overloaded function. (#938)
 
 2.20.0 / 2019-04-02

Readme.md

@@ -503,6 +503,11 @@ You can enable `--harmony` option in two ways:
 * Use `#! /usr/bin/env node --harmony` in the sub-commands scripts. (Note Windows does not support this pattern.)
 * Use the `--harmony` option when call the command, like `node --harmony examples/pm publish`. The `--harmony` option will be preserved when spawning sub-command process.
 
+### Node debugging
+
+If you are using the node inspector for [debugging](https://nodejs.org/en/docs/guides/debugging-getting-started/) git-style executable (sub)commands using `node -inspect` et al,
+the inspector port is incremented by 1 for the spawned subcommand.
+
 ## Examples
 
 ```js

index.js

@@ -542,7 +542,7 @@ Command.prototype.executeSubCommand = function(argv, args, unknown) {
     if (isExplicitJS) {
       args.unshift(bin);
       // add executable arguments to spawn
-      args = (process.execArgv || []).concat(args);
+      args = incrementNodeInspectorPort(process.execArgv).concat(args);
 
       proc = spawn(process.argv[0], args, { stdio: 'inherit', customFds: [0, 1, 2] });
     } else {
@@ -551,7 +551,7 @@ Command.prototype.executeSubCommand = function(argv, args, unknown) {
   } else {
     args.unshift(bin);
     // add executable arguments to spawn
-    args = (process.execArgv || []).concat(args);
+    args = incrementNodeInspectorPort(process.execArgv).concat(args);
     proc = spawn(process.execPath, args, { stdio: 'inherit' });
   }
 
@@ -1261,3 +1261,50 @@ function exists(file) {
     return false;
   }
 }
+
+/**
+ * Scan arguments and increment port number for inspect calls (to avoid conflicts when spawning new command).
+ *
+ * @param {string[]} args - array of arguments from node.execArgv
+ * @returns {string[]}
+ * @api private
+ */
+
+function incrementNodeInspectorPort(args) {
+  // Testing for these options:
+  //  --inspect[=[host:]port]
+  //  --inspect-brk[=[host:]port]
+  //  --inspect-port=[host:]port
+  return args.map((arg) => {
+    var result = arg;
+    if (arg.indexOf('--inspect') === 0) {
+      var debugOption;
+      var debugHost = '127.0.0.1';
+      var debugPort = '9229';
+      var match;
+      if ((match = arg.match(/^(--inspect(-brk)?)$/)) !== null) {
+        // e.g. --inspect
+        debugOption = match[1];
+      } else if ((match = arg.match(/^(--inspect(-brk|-port)?)=([^:]+)$/)) !== null) {
+        debugOption = match[1];
+        if (/^\d+$/.test(match[3])) {
+          // e.g. --inspect=1234
+          debugPort = match[3];
+        } else {
+          // e.g. --inspect=localhost
+          debugHost = match[3];
+        }
+      } else if ((match = arg.match(/^(--inspect(-brk|-port)?)=([^:]+):(\d+)$/)) !== null) {
+        // e.g. --inspect=localhost:1234
+        debugOption = match[1];
+        debugHost = match[3];
+        debugPort = match[4];
+      }
+
+      if (debugOption && debugPort !== '0') {
+        result = `${debugOption}=${debugHost}:${parseInt(debugPort) + 1}`;
+      }
+    }
+    return result;
+  });
+}

test/fixtures/inspect-sub.js

@@ -0,0 +1 @@
+console.log(process.execArgv);

test/fixtures/inspect.js

@@ -0,0 +1,7 @@
+#!/usr/bin/env node
+
+var program = require('../../');
+
+program
+  .command('sub', 'install one or more packages')
+  .parse(process.argv);

test/test.command.executableSubcommand.inspect.js

@@ -0,0 +1,50 @@
+const exec = require('child_process').exec;
+const path = require('path');
+const should = require('should');
+
+var inspectCommand = path.join(__dirname, 'fixtures', 'inspect.js');
+
+exec(`node ${inspectCommand} sub`, function(_error, stdout, stderr) {
+  stdout.should.equal('[]\n');
+});
+
+exec(`node --harmony ${inspectCommand} sub`, function(_error, stdout, stderr) {
+  stdout.should.equal("[ '--harmony' ]\n");
+});
+
+// Skip tests for node 6 when --inspect was only prototype
+if (process.version.substr(1).split('.')[0] === '6') {
+  console.log('Skipping --inspect tests for node 6');
+} else {
+  // Test that inspector port gets incremented.
+  // If we reuse port we can get conflicts because port not released fast enough.
+
+  // --inspect defaults to 127.0.0.1:9229, port should be incremented
+  exec(`node --inspect ${inspectCommand} sub`, function(_error, stdout, stderr) {
+    stdout.should.equal("[ '--inspect=127.0.0.1:9230' ]\n");
+  });
+
+  // custom port
+  exec(`node --inspect=9240 ${inspectCommand} sub`, function(_error, stdout, stderr) {
+    stdout.should.equal("[ '--inspect=127.0.0.1:9241' ]\n");
+  });
+  // zero is special, random port
+  exec(`node --inspect=0 ${inspectCommand} sub`, function(_error, stdout, stderr) {
+    stdout.should.equal("[ '--inspect=0' ]\n");
+  });
+
+  // ip address
+  exec(`node --inspect=127.0.0.1:9250 ${inspectCommand} sub`, function(_error, stdout, stderr) {
+    stdout.should.equal("[ '--inspect=127.0.0.1:9251' ]\n");
+  });
+
+  // localhost
+  exec(`node --inspect=localhost:9260 ${inspectCommand} sub`, function(_error, stdout, stderr) {
+    stdout.should.equal("[ '--inspect=localhost:9261' ]\n");
+  });
+
+  // --inspect-port, just test most likely format
+  exec(`node --inspect-port=9270 ${inspectCommand} sub`, function(_error, stdout, stderr) {
+    stdout.should.equal("[ '--inspect-port=127.0.0.1:9271' ]\n");
+  });
+}