From 93a01d6b0144f4988eb5588ac4c050c67f642426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Fri, 8 Nov 2019 18:38:26 +0100 Subject: [PATCH 1/3] module: ignore resolution failures for inspect-brk The resolution for the main entry point may fail when the resolution requires a preloaded module to be executed first (for example when adding new extensions to the resolution process). Silently skipping such failures allow us to defer the resolution as long as needed without having any adverse change (since the main entry point won't resolve anyway if it really can't be resolved at all). --- lib/internal/modules/cjs/loader.js | 7 +++-- .../test-resolution-inspect-brk-main.ext | 0 .../test-resolution-inspect-brk-resolver.js | 5 ++++ .../sequential/test-resolution-inspect-brk.js | 29 +++++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/test-resolution-inspect-brk-main.ext create mode 100644 test/fixtures/test-resolution-inspect-brk-resolver.js create mode 100644 test/sequential/test-resolution-inspect-brk.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 8cec27e6c4229b..b9c821cfe10e16 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1091,14 +1091,17 @@ Module.prototype._compile = function(content, filename) { if (!resolvedArgv) { // We enter the repl if we're not given a filename argument. if (process.argv[1]) { - resolvedArgv = Module._resolveFilename(process.argv[1], null, false); + // The resolution may fail if it requires a preload script + try { + resolvedArgv = Module._resolveFilename(process.argv[1], null, false); + } catch {} } else { resolvedArgv = 'repl'; } } // Set breakpoint on module start - if (!hasPausedEntry && filename === resolvedArgv) { + if (resolvedArgv && !hasPausedEntry && filename === resolvedArgv) { hasPausedEntry = true; inspectorWrapper = internalBinding('inspector').callAndPauseOnStart; } diff --git a/test/fixtures/test-resolution-inspect-brk-main.ext b/test/fixtures/test-resolution-inspect-brk-main.ext new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/test/fixtures/test-resolution-inspect-brk-resolver.js b/test/fixtures/test-resolution-inspect-brk-resolver.js new file mode 100644 index 00000000000000..fdfb5ca5b170c2 --- /dev/null +++ b/test/fixtures/test-resolution-inspect-brk-resolver.js @@ -0,0 +1,5 @@ +'use strict'; +// eslint-disable-next-line no-unused-vars +const common = require('../common'); + +require.extensions['.ext'] = require.extensions['.js']; diff --git a/test/sequential/test-resolution-inspect-brk.js b/test/sequential/test-resolution-inspect-brk.js new file mode 100644 index 00000000000000..2af32426c03f57 --- /dev/null +++ b/test/sequential/test-resolution-inspect-brk.js @@ -0,0 +1,29 @@ +'use strict'; +const common = require('../common'); +common.skipIfInspectorDisabled(); + +// A test to ensure that preload modules are given a chance to execute before +// resolving the main entry point with --inspect-brk active. + +const assert = require('assert'); +const cp = require('child_process'); +const path = require('path'); + +function test(execArgv) { + const child = cp.spawn(process.execPath, execArgv); + + child.stderr.once('data', common.mustCall(function() { + child.kill('SIGTERM'); + })); + + child.on('exit', common.mustCall(function(code, signal) { + assert.strictEqual(signal, 'SIGTERM'); + })); +} + +test([ + '--require', + path.join(__dirname, '../fixtures/test-resolution-inspect-brk-resolver.js'), + '--inspect-brk', + '../fixtures/test-resolution-inspect-resolver-main.ext', +]); From 3395d159a2615e08b7f3c5f7fc33a87bd8e9d695 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Tue, 19 Nov 2019 15:05:04 -0500 Subject: [PATCH 2/3] Update loader.js --- lib/internal/modules/cjs/loader.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index b9c821cfe10e16..7ace3723a1c9d2 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1094,7 +1094,11 @@ Module.prototype._compile = function(content, filename) { // The resolution may fail if it requires a preload script try { resolvedArgv = Module._resolveFilename(process.argv[1], null, false); - } catch {} + } catch { + // We only expect this codepath to be reached in the case of a + // preloaded module (it will fail earlier with the main entry) + assert(Array.isArray(getOptionValue('--require'))); + } } else { resolvedArgv = 'repl'; } From 9b8f623fbbd6cf752039cc0297e591e97eb78c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Wed, 20 Nov 2019 12:05:48 -0500 Subject: [PATCH 3/3] Update loader.js --- lib/internal/modules/cjs/loader.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 7ace3723a1c9d2..a69978b8231c75 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1091,7 +1091,6 @@ Module.prototype._compile = function(content, filename) { if (!resolvedArgv) { // We enter the repl if we're not given a filename argument. if (process.argv[1]) { - // The resolution may fail if it requires a preload script try { resolvedArgv = Module._resolveFilename(process.argv[1], null, false); } catch {