Skip to content

Commit 3d5b4cd

Browse files
committed
module: add requireStack to all error paths
Previously, `requireStack` was only set on the final fallback error in `Module._resolveFilename`. Errors thrown earlier in the resolution pipeline β€” from `tryPackage` (via `Module._findPath`) when a `package.json` `main` field points to a missing file, from `trySelf` during self-referential package resolution, and from `createEsmNotFoundErr` β€” did not include `requireStack`. Fix this by building `requireStack` from the parent chain before any resolution attempt and propagating it to all `MODULE_NOT_FOUND` error paths: - Wrap `trySelf` and `Module._findPath` calls in try-catch blocks that attach `requireStack` to any thrown `MODULE_NOT_FOUND` error that does not already have one. - Add an optional `requireStack` parameter to `createEsmNotFoundErr` and pass it from `_resolveFilename` where the error is thrown directly. Signed-off-by: sangwook <rewq5991@gmail.com>
1 parent 8d3245e commit 3d5b4cd

3 files changed

Lines changed: 35 additions & 14 deletions

File tree

β€Žlib/internal/modules/cjs/loader.jsβ€Ž

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,6 @@ function tryPackage(requestPath, exts, isMain, originalPath) {
562562
err.code = 'MODULE_NOT_FOUND';
563563
err.path = pjsonPath;
564564
err.requestPath = originalPath;
565-
// TODO(BridgeAR): Add the requireStack as well.
566565
throw err;
567566
} else {
568567
process.emitWarning(
@@ -1475,6 +1474,14 @@ Module._resolveFilename = function(request, parent, isMain, options) {
14751474

14761475
const parentPath = trySelfParentPath(parent);
14771476

1477+
const requireStack = [];
1478+
for (let cursor = parent;
1479+
cursor;
1480+
// TODO(joyeecheung): it makes more sense to use kLastModuleParent here.
1481+
cursor = cursor[kFirstModuleParent]) {
1482+
ArrayPrototypePush(requireStack, cursor.filename || cursor.id);
1483+
}
1484+
14781485
if (request[0] === '#' && (parent.filename || parent.id === '<repl>')) {
14791486
const pkg = packageJsonReader.getNearestParentPackageJSON(parentPath);
14801487
if (pkg?.data.imports != null) {
@@ -1487,15 +1494,23 @@ Module._resolveFilename = function(request, parent, isMain, options) {
14871494
);
14881495
} catch (e) {
14891496
if (e.code === 'ERR_MODULE_NOT_FOUND') {
1490-
throw createEsmNotFoundErr(request);
1497+
throw createEsmNotFoundErr(request, undefined, requireStack);
14911498
}
14921499
throw e;
14931500
}
14941501
}
14951502
}
14961503

14971504
// Try module self resolution first
1498-
const selfResolved = trySelf(parentPath, request, conditions);
1505+
let selfResolved;
1506+
try {
1507+
selfResolved = trySelf(parentPath, request, conditions);
1508+
} catch (e) {
1509+
if (e.code === 'MODULE_NOT_FOUND' && e.requireStack === undefined) {
1510+
e.requireStack = requireStack;
1511+
}
1512+
throw e;
1513+
}
14991514
if (selfResolved) {
15001515
const cacheKey = request + '\x00' +
15011516
(paths.length === 1 ? paths[0] : ArrayPrototypeJoin(paths, '\x00'));
@@ -1504,15 +1519,16 @@ Module._resolveFilename = function(request, parent, isMain, options) {
15041519
}
15051520

15061521
// Look up the filename first, since that's the cache key.
1507-
const filename = Module._findPath(request, paths, isMain, conditions);
1508-
if (filename) { return filename; }
1509-
const requireStack = [];
1510-
for (let cursor = parent;
1511-
cursor;
1512-
// TODO(joyeecheung): it makes more sense to use kLastModuleParent here.
1513-
cursor = cursor[kFirstModuleParent]) {
1514-
ArrayPrototypePush(requireStack, cursor.filename || cursor.id);
1522+
let filename;
1523+
try {
1524+
filename = Module._findPath(request, paths, isMain, conditions);
1525+
} catch (e) {
1526+
if (e.code === 'MODULE_NOT_FOUND' && e.requireStack === undefined) {
1527+
e.requireStack = requireStack;
1528+
}
1529+
throw e;
15151530
}
1531+
if (filename) { return filename; }
15161532
let message = `Cannot find module '${request}'`;
15171533
if (requireStack.length > 0) {
15181534
message = message + '\nRequire stack:\n- ' +
@@ -1552,16 +1568,19 @@ function finalizeEsmResolution(resolved, parentPath, pkgPath) {
15521568
* Creates an error object for when a requested ES module cannot be found.
15531569
* @param {string} request The name of the requested module
15541570
* @param {string} [path] The path to the requested module
1571+
* @param {string[]} [requireStack] The require stack at the time of the error
15551572
* @returns {Error}
15561573
*/
1557-
function createEsmNotFoundErr(request, path) {
1574+
function createEsmNotFoundErr(request, path, requireStack) {
15581575
// eslint-disable-next-line no-restricted-syntax
15591576
const err = new Error(`Cannot find module '${request}'`);
15601577
err.code = 'MODULE_NOT_FOUND';
15611578
if (path) {
15621579
err.path = path;
15631580
}
1564-
// TODO(BridgeAR): Add the requireStack as well.
1581+
if (requireStack) {
1582+
err.requireStack = requireStack;
1583+
}
15651584
return err;
15661585
}
15671586

β€Žtest/parallel/test-module-loading-error.jsβ€Ž

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ assert.throws(
9191
() => { require('../fixtures/packages/is-dir'); },
9292
common.isAIX ? { code: 'ERR_INVALID_PACKAGE_CONFIG' } : {
9393
code: 'MODULE_NOT_FOUND',
94-
message: /Cannot find module '\.\.\/fixtures\/packages\/is-dir'/
94+
message: /Cannot find module '\.\.\/fixtures\/packages\/is-dir'/,
95+
requireStack: [__filename],
9596
}
9697
);

β€Žtest/sequential/test-module-loading.jsβ€Ž

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ assert.throws(
123123
message: /packages[/\\]missing-main-no-index[/\\]doesnotexist\.js'\. Please.+package\.json.+valid "main"/,
124124
path: /fixtures[/\\]packages[/\\]missing-main-no-index[/\\]package\.json/,
125125
requestPath: /^\.\.[/\\]fixtures[/\\]packages[/\\]missing-main-no-index$/,
126+
requireStack: [__filename],
126127
}
127128
);
128129

0 commit comments

Comments
Β (0)