Skip to content

Commit 74051c6

Browse files
guybedfordMylesBorins
authored andcommitted
inspector: --inspect-brk for es modules
Reworked rebase of PR #17360 with feedback PR-URL: #18194 Fixes: #17340 Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 9e7f863 commit 74051c6

22 files changed

+202
-22
lines changed

lib/internal/loader/Loader.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class Loader {
4444
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'base', 'string');
4545

4646
this.base = base;
47+
this.isMain = true;
4748

4849
// methods which translate input code or other information
4950
// into es modules
@@ -132,7 +133,15 @@ class Loader {
132133
loaderInstance = translators.get(format);
133134
}
134135

135-
job = new ModuleJob(this, url, loaderInstance);
136+
let inspectBrk = false;
137+
if (this.isMain) {
138+
if (process._breakFirstLine) {
139+
delete process._breakFirstLine;
140+
inspectBrk = true;
141+
}
142+
this.isMain = false;
143+
}
144+
job = new ModuleJob(this, url, loaderInstance, inspectBrk);
136145
this.moduleMap.set(url, job);
137146
return job;
138147
}

lib/internal/loader/ModuleJob.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const enableDebug = (process.env.NODE_DEBUG || '').match(/\besm\b/) ||
1414
class ModuleJob {
1515
// `loader` is the Loader instance used for loading dependencies.
1616
// `moduleProvider` is a function
17-
constructor(loader, url, moduleProvider) {
17+
constructor(loader, url, moduleProvider, inspectBrk) {
1818
this.loader = loader;
1919
this.error = null;
2020
this.hadError = false;
@@ -30,6 +30,10 @@ class ModuleJob {
3030
const dependencyJobs = [];
3131
({ module: this.module,
3232
reflect: this.reflect } = await this.modulePromise);
33+
if (inspectBrk) {
34+
const initWrapper = process.binding('inspector').callAndPauseOnStart;
35+
initWrapper(this.module.instantiate, this.module);
36+
}
3337
assert(this.module instanceof ModuleWrap);
3438
this.module.link(async (dependencySpecifier) => {
3539
const dependencyJobPromise =

lib/module.js

+1
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,7 @@ Module._load = function(request, parent, isMain) {
464464
ESMLoader = new Loader();
465465
const userLoader = process.binding('config').userLoader;
466466
if (userLoader) {
467+
ESMLoader.isMain = false;
467468
const hooks = await ESMLoader.import(userLoader);
468469
ESMLoader = new Loader();
469470
ESMLoader.hook(hooks);

test/common/inspector-helper.js

+33-10
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ const fs = require('fs');
55
const http = require('http');
66
const fixtures = require('../common/fixtures');
77
const { spawn } = require('child_process');
8-
const url = require('url');
8+
const { URL, parse: parseURL } = require('url');
9+
const { getURLFromFilePath } = require('internal/url');
10+
const path = require('path');
911

1012
const _MAINSCRIPT = fixtures.path('loop.js');
1113
const DEBUG = false;
@@ -171,8 +173,9 @@ class InspectorSession {
171173
const scriptId = script['scriptId'];
172174
const url = script['url'];
173175
this._scriptsIdsByUrl.set(scriptId, url);
174-
if (url === _MAINSCRIPT)
176+
if (getURLFromFilePath(url).toString() === this.scriptURL().toString()) {
175177
this.mainScriptId = scriptId;
178+
}
176179
}
177180

178181
if (this._notificationCallback) {
@@ -238,11 +241,13 @@ class InspectorSession {
238241
return notification;
239242
}
240243

241-
_isBreakOnLineNotification(message, line, url) {
244+
_isBreakOnLineNotification(message, line, expectedScriptPath) {
242245
if ('Debugger.paused' === message['method']) {
243246
const callFrame = message['params']['callFrames'][0];
244247
const location = callFrame['location'];
245-
assert.strictEqual(url, this._scriptsIdsByUrl.get(location['scriptId']));
248+
const scriptPath = this._scriptsIdsByUrl.get(location['scriptId']);
249+
assert(scriptPath.toString() === expectedScriptPath.toString(),
250+
`${scriptPath} !== ${expectedScriptPath}`);
246251
assert.strictEqual(line, location['lineNumber']);
247252
return true;
248253
}
@@ -291,12 +296,26 @@ class InspectorSession {
291296
'Waiting for the debugger to disconnect...');
292297
await this.disconnect();
293298
}
299+
300+
scriptPath() {
301+
return this._instance.scriptPath();
302+
}
303+
304+
script() {
305+
return this._instance.script();
306+
}
307+
308+
scriptURL() {
309+
return getURLFromFilePath(this.scriptPath());
310+
}
294311
}
295312

296313
class NodeInstance {
297314
constructor(inspectorFlags = ['--inspect-brk=0'],
298315
scriptContents = '',
299316
scriptFile = _MAINSCRIPT) {
317+
this._scriptPath = scriptFile;
318+
this._script = scriptFile ? null : scriptContents;
300319
this._portCallback = null;
301320
this.portPromise = new Promise((resolve) => this._portCallback = resolve);
302321
this._process = spawnChildProcess(inspectorFlags, scriptContents,
@@ -375,7 +394,7 @@ class NodeInstance {
375394
const port = await this.portPromise;
376395
return http.get({
377396
port,
378-
path: url.parse(devtoolsUrl).path,
397+
path: parseURL(devtoolsUrl).path,
379398
headers: {
380399
'Connection': 'Upgrade',
381400
'Upgrade': 'websocket',
@@ -425,10 +444,16 @@ class NodeInstance {
425444
kill() {
426445
this._process.kill();
427446
}
428-
}
429447

430-
function readMainScriptSource() {
431-
return fs.readFileSync(_MAINSCRIPT, 'utf8');
448+
scriptPath() {
449+
return this._scriptPath;
450+
}
451+
452+
script() {
453+
if (this._script === null)
454+
this._script = fs.readFileSync(this.scriptPath(), 'utf8');
455+
return this._script;
456+
}
432457
}
433458

434459
function onResolvedOrRejected(promise, callback) {
@@ -469,7 +494,5 @@ function fires(promise, error, timeoutMs) {
469494
}
470495

471496
module.exports = {
472-
mainScriptPath: _MAINSCRIPT,
473-
readMainScriptSource,
474497
NodeInstance
475498
};

test/fixtures/es-modules/loop.mjs

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
var t = 1;
2+
var k = 1;
3+
console.log('A message', 5);
4+
while (t > 0) {
5+
if (t++ === 1000) {
6+
t = 0;
7+
console.log(`Outputed message #${k++}`);
8+
}
9+
}
10+
process.exit(55);

test/parallel/test-inspect-async-hook-setup-at-inspect.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/parallel/test-inspector-esm.js

+120
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
const common = require('../common');
4+
5+
common.skipIfInspectorDisabled();
6+
7+
const assert = require('assert');
8+
const fixtures = require('../common/fixtures');
9+
const { NodeInstance } = require('../common/inspector-helper.js');
10+
11+
function assertNoUrlsWhileConnected(response) {
12+
assert.strictEqual(response.length, 1);
13+
assert.ok(!response[0].hasOwnProperty('devtoolsFrontendUrl'));
14+
assert.ok(!response[0].hasOwnProperty('webSocketDebuggerUrl'));
15+
}
16+
17+
function assertScopeValues({ result }, expected) {
18+
const unmatched = new Set(Object.keys(expected));
19+
for (const actual of result) {
20+
const value = expected[actual['name']];
21+
assert.strictEqual(actual['value']['value'], value);
22+
unmatched.delete(actual['name']);
23+
}
24+
assert.deepStrictEqual(Array.from(unmatched.values()), []);
25+
}
26+
27+
async function testBreakpointOnStart(session) {
28+
console.log('[test]',
29+
'Verifying debugger stops on start (--inspect-brk option)');
30+
const commands = [
31+
{ 'method': 'Runtime.enable' },
32+
{ 'method': 'Debugger.enable' },
33+
{ 'method': 'Debugger.setPauseOnExceptions',
34+
'params': { 'state': 'none' } },
35+
{ 'method': 'Debugger.setAsyncCallStackDepth',
36+
'params': { 'maxDepth': 0 } },
37+
{ 'method': 'Profiler.enable' },
38+
{ 'method': 'Profiler.setSamplingInterval',
39+
'params': { 'interval': 100 } },
40+
{ 'method': 'Debugger.setBlackboxPatterns',
41+
'params': { 'patterns': [] } },
42+
{ 'method': 'Runtime.runIfWaitingForDebugger' }
43+
];
44+
45+
await session.send(commands);
46+
await session.waitForBreakOnLine(0, session.scriptURL());
47+
}
48+
49+
async function testBreakpoint(session) {
50+
console.log('[test]', 'Setting a breakpoint and verifying it is hit');
51+
const commands = [
52+
{ 'method': 'Debugger.setBreakpointByUrl',
53+
'params': { 'lineNumber': 5,
54+
'url': session.scriptURL(),
55+
'columnNumber': 0,
56+
'condition': ''
57+
}
58+
},
59+
{ 'method': 'Debugger.resume' },
60+
];
61+
await session.send(commands);
62+
const { scriptSource } = await session.send({
63+
'method': 'Debugger.getScriptSource',
64+
'params': { 'scriptId': session.mainScriptId } });
65+
assert(scriptSource && (scriptSource.includes(session.script())),
66+
`Script source is wrong: ${scriptSource}`);
67+
68+
await session.waitForConsoleOutput('log', ['A message', 5]);
69+
const paused = await session.waitForBreakOnLine(5, session.scriptURL());
70+
const scopeId = paused.params.callFrames[0].scopeChain[0].object.objectId;
71+
72+
console.log('[test]', 'Verify we can read current application state');
73+
const response = await session.send({
74+
'method': 'Runtime.getProperties',
75+
'params': {
76+
'objectId': scopeId,
77+
'ownProperties': false,
78+
'accessorPropertiesOnly': false,
79+
'generatePreview': true
80+
}
81+
});
82+
assertScopeValues(response, { t: 1001, k: 1 });
83+
84+
let { result } = await session.send({
85+
'method': 'Debugger.evaluateOnCallFrame', 'params': {
86+
'callFrameId': '{"ordinal":0,"injectedScriptId":1}',
87+
'expression': 'k + t',
88+
'objectGroup': 'console',
89+
'includeCommandLineAPI': true,
90+
'silent': false,
91+
'returnByValue': false,
92+
'generatePreview': true
93+
}
94+
});
95+
96+
assert.strictEqual(result['value'], 1002);
97+
98+
result = (await session.send({
99+
'method': 'Runtime.evaluate', 'params': {
100+
'expression': '5 * 5'
101+
}
102+
})).result;
103+
assert.strictEqual(result['value'], 25);
104+
}
105+
106+
async function runTest() {
107+
const child = new NodeInstance(['--inspect-brk=0', '--experimental-modules'],
108+
'', fixtures.path('es-modules/loop.mjs'));
109+
110+
const session = await child.connectInspectorSession();
111+
assertNoUrlsWhileConnected(await child.httpGet(null, '/json/list'));
112+
await testBreakpointOnStart(session);
113+
await testBreakpoint(session);
114+
await session.runToCompletion();
115+
assert.strictEqual((await child.expectShutdown()).exitCode, 55);
116+
}
117+
118+
common.crashOnUnhandledRejection();
119+
120+
runTest();

test/parallel/test-inspector-no-crash-ws-after-bindings.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/sequential/test-inspector-async-hook-setup-at-inspect-brk.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/sequential/test-inspector-async-hook-setup-at-signal.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/sequential/test-inspector-async-stack-traces-promise-then.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/sequential/test-inspector-async-stack-traces-set-interval.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/sequential/test-inspector-break-e.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34

test/sequential/test-inspector-break-when-eval.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/sequential/test-inspector-debug-brk-flag.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34

45
common.skipIfInspectorDisabled();
56

67
const assert = require('assert');
7-
const { mainScriptPath,
8-
NodeInstance } = require('../common/inspector-helper.js');
8+
const { NodeInstance } = require('../common/inspector-helper.js');
99

1010
async function testBreakpointOnStart(session) {
1111
const commands = [
@@ -24,7 +24,7 @@ async function testBreakpointOnStart(session) {
2424
];
2525

2626
session.send(commands);
27-
await session.waitForBreakOnLine(0, mainScriptPath);
27+
await session.waitForBreakOnLine(0, session.scriptPath());
2828
}
2929

3030
async function runTests() {

test/sequential/test-inspector-debug-end.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/sequential/test-inspector-exception.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
const fixtures = require('../common/fixtures');

test/sequential/test-inspector-ip-detection.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34

test/sequential/test-inspector-not-blocked-on-idle.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/sequential/test-inspector-scriptparsed-context.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

test/sequential/test-inspector-stop-profile-after-done.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();

0 commit comments

Comments
 (0)