From a02e3e2d5f1f96f3c408270d45935afdd5d1fffc Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Wed, 13 Feb 2019 13:21:10 -0800 Subject: [PATCH] lib: improve error message for MODULE_NOT_FOUND Include the require stack in the reported error message. PR-URL: https://github.com/nodejs/node/pull/25690 Reviewed-By: Anna Henningsen Reviewed-By: Joyee Cheung Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/internal/modules/cjs/loader.js | 6 +++++- test/fixtures/require-resolve.js | 6 +++--- test/parallel/test-internal-modules.js | 2 +- test/parallel/test-loaders-hidden-from-users.js | 4 ++-- test/parallel/test-module-loading-error.js | 7 ++++--- test/parallel/test-module-multi-extensions.js | 6 +++--- test/parallel/test-repl.js | 2 ++ test/sequential/test-module-loading.js | 15 ++++++++------- 8 files changed, 28 insertions(+), 20 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index e420a10e4867a1..b636f39041a5fc 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -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; diff --git a/test/fixtures/require-resolve.js b/test/fixtures/require-resolve.js index 2a0dc8846ce3cb..aa496e5eb66f70 100644 --- a/test/fixtures/require-resolve.js +++ b/test/fixtures/require-resolve.js @@ -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( diff --git a/test/parallel/test-internal-modules.js b/test/parallel/test-internal-modules.js index c61d957acd0542..e6bd0c9e141328 100644 --- a/test/parallel/test-internal-modules.js +++ b/test/parallel/test-internal-modules.js @@ -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')), diff --git a/test/parallel/test-loaders-hidden-from-users.js b/test/parallel/test-loaders-hidden-from-users.js index 0d752f5718b729..104681bd792bdf 100644 --- a/test/parallel/test-loaders-hidden-from-users.js +++ b/test/parallel/test-loaders-hidden-from-users.js @@ -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'/ } ); @@ -20,6 +20,6 @@ common.expectsError( require('owo'); }, { code: 'MODULE_NOT_FOUND', - message: 'Cannot find module \'owo\'' + message: /Cannot find module 'owo'/ } ); diff --git a/test/parallel/test-module-loading-error.js b/test/parallel/test-module-loading-error.js index 67dee6a3ba1d4f..40b6de72a3299b 100644 --- a/test/parallel/test-module-loading-error.js +++ b/test/parallel/test-module-loading-error.js @@ -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'/ + } +); diff --git a/test/parallel/test-module-multi-extensions.js b/test/parallel/test-module-multi-extensions.js index be17bc5d025508..1257854115a5b5 100644 --- a/test/parallel/test-module-multi-extensions.js +++ b/test/parallel/test-module-multi-extensions.js @@ -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]; @@ -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); @@ -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); diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 14ada1fe601cca..a53f4a2833d439 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -534,6 +534,8 @@ const errorTests = [ expect: [ 'Thrown:', /^{ Error: Cannot find module 'internal\/repl'/, + /^Require stack:/, + /^- /, /^ at .*/, /^ at .*/, /^ at .*/, diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js index 6cfa4da04e2c04..90ab1d4f4f285c 100644 --- a/test/sequential/test-module-loading.js +++ b/test/sequential/test-module-loading.js @@ -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; @@ -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