From 2a35e628ed1dc5ca3c11f41bc813eaad6ef38037 Mon Sep 17 00:00:00 2001 From: Jan Krems Date: Sun, 17 May 2020 11:19:05 -0700 Subject: [PATCH] deps: update node-inspect to v2.0.0 Highlights: * Remove use of `process.binding` on modern node (@addaleax) * Increase timeout for port checking (@yilmazdurmaz) * Auto-resume on start when `NODE_INSPECT_RESUME_ON_START` is set (@dolsem) Compare: https://github.com/nodejs/node-inspect/compare/v1.11.6...v2.0.0 PR-URL: https://github.com/nodejs/node/pull/33447 Reviewed-By: Anna Henningsen Reviewed-By: Matheus Marchini Reviewed-By: Ben Noordhuis --- deps/node-inspect/.github/workflows/ci.yml | 31 ++++++++ deps/node-inspect/lib/_inspect.js | 40 +++++------ .../node-inspect/lib/internal/inspect_repl.js | 19 ++++- deps/node-inspect/package.json | 4 +- deps/node-inspect/test/cli/address.test.js | 71 +++++++++++++++++++ .../test/cli/invalid-args.test.js | 2 +- deps/node-inspect/test/cli/launch.test.js | 20 ++++++ deps/node-inspect/test/cli/start-cli.js | 8 +-- 8 files changed, 166 insertions(+), 29 deletions(-) create mode 100644 deps/node-inspect/.github/workflows/ci.yml create mode 100644 deps/node-inspect/test/cli/address.test.js diff --git a/deps/node-inspect/.github/workflows/ci.yml b/deps/node-inspect/.github/workflows/ci.yml new file mode 100644 index 00000000000000..968316a34779a5 --- /dev/null +++ b/deps/node-inspect/.github/workflows/ci.yml @@ -0,0 +1,31 @@ +name: Node CI + +on: [push, pull_request] + +jobs: + build: + + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + node-version: + # See https://github.com/nodejs/node-inspect/pull/78 + # - 10.x + - 12.x + - 13.x + + steps: + - uses: actions/checkout@v2 + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node-version }} + - name: npm install, build, and test + run: | + npm install + npm run build --if-present + npm test + env: + CI: true diff --git a/deps/node-inspect/lib/_inspect.js b/deps/node-inspect/lib/_inspect.js index 4d931f7a2bc664..aac278db0a891e 100644 --- a/deps/node-inspect/lib/_inspect.js +++ b/deps/node-inspect/lib/_inspect.js @@ -30,15 +30,15 @@ const runAsStandalone = typeof __dirname !== 'undefined'; const [ InspectClient, createRepl ] = runAsStandalone ? // This copy of node-inspect is on-disk, relative paths make sense. - [ - require('./internal/inspect_client'), - require('./internal/inspect_repl') - ] : + [ + require('./internal/inspect_client'), + require('./internal/inspect_repl') + ] : // This copy of node-inspect is built into the node executable. - [ - require('node-inspect/lib/internal/inspect_client'), - require('node-inspect/lib/internal/inspect_repl') - ]; + [ + require('node-inspect/lib/internal/inspect_client'), + require('node-inspect/lib/internal/inspect_repl') + ]; const debuglog = util.debuglog('inspect'); @@ -49,8 +49,8 @@ class StartupError extends Error { } } -function portIsFree(host, port, timeout = 2000) { - if (port === 0) return Promise.resolve(); // Binding to a random port. +function portIsFree(host, port, timeout = 9999) { + if (port === 0) return Promise.resolve(); // Binding to a random port. const retryDelay = 150; let didTimeOut = false; @@ -96,9 +96,9 @@ function runScript(script, scriptArgs, inspectHost, inspectPort, childPrint) { return new Promise((resolve) => { const needDebugBrk = process.version.match(/^v(6|7)\./); const args = (needDebugBrk ? - ['--inspect', `--debug-brk=${inspectPort}`] : - [`--inspect-brk=${inspectPort}`]) - .concat([script], scriptArgs); + ['--inspect', `--debug-brk=${inspectPort}`] : + [`--inspect-brk=${inspectPort}`]) + .concat([script], scriptArgs); const child = spawn(process.execPath, args); child.stdout.setEncoding('utf8'); child.stderr.setEncoding('utf8'); @@ -154,11 +154,11 @@ class NodeInspector { if (options.script) { this._runScript = runScript.bind(null, - options.script, - options.scriptArgs, - options.host, - options.port, - this.childPrint.bind(this)); + options.script, + options.scriptArgs, + options.host, + options.port, + this.childPrint.bind(this)); } else { this._runScript = () => Promise.resolve([null, options.port, options.host]); @@ -333,8 +333,8 @@ function parseArgv([target, ...args]) { } function startInspect(argv = process.argv.slice(2), - stdin = process.stdin, - stdout = process.stdout) { + stdin = process.stdin, + stdout = process.stdout) { /* eslint-disable no-console */ if (argv.length < 1) { const invokedAs = runAsStandalone ? diff --git a/deps/node-inspect/lib/internal/inspect_repl.js b/deps/node-inspect/lib/internal/inspect_repl.js index d9d3f89f03a408..bfbedf66a71b79 100644 --- a/deps/node-inspect/lib/internal/inspect_repl.js +++ b/deps/node-inspect/lib/internal/inspect_repl.js @@ -85,9 +85,16 @@ function extractFunctionName(description) { return fnNameMatch ? `: ${fnNameMatch[1]}` : ''; } -const NATIVES = process.binding('natives'); +const PUBLIC_BUILTINS = require('module').builtinModules; +const NATIVES = PUBLIC_BUILTINS ? process.binding('natives') : {}; function isNativeUrl(url) { - return url.replace('.js', '') in NATIVES || url === 'bootstrap_node.js'; + url = url.replace(/\.js$/, ''); + if (PUBLIC_BUILTINS) { + if (url.startsWith('internal/') || PUBLIC_BUILTINS.includes(url)) + return true; + } + + return url in NATIVES || url === 'bootstrap_node'; } function getRelativePath(filenameOrURL) { @@ -775,6 +782,14 @@ function createRepl(inspector) { } Debugger.on('paused', ({ callFrames, reason /* , hitBreakpoints */ }) => { + if (process.env.NODE_INSPECT_RESUME_ON_START === '1' && + reason === 'Break on start') { + debuglog('Paused on start, but NODE_INSPECT_RESUME_ON_START' + + ' environment variable is set to 1, resuming'); + inspector.client.callMethod('Debugger.resume'); + return; + } + // Save execution context's data currentBacktrace = Backtrace.from(callFrames); selectedFrame = currentBacktrace[0]; diff --git a/deps/node-inspect/package.json b/deps/node-inspect/package.json index c64582703cc6d9..925fb03f21a53d 100644 --- a/deps/node-inspect/package.json +++ b/deps/node-inspect/package.json @@ -1,6 +1,6 @@ { "name": "node-inspect", - "version": "1.11.6", + "version": "2.0.0", "description": "Node Inspect", "license": "MIT", "main": "lib/_inspect.js", @@ -27,7 +27,7 @@ }, "dependencies": {}, "devDependencies": { - "eslint": "^3.10.2", + "eslint": "^6.8.0", "nlm": "^3.0.0", "tap": "^10.7.0" }, diff --git a/deps/node-inspect/test/cli/address.test.js b/deps/node-inspect/test/cli/address.test.js new file mode 100644 index 00000000000000..1dbe4f37b42175 --- /dev/null +++ b/deps/node-inspect/test/cli/address.test.js @@ -0,0 +1,71 @@ +'use strict'; +const { spawn } = require('child_process'); +const Path = require('path'); +const { test } = require('tap'); + +const startCLI = require('./start-cli'); + +// NOTE(oyyd): We might want to import this regexp from "lib/_inspect.js"? +const kDebuggerMsgReg = /Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\//; + +function launchTarget(...args) { + const childProc = spawn(process.execPath, args); + return new Promise((resolve, reject) => { + const onExit = () => { + reject(new Error('Child process exits unexpectly')); + }; + childProc.on('exit', onExit); + childProc.stderr.setEncoding('utf8'); + childProc.stderr.on('data', (data) => { + const ret = kDebuggerMsgReg.exec(data); + childProc.removeListener('exit', onExit); + if (ret) { + resolve({ + childProc, + host: ret[1], + port: ret[2], + }); + } + }); + }); +} + +// process.debugPort is our proxy for "the version of node used to run this +// test suite doesn't support SIGUSR1 for enabling --inspect for a process". +const defaultsToOldProtocol = process.debugPort === 5858; + +test('examples/alive.js', { skip: defaultsToOldProtocol }, (t) => { + const script = Path.join('examples', 'alive.js'); + let cli = null; + let target = null; + + function cleanup(error) { + if (cli) { + cli.quit(); + cli = null; + } + if (target) { + target.kill(); + target = null; + } + if (error) throw error; + } + + return launchTarget('--inspect=0', script) + .then(({ childProc, host, port }) => { + target = childProc; + cli = startCLI([`${host || '127.0.0.1'}:${port}`]); + return cli.waitForPrompt(); + }) + .then(() => cli.command('sb("alive.js", 3)')) + .then(() => cli.waitFor(/break/)) + .then(() => cli.waitForPrompt()) + .then(() => { + t.match( + cli.output, + '> 3 ++x;', + 'marks the 3rd line'); + }) + .then(() => cleanup()) + .then(null, cleanup); +}); diff --git a/deps/node-inspect/test/cli/invalid-args.test.js b/deps/node-inspect/test/cli/invalid-args.test.js index c1aaeb6a9ce750..86428a3ec27030 100644 --- a/deps/node-inspect/test/cli/invalid-args.test.js +++ b/deps/node-inspect/test/cli/invalid-args.test.js @@ -27,7 +27,7 @@ test('launch w/ invalid host:port', (t) => { }); }); -test('launch w/ unavailable port', async (t) => { +test('launch w/ unavailable port', async(t) => { const blocker = createServer((socket) => socket.end()); const port = await new Promise((resolve, reject) => { blocker.on('error', reject); diff --git a/deps/node-inspect/test/cli/launch.test.js b/deps/node-inspect/test/cli/launch.test.js index 087a46c54bb8c6..c4ff3d855a82bc 100644 --- a/deps/node-inspect/test/cli/launch.test.js +++ b/deps/node-inspect/test/cli/launch.test.js @@ -174,3 +174,23 @@ test('run after quit / restart', (t) => { .then(() => cli.quit()) .then(null, onFatal); }); + +test('auto-resume on start if the environment variable is defined', (t) => { + const script = Path.join('examples', 'break.js'); + + const cli = startCLI([script], [], { + env: { NODE_INSPECT_RESUME_ON_START: '1' } + }); + + return cli.waitForInitialBreak() + .then(() => { + t.match( + cli.breakInfo, + { filename: script, line: 10 }, + 'skips to the first breakpoint'); + }) + .then(() => cli.quit()) + .then((code) => { + t.equal(code, 0, 'exits with success'); + }); +}); diff --git a/deps/node-inspect/test/cli/start-cli.js b/deps/node-inspect/test/cli/start-cli.js index e2fa5fe47c9ebe..32c666c7647c68 100644 --- a/deps/node-inspect/test/cli/start-cli.js +++ b/deps/node-inspect/test/cli/start-cli.js @@ -8,8 +8,8 @@ tap.test('startCLI', (t) => t.end()); const CLI = process.env.USE_EMBEDDED_NODE_INSPECT === '1' ? - 'inspect' : - require.resolve('../../cli.js'); + 'inspect' : + require.resolve('../../cli.js'); const BREAK_MESSAGE = new RegExp('(?:' + [ 'assert', 'break', 'break on start', 'debugCommand', @@ -20,8 +20,8 @@ function isPreBreak(output) { return /Break on start/.test(output) && /1 \(function \(exports/.test(output); } -function startCLI(args, flags = []) { - const child = spawn(process.execPath, [...flags, CLI, ...args]); +function startCLI(args, flags = [], spawnOpts = {}) { + const child = spawn(process.execPath, [...flags, CLI, ...args], spawnOpts); let isFirstStdoutChunk = true; const outputBuffer = [];