From 1549c8e077422b79573936960daaa4c695620310 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] 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). PR-URL: https://github.com/nodejs/node/pull/30336 Reviewed-By: Guy Bedford Reviewed-By: Joyee Cheung --- lib/internal/modules/cjs/loader.js | 10 +++++-- .../test-resolution-inspect-brk-main.ext | 0 .../test-resolution-inspect-brk-resolver.js | 5 ++++ .../sequential/test-resolution-inspect-brk.js | 29 +++++++++++++++++++ 4 files changed, 42 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 84b2b4772b5..687d39563d1 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1164,14 +1164,20 @@ 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); + try { + resolvedArgv = Module._resolveFilename(process.argv[1], null, false); + } 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'; } } // 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 00000000000..e69de29bb2d 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 00000000000..fdfb5ca5b17 --- /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 00000000000..2af32426c03 --- /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', +]);