From 74051c64aac1f95d374ef36bc21f516d349c04cb Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 17 Jan 2018 00:35:54 +0200 Subject: [PATCH] inspector: --inspect-brk for es modules Reworked rebase of PR #17360 with feedback PR-URL: https://github.com/nodejs/node/pull/18194 Fixes: https://github.com/nodejs/node/issues/17340 Reviewed-By: Eugene Ostroukhov Reviewed-By: James M Snell --- lib/internal/loader/Loader.js | 11 +- lib/internal/loader/ModuleJob.js | 6 +- lib/module.js | 1 + test/common/inspector-helper.js | 43 +++++-- test/fixtures/es-modules/loop.mjs | 10 ++ ...est-inspect-async-hook-setup-at-inspect.js | 1 + test/parallel/test-inspector-esm.js | 120 ++++++++++++++++++ ...st-inspector-no-crash-ws-after-bindings.js | 1 + ...spector-async-hook-setup-at-inspect-brk.js | 1 + ...st-inspector-async-hook-setup-at-signal.js | 1 + ...spector-async-stack-traces-promise-then.js | 1 + ...spector-async-stack-traces-set-interval.js | 1 + test/sequential/test-inspector-break-e.js | 1 + .../test-inspector-break-when-eval.js | 1 + .../test-inspector-debug-brk-flag.js | 6 +- test/sequential/test-inspector-debug-end.js | 1 + test/sequential/test-inspector-exception.js | 1 + .../sequential/test-inspector-ip-detection.js | 1 + .../test-inspector-not-blocked-on-idle.js | 1 + .../test-inspector-scriptparsed-context.js | 1 + .../test-inspector-stop-profile-after-done.js | 1 + test/sequential/test-inspector.js | 13 +- 22 files changed, 202 insertions(+), 22 deletions(-) create mode 100644 test/fixtures/es-modules/loop.mjs create mode 100644 test/parallel/test-inspector-esm.js diff --git a/lib/internal/loader/Loader.js b/lib/internal/loader/Loader.js index 2e6a69af316261..eda42645f170f6 100644 --- a/lib/internal/loader/Loader.js +++ b/lib/internal/loader/Loader.js @@ -44,6 +44,7 @@ class Loader { throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'base', 'string'); this.base = base; + this.isMain = true; // methods which translate input code or other information // into es modules @@ -132,7 +133,15 @@ class Loader { loaderInstance = translators.get(format); } - job = new ModuleJob(this, url, loaderInstance); + let inspectBrk = false; + if (this.isMain) { + if (process._breakFirstLine) { + delete process._breakFirstLine; + inspectBrk = true; + } + this.isMain = false; + } + job = new ModuleJob(this, url, loaderInstance, inspectBrk); this.moduleMap.set(url, job); return job; } diff --git a/lib/internal/loader/ModuleJob.js b/lib/internal/loader/ModuleJob.js index 8aa1b4b0513500..2d6325b85c6322 100644 --- a/lib/internal/loader/ModuleJob.js +++ b/lib/internal/loader/ModuleJob.js @@ -14,7 +14,7 @@ const enableDebug = (process.env.NODE_DEBUG || '').match(/\besm\b/) || class ModuleJob { // `loader` is the Loader instance used for loading dependencies. // `moduleProvider` is a function - constructor(loader, url, moduleProvider) { + constructor(loader, url, moduleProvider, inspectBrk) { this.loader = loader; this.error = null; this.hadError = false; @@ -30,6 +30,10 @@ class ModuleJob { const dependencyJobs = []; ({ module: this.module, reflect: this.reflect } = await this.modulePromise); + if (inspectBrk) { + const initWrapper = process.binding('inspector').callAndPauseOnStart; + initWrapper(this.module.instantiate, this.module); + } assert(this.module instanceof ModuleWrap); this.module.link(async (dependencySpecifier) => { const dependencyJobPromise = diff --git a/lib/module.js b/lib/module.js index 63e878b17e0f4c..22a3e354c31617 100644 --- a/lib/module.js +++ b/lib/module.js @@ -464,6 +464,7 @@ Module._load = function(request, parent, isMain) { ESMLoader = new Loader(); const userLoader = process.binding('config').userLoader; if (userLoader) { + ESMLoader.isMain = false; const hooks = await ESMLoader.import(userLoader); ESMLoader = new Loader(); ESMLoader.hook(hooks); diff --git a/test/common/inspector-helper.js b/test/common/inspector-helper.js index 1f1738e31b75a8..5e927f215055b9 100644 --- a/test/common/inspector-helper.js +++ b/test/common/inspector-helper.js @@ -5,7 +5,9 @@ const fs = require('fs'); const http = require('http'); const fixtures = require('../common/fixtures'); const { spawn } = require('child_process'); -const url = require('url'); +const { URL, parse: parseURL } = require('url'); +const { getURLFromFilePath } = require('internal/url'); +const path = require('path'); const _MAINSCRIPT = fixtures.path('loop.js'); const DEBUG = false; @@ -171,8 +173,9 @@ class InspectorSession { const scriptId = script['scriptId']; const url = script['url']; this._scriptsIdsByUrl.set(scriptId, url); - if (url === _MAINSCRIPT) + if (getURLFromFilePath(url).toString() === this.scriptURL().toString()) { this.mainScriptId = scriptId; + } } if (this._notificationCallback) { @@ -238,11 +241,13 @@ class InspectorSession { return notification; } - _isBreakOnLineNotification(message, line, url) { + _isBreakOnLineNotification(message, line, expectedScriptPath) { if ('Debugger.paused' === message['method']) { const callFrame = message['params']['callFrames'][0]; const location = callFrame['location']; - assert.strictEqual(url, this._scriptsIdsByUrl.get(location['scriptId'])); + const scriptPath = this._scriptsIdsByUrl.get(location['scriptId']); + assert(scriptPath.toString() === expectedScriptPath.toString(), + `${scriptPath} !== ${expectedScriptPath}`); assert.strictEqual(line, location['lineNumber']); return true; } @@ -291,12 +296,26 @@ class InspectorSession { 'Waiting for the debugger to disconnect...'); await this.disconnect(); } + + scriptPath() { + return this._instance.scriptPath(); + } + + script() { + return this._instance.script(); + } + + scriptURL() { + return getURLFromFilePath(this.scriptPath()); + } } class NodeInstance { constructor(inspectorFlags = ['--inspect-brk=0'], scriptContents = '', scriptFile = _MAINSCRIPT) { + this._scriptPath = scriptFile; + this._script = scriptFile ? null : scriptContents; this._portCallback = null; this.portPromise = new Promise((resolve) => this._portCallback = resolve); this._process = spawnChildProcess(inspectorFlags, scriptContents, @@ -375,7 +394,7 @@ class NodeInstance { const port = await this.portPromise; return http.get({ port, - path: url.parse(devtoolsUrl).path, + path: parseURL(devtoolsUrl).path, headers: { 'Connection': 'Upgrade', 'Upgrade': 'websocket', @@ -425,10 +444,16 @@ class NodeInstance { kill() { this._process.kill(); } -} -function readMainScriptSource() { - return fs.readFileSync(_MAINSCRIPT, 'utf8'); + scriptPath() { + return this._scriptPath; + } + + script() { + if (this._script === null) + this._script = fs.readFileSync(this.scriptPath(), 'utf8'); + return this._script; + } } function onResolvedOrRejected(promise, callback) { @@ -469,7 +494,5 @@ function fires(promise, error, timeoutMs) { } module.exports = { - mainScriptPath: _MAINSCRIPT, - readMainScriptSource, NodeInstance }; diff --git a/test/fixtures/es-modules/loop.mjs b/test/fixtures/es-modules/loop.mjs new file mode 100644 index 00000000000000..33a382bdde7dab --- /dev/null +++ b/test/fixtures/es-modules/loop.mjs @@ -0,0 +1,10 @@ +var t = 1; +var k = 1; +console.log('A message', 5); +while (t > 0) { + if (t++ === 1000) { + t = 0; + console.log(`Outputed message #${k++}`); + } +} +process.exit(55); \ No newline at end of file diff --git a/test/parallel/test-inspect-async-hook-setup-at-inspect.js b/test/parallel/test-inspect-async-hook-setup-at-inspect.js index 869ec21ca991a4..6b1c875138ba01 100644 --- a/test/parallel/test-inspect-async-hook-setup-at-inspect.js +++ b/test/parallel/test-inspect-async-hook-setup-at-inspect.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/parallel/test-inspector-esm.js b/test/parallel/test-inspector-esm.js new file mode 100644 index 00000000000000..b5a1365212a4ac --- /dev/null +++ b/test/parallel/test-inspector-esm.js @@ -0,0 +1,120 @@ +// Flags: --expose-internals +'use strict'; +const common = require('../common'); + +common.skipIfInspectorDisabled(); + +const assert = require('assert'); +const fixtures = require('../common/fixtures'); +const { NodeInstance } = require('../common/inspector-helper.js'); + +function assertNoUrlsWhileConnected(response) { + assert.strictEqual(response.length, 1); + assert.ok(!response[0].hasOwnProperty('devtoolsFrontendUrl')); + assert.ok(!response[0].hasOwnProperty('webSocketDebuggerUrl')); +} + +function assertScopeValues({ result }, expected) { + const unmatched = new Set(Object.keys(expected)); + for (const actual of result) { + const value = expected[actual['name']]; + assert.strictEqual(actual['value']['value'], value); + unmatched.delete(actual['name']); + } + assert.deepStrictEqual(Array.from(unmatched.values()), []); +} + +async function testBreakpointOnStart(session) { + console.log('[test]', + 'Verifying debugger stops on start (--inspect-brk option)'); + const commands = [ + { 'method': 'Runtime.enable' }, + { 'method': 'Debugger.enable' }, + { 'method': 'Debugger.setPauseOnExceptions', + 'params': { 'state': 'none' } }, + { 'method': 'Debugger.setAsyncCallStackDepth', + 'params': { 'maxDepth': 0 } }, + { 'method': 'Profiler.enable' }, + { 'method': 'Profiler.setSamplingInterval', + 'params': { 'interval': 100 } }, + { 'method': 'Debugger.setBlackboxPatterns', + 'params': { 'patterns': [] } }, + { 'method': 'Runtime.runIfWaitingForDebugger' } + ]; + + await session.send(commands); + await session.waitForBreakOnLine(0, session.scriptURL()); +} + +async function testBreakpoint(session) { + console.log('[test]', 'Setting a breakpoint and verifying it is hit'); + const commands = [ + { 'method': 'Debugger.setBreakpointByUrl', + 'params': { 'lineNumber': 5, + 'url': session.scriptURL(), + 'columnNumber': 0, + 'condition': '' + } + }, + { 'method': 'Debugger.resume' }, + ]; + await session.send(commands); + const { scriptSource } = await session.send({ + 'method': 'Debugger.getScriptSource', + 'params': { 'scriptId': session.mainScriptId } }); + assert(scriptSource && (scriptSource.includes(session.script())), + `Script source is wrong: ${scriptSource}`); + + await session.waitForConsoleOutput('log', ['A message', 5]); + const paused = await session.waitForBreakOnLine(5, session.scriptURL()); + const scopeId = paused.params.callFrames[0].scopeChain[0].object.objectId; + + console.log('[test]', 'Verify we can read current application state'); + const response = await session.send({ + 'method': 'Runtime.getProperties', + 'params': { + 'objectId': scopeId, + 'ownProperties': false, + 'accessorPropertiesOnly': false, + 'generatePreview': true + } + }); + assertScopeValues(response, { t: 1001, k: 1 }); + + let { result } = await session.send({ + 'method': 'Debugger.evaluateOnCallFrame', 'params': { + 'callFrameId': '{"ordinal":0,"injectedScriptId":1}', + 'expression': 'k + t', + 'objectGroup': 'console', + 'includeCommandLineAPI': true, + 'silent': false, + 'returnByValue': false, + 'generatePreview': true + } + }); + + assert.strictEqual(result['value'], 1002); + + result = (await session.send({ + 'method': 'Runtime.evaluate', 'params': { + 'expression': '5 * 5' + } + })).result; + assert.strictEqual(result['value'], 25); +} + +async function runTest() { + const child = new NodeInstance(['--inspect-brk=0', '--experimental-modules'], + '', fixtures.path('es-modules/loop.mjs')); + + const session = await child.connectInspectorSession(); + assertNoUrlsWhileConnected(await child.httpGet(null, '/json/list')); + await testBreakpointOnStart(session); + await testBreakpoint(session); + await session.runToCompletion(); + assert.strictEqual((await child.expectShutdown()).exitCode, 55); +} + +common.crashOnUnhandledRejection(); + +runTest(); diff --git a/test/parallel/test-inspector-no-crash-ws-after-bindings.js b/test/parallel/test-inspector-no-crash-ws-after-bindings.js index 286373068e8e9b..15fa96952ed125 100644 --- a/test/parallel/test-inspector-no-crash-ws-after-bindings.js +++ b/test/parallel/test-inspector-no-crash-ws-after-bindings.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js b/test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js index 980e9e4d46e1a9..e0c3b4dcb86e37 100644 --- a/test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js +++ b/test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector-async-hook-setup-at-signal.js b/test/sequential/test-inspector-async-hook-setup-at-signal.js index 5ff7dec9473ac2..e0b87b0ebb162c 100644 --- a/test/sequential/test-inspector-async-hook-setup-at-signal.js +++ b/test/sequential/test-inspector-async-hook-setup-at-signal.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector-async-stack-traces-promise-then.js b/test/sequential/test-inspector-async-stack-traces-promise-then.js index 9ed61d5ae13675..b5cb651ff4efca 100644 --- a/test/sequential/test-inspector-async-stack-traces-promise-then.js +++ b/test/sequential/test-inspector-async-stack-traces-promise-then.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector-async-stack-traces-set-interval.js b/test/sequential/test-inspector-async-stack-traces-set-interval.js index e778bfc80280b0..326032d40b20e6 100644 --- a/test/sequential/test-inspector-async-stack-traces-set-interval.js +++ b/test/sequential/test-inspector-async-stack-traces-set-interval.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector-break-e.js b/test/sequential/test-inspector-break-e.js index 9ae47253f49484..8db403ad2cd0d0 100644 --- a/test/sequential/test-inspector-break-e.js +++ b/test/sequential/test-inspector-break-e.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); diff --git a/test/sequential/test-inspector-break-when-eval.js b/test/sequential/test-inspector-break-when-eval.js index e5d01cb189337a..b14e01a30185ed 100644 --- a/test/sequential/test-inspector-break-when-eval.js +++ b/test/sequential/test-inspector-break-when-eval.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector-debug-brk-flag.js b/test/sequential/test-inspector-debug-brk-flag.js index 235e7043d80f45..61eb4f97c655dc 100644 --- a/test/sequential/test-inspector-debug-brk-flag.js +++ b/test/sequential/test-inspector-debug-brk-flag.js @@ -1,11 +1,11 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); const assert = require('assert'); -const { mainScriptPath, - NodeInstance } = require('../common/inspector-helper.js'); +const { NodeInstance } = require('../common/inspector-helper.js'); async function testBreakpointOnStart(session) { const commands = [ @@ -24,7 +24,7 @@ async function testBreakpointOnStart(session) { ]; session.send(commands); - await session.waitForBreakOnLine(0, mainScriptPath); + await session.waitForBreakOnLine(0, session.scriptPath()); } async function runTests() { diff --git a/test/sequential/test-inspector-debug-end.js b/test/sequential/test-inspector-debug-end.js index effef9f23315b9..dadee26258d346 100644 --- a/test/sequential/test-inspector-debug-end.js +++ b/test/sequential/test-inspector-debug-end.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector-exception.js b/test/sequential/test-inspector-exception.js index 743742f50f1b20..3f83b37c7265a8 100644 --- a/test/sequential/test-inspector-exception.js +++ b/test/sequential/test-inspector-exception.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const fixtures = require('../common/fixtures'); diff --git a/test/sequential/test-inspector-ip-detection.js b/test/sequential/test-inspector-ip-detection.js index f7dee4494d3c03..a69a57f55fcbe3 100644 --- a/test/sequential/test-inspector-ip-detection.js +++ b/test/sequential/test-inspector-ip-detection.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); diff --git a/test/sequential/test-inspector-not-blocked-on-idle.js b/test/sequential/test-inspector-not-blocked-on-idle.js index 68e4eaaa57cd3f..3b1befe35df199 100644 --- a/test/sequential/test-inspector-not-blocked-on-idle.js +++ b/test/sequential/test-inspector-not-blocked-on-idle.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector-scriptparsed-context.js b/test/sequential/test-inspector-scriptparsed-context.js index f295d737da7e34..abffbfe5fc67f2 100644 --- a/test/sequential/test-inspector-scriptparsed-context.js +++ b/test/sequential/test-inspector-scriptparsed-context.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector-stop-profile-after-done.js b/test/sequential/test-inspector-stop-profile-after-done.js index d0285df175b66f..7069e490255ce5 100644 --- a/test/sequential/test-inspector-stop-profile-after-done.js +++ b/test/sequential/test-inspector-stop-profile-after-done.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); diff --git a/test/sequential/test-inspector.js b/test/sequential/test-inspector.js index 992a12e90229ed..c34c953006276a 100644 --- a/test/sequential/test-inspector.js +++ b/test/sequential/test-inspector.js @@ -1,12 +1,11 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); const assert = require('assert'); -const { mainScriptPath, - readMainScriptSource, - NodeInstance } = require('../common/inspector-helper.js'); +const { NodeInstance } = require('../common/inspector-helper.js'); function checkListResponse(response) { assert.strictEqual(1, response.length); @@ -75,7 +74,7 @@ async function testBreakpointOnStart(session) { ]; await session.send(commands); - await session.waitForBreakOnLine(0, mainScriptPath); + await session.waitForBreakOnLine(0, session.scriptPath()); } async function testBreakpoint(session) { @@ -83,7 +82,7 @@ async function testBreakpoint(session) { const commands = [ { 'method': 'Debugger.setBreakpointByUrl', 'params': { 'lineNumber': 5, - 'url': mainScriptPath, + 'url': session.scriptPath(), 'columnNumber': 0, 'condition': '' } @@ -94,11 +93,11 @@ async function testBreakpoint(session) { const { scriptSource } = await session.send({ 'method': 'Debugger.getScriptSource', 'params': { 'scriptId': session.mainScriptId } }); - assert(scriptSource && (scriptSource.includes(readMainScriptSource())), + assert(scriptSource && (scriptSource.includes(session.script())), `Script source is wrong: ${scriptSource}`); await session.waitForConsoleOutput('log', ['A message', 5]); - const paused = await session.waitForBreakOnLine(5, mainScriptPath); + const paused = await session.waitForBreakOnLine(5, session.scriptPath()); const scopeId = paused.params.callFrames[0].scopeChain[0].object.objectId; console.log('[test]', 'Verify we can read current application state');