From 1b53561b5e4d87e4072aef4e2938a346e77d0396 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 27 Oct 2017 10:28:47 +0200 Subject: [PATCH] fix: Make --inspect-port=0 work Selecting port zero makes the inspector bind to a random port. The inspector prints the URL it listens on to stderr. Parse the output from the child process to find the port number. This commit coincidentally also makes `node inspect --port=0 x.js` work and a regression test to that effect has been added. Note that connecting to an existing process that listens on a non-standard port with `node inspect -p ` does not work because then we never see the output from the inspector. Fixes: https://github.com/nodejs/node/issues/16469 --- lib/_inspect.js | 31 ++++++++++---------------- lib/internal/inspect_client.js | 10 +++++---- test/cli/launch.test.js | 40 ++++++++++++++++++++++++++++++++++ test/cli/start-cli.js | 4 ++-- 4 files changed, 60 insertions(+), 25 deletions(-) diff --git a/lib/_inspect.js b/lib/_inspect.js index 2691227..d846efb 100644 --- a/lib/_inspect.js +++ b/lib/_inspect.js @@ -42,18 +42,9 @@ const [ InspectClient, createRepl ] = const debuglog = util.debuglog('inspect'); -const DEBUG_PORT_PATTERN = /^--(?:debug|inspect)(?:-port|-brk)?=(\d{1,5})$/; -function getDefaultPort() { - for (const arg of process.execArgv) { - const match = arg.match(DEBUG_PORT_PATTERN); - if (match) { - return +match[1]; - } - } - return 9229; -} - function portIsFree(host, port, timeout = 2000) { + if (port === 0) return Promise.resolve(); // Binding to a random port. + const retryDelay = 150; let didTimeOut = false; @@ -110,9 +101,11 @@ function runScript(script, scriptArgs, inspectHost, inspectPort, childPrint) { let output = ''; function waitForListenHint(text) { output += text; - if (/Debugger listening on/.test(output)) { + if (/Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\//.test(output)) { + const host = RegExp.$1; + const port = Number.parseInt(RegExp.$2); child.stderr.removeListener('data', waitForListenHint); - resolve(child); + resolve([child, port, host]); } } @@ -160,10 +153,11 @@ class NodeInspector { options.port, this.childPrint.bind(this)); } else { - this._runScript = () => Promise.resolve(null); + this._runScript = + () => Promise.resolve([null, options.port, options.host]); } - this.client = new InspectClient(options.port, options.host); + this.client = new InspectClient(); this.domainNames = ['Debugger', 'HeapProfiler', 'Profiler', 'Runtime']; this.domainNames.forEach((domain) => { @@ -223,9 +217,8 @@ class NodeInspector { run() { this.killChild(); - const { host, port } = this.options; - return this._runScript().then((child) => { + return this._runScript().then(([child, port, host]) => { this.child = child; let connectionAttempts = 0; @@ -233,7 +226,7 @@ class NodeInspector { ++connectionAttempts; debuglog('connection attempt #%d', connectionAttempts); this.stdout.write('.'); - return this.client.connect() + return this.client.connect(port, host) .then(() => { debuglog('connection established'); this.stdout.write(' ok'); @@ -288,7 +281,7 @@ class NodeInspector { function parseArgv([target, ...args]) { let host = '127.0.0.1'; - let port = getDefaultPort(); + let port = 9229; let isRemote = false; let script = target; let scriptArgs = args; diff --git a/lib/internal/inspect_client.js b/lib/internal/inspect_client.js index c247e2a..9b8529d 100644 --- a/lib/internal/inspect_client.js +++ b/lib/internal/inspect_client.js @@ -164,12 +164,12 @@ function decodeFrameHybi17(data) { } class Client extends EventEmitter { - constructor(port, host) { + constructor() { super(); this.handleChunk = this._handleChunk.bind(this); - this._port = port; - this._host = host; + this._port = undefined; + this._host = undefined; this.reset(); } @@ -284,7 +284,9 @@ class Client extends EventEmitter { }); } - connect() { + connect(port, host) { + this._port = port; + this._host = host; return this._discoverWebsocketPath() .then((urlPath) => this._connectWebsocket(urlPath)); } diff --git a/test/cli/launch.test.js b/test/cli/launch.test.js index f7efc6e..8808d47 100644 --- a/test/cli/launch.test.js +++ b/test/cli/launch.test.js @@ -26,6 +26,46 @@ test('custom port', (t) => { }); }); +test('random port', (t) => { + const script = Path.join('examples', 'three-lines.js'); + + const cli = startCLI(['--port=0', script]); + + return cli.waitForInitialBreak() + .then(() => cli.waitForPrompt()) + .then(() => { + t.match(cli.output, 'debug>', 'prints a prompt'); + t.match( + cli.output, + /< Debugger listening on /, + 'forwards child output'); + }) + .then(() => cli.quit()) + .then((code) => { + t.equal(code, 0, 'exits with success'); + }); +}); + +test('random port with --inspect-port=0', (t) => { + const script = Path.join('examples', 'three-lines.js'); + + const cli = startCLI([script], ['--inspect-port=0']); + + return cli.waitForInitialBreak() + .then(() => cli.waitForPrompt()) + .then(() => { + t.match(cli.output, 'debug>', 'prints a prompt'); + t.match( + cli.output, + /< Debugger listening on /, + 'forwards child output'); + }) + .then(() => cli.quit()) + .then((code) => { + t.equal(code, 0, 'exits with success'); + }); +}); + test('examples/three-lines.js', (t) => { const script = Path.join('examples', 'three-lines.js'); const cli = startCLI([script]); diff --git a/test/cli/start-cli.js b/test/cli/start-cli.js index ae90430..b086dcd 100644 --- a/test/cli/start-cli.js +++ b/test/cli/start-cli.js @@ -16,8 +16,8 @@ const BREAK_MESSAGE = new RegExp('(?:' + [ 'exception', 'other', 'promiseRejection', ].join('|') + ') in', 'i'); -function startCLI(args) { - const child = spawn(process.execPath, [CLI, ...args]); +function startCLI(args, flags = []) { + const child = spawn(process.execPath, [...flags, CLI, ...args]); let isFirstStdoutChunk = true; const outputBuffer = [];