Skip to content

Commit

Permalink
inspector: --inspect-brk for es modules
Browse files Browse the repository at this point in the history
Reworked rebase of PR nodejs#17360 with feedback

PR-URL: nodejs#18194
Fixes: nodejs#17340
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
guybedford authored and MayaLekova committed May 8, 2018
1 parent 8392f00 commit 5fbf407
Show file tree
Hide file tree
Showing 22 changed files with 202 additions and 22 deletions.
11 changes: 10 additions & 1 deletion lib/internal/loader/Loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion lib/internal/loader/ModuleJob.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 =
Expand Down
1 change: 1 addition & 0 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,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);
Expand Down
43 changes: 33 additions & 10 deletions test/common/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -469,7 +494,5 @@ function fires(promise, error, timeoutMs) {
}

module.exports = {
mainScriptPath: _MAINSCRIPT,
readMainScriptSource,
NodeInstance
};
10 changes: 10 additions & 0 deletions test/fixtures/es-modules/loop.mjs
Original file line number Diff line number Diff line change
@@ -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);
1 change: 1 addition & 0 deletions test/parallel/test-inspect-async-hook-setup-at-inspect.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
120 changes: 120 additions & 0 deletions test/parallel/test-inspector-esm.js
Original file line number Diff line number Diff line change
@@ -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();
1 change: 1 addition & 0 deletions test/parallel/test-inspector-no-crash-ws-after-bindings.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
1 change: 1 addition & 0 deletions test/sequential/test-inspector-break-e.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');

Expand Down
1 change: 1 addition & 0 deletions test/sequential/test-inspector-break-when-eval.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
6 changes: 3 additions & 3 deletions test/sequential/test-inspector-debug-brk-flag.js
Original file line number Diff line number Diff line change
@@ -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 = [
Expand All @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions test/sequential/test-inspector-debug-end.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
1 change: 1 addition & 0 deletions test/sequential/test-inspector-exception.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
const fixtures = require('../common/fixtures');
Expand Down
1 change: 1 addition & 0 deletions test/sequential/test-inspector-ip-detection.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');

Expand Down
1 change: 1 addition & 0 deletions test/sequential/test-inspector-not-blocked-on-idle.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
1 change: 1 addition & 0 deletions test/sequential/test-inspector-scriptparsed-context.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
1 change: 1 addition & 0 deletions test/sequential/test-inspector-stop-profile-after-done.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
Expand Down
Loading

0 comments on commit 5fbf407

Please sign in to comment.