Skip to content

Commit

Permalink
lib: improve error message for MODULE_NOT_FOUND
Browse files Browse the repository at this point in the history
Include the require stack in the reported error message.

PR-URL: #25690
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
ofrobots committed Feb 15, 2019
1 parent 05cd1a0 commit a02e3e2
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 20 deletions.
6 changes: 5 additions & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -612,8 +612,12 @@ Module._resolveFilename = function(request, parent, isMain, options) {
cursor = cursor.parent) {
requireStack.push(cursor.filename || cursor.id);
}
let message = `Cannot find module '${request}'`;
if (requireStack.length > 0) {
message = message + '\nRequire stack:\n- ' + requireStack.join('\n- ');
}
// eslint-disable-next-line no-restricted-syntax
var err = new Error(`Cannot find module '${request}'`);
var err = new Error(message);
err.code = 'MODULE_NOT_FOUND';
err.requireStack = requireStack;
throw err;
Expand Down
6 changes: 3 additions & 3 deletions test/fixtures/require-resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,21 @@ assert.strictEqual(
// Verify that existing paths are removed.
assert.throws(() => {
require.resolve('bar', { paths: [] })
}, /^Error: Cannot find module 'bar'$/);
}, /^Error: Cannot find module 'bar'/);

// Verify that resolution path can be overwritten.
{
// three.js cannot be loaded from this file by default.
assert.throws(() => {
require.resolve('three')
}, /^Error: Cannot find module 'three'$/);
}, /^Error: Cannot find module 'three'/);

// If the nested-index directory is provided as a resolve path, 'three'
// cannot be found because nested-index is used as a starting point and not
// a searched directory.
assert.throws(() => {
require.resolve('three', { paths: [nestedIndex] })
}, /^Error: Cannot find module 'three'$/);
}, /^Error: Cannot find module 'three'/);

// Resolution from nested index directory also checks node_modules.
assert.strictEqual(
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-internal-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const assert = require('assert');

assert.throws(function() {
require('internal/freelist');
}, /^Error: Cannot find module 'internal\/freelist'$/);
}, /^Error: Cannot find module 'internal\/freelist'/);

assert.strictEqual(
require(fixtures.path('internal-modules')),
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-loaders-hidden-from-users.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ common.expectsError(
require('internal/bootstrap/loaders');
}, {
code: 'MODULE_NOT_FOUND',
message: 'Cannot find module \'internal/bootstrap/loaders\''
message: /Cannot find module 'internal\/bootstrap\/loaders'/
}
);

Expand All @@ -20,6 +20,6 @@ common.expectsError(
require('owo');
}, {
code: 'MODULE_NOT_FOUND',
message: 'Cannot find module \'owo\''
message: /Cannot find module 'owo'/
}
);
7 changes: 4 additions & 3 deletions test/parallel/test-module-loading-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ common.expectsError(
message: 'The argument \'id\' must be a non-empty string. Received \'\''
});

common.expectsError(
assert.throws(
() => { require('../fixtures/packages/is-dir'); },
{
code: 'MODULE_NOT_FOUND',
message: 'Cannot find module \'../fixtures/packages/is-dir\''
});
message: /Cannot find module '\.\.\/fixtures\/packages\/is-dir'/
}
);
6 changes: 3 additions & 3 deletions test/parallel/test-module-multi-extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fs.writeFileSync(dotfileWithExtension, 'console.log(__filename);', 'utf8');
require(modulePath);
assert.throws(
() => require(`${modulePath}.foo`),
new Error(`Cannot find module '${modulePath}.foo'`)
(err) => err.message.startsWith(`Cannot find module '${modulePath}.foo'`)
);
require(`${modulePath}.foo.bar`);
delete require.cache[file];
Expand All @@ -47,7 +47,7 @@ fs.writeFileSync(dotfileWithExtension, 'console.log(__filename);', 'utf8');
const modulePath = path.join(tmpdir.path, 'test-extensions');
assert.throws(
() => require(modulePath),
new Error(`Cannot find module '${modulePath}'`)
(err) => err.message.startsWith(`Cannot find module '${modulePath}'`)
);
delete require.cache[file];
Module._pathCache = Object.create(null);
Expand All @@ -69,7 +69,7 @@ fs.writeFileSync(dotfileWithExtension, 'console.log(__filename);', 'utf8');
const modulePath = path.join(tmpdir.path, 'test-extensions.foo');
assert.throws(
() => require(modulePath),
new Error(`Cannot find module '${modulePath}'`)
(err) => err.message.startsWith(`Cannot find module '${modulePath}'`)
);
delete require.extensions['.foo.bar'];
Module._pathCache = Object.create(null);
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,8 @@ const errorTests = [
expect: [
'Thrown:',
/^{ Error: Cannot find module 'internal\/repl'/,
/^Require stack:/,
/^- <repl>/,
/^ at .*/,
/^ at .*/,
/^ at .*/,
Expand Down
15 changes: 8 additions & 7 deletions test/sequential/test-module-loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ require('../fixtures/node_modules/foo');
assert.ok(my_path.path_func instanceof Function);
// this one does not exist and should throw
assert.throws(function() { require('./utils'); },
/^Error: Cannot find module '\.\/utils'$/);
/^Error: Cannot find module '\.\/utils'/);
}

let errorThrown = false;
Expand Down Expand Up @@ -170,12 +170,13 @@ assert.strictEqual(require('../fixtures/registerExt2').custom, 'passed');
assert.strictEqual(require('../fixtures/foo').foo, 'ok');

// Should not attempt to load a directory
try {
tmpdir.refresh();
require(tmpdir.path);
} catch (err) {
assert.strictEqual(err.message, `Cannot find module '${tmpdir.path}'`);
}
assert.throws(
() => {
tmpdir.refresh();
require(tmpdir.path);
},
(err) => err.message.startsWith(`Cannot find module '${tmpdir.path}`)
);

{
// Check load order is as expected
Expand Down

0 comments on commit a02e3e2

Please sign in to comment.