inspector: --inspect-brk for es modules

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>
MayaLekova · May 8, 2018 · 5fbf407 · 5fbf407
1 parent 8392f00
commit 5fbf407
Show file tree

Hide file tree

Showing 22 changed files with 202 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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);

diff --git 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
@@ -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);
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,3 +1,4 @@
+// Flags: --expose-internals
 'use strict';
 const common = require('../common');
 common.skipIfInspectorDisabled();