From aed7a7391d014594b034fa92c4daf1a6d50e3f5a Mon Sep 17 00:00:00 2001 From: John Babak Date: Sun, 2 Oct 2016 03:43:37 +0300 Subject: [PATCH] utils: [Fix] report the error stack on a resolution error Fixes #536. Resolve errors are most likely caused by invalid configuration, and the reason is easier to determine with the full details rather than just `err.message`. With this change, it reports something like: ``` import/no-unresolved: Resolve error: SyntaxError: Unexpected token import at exports.runInThisContext (vm.js:53:16) at Module._compile (module.js:387:25) at Object.Module._extensions..js (module.js:422:10) at Module.load (module.js:357:32) at Function.Module._load (module.js:314:12) at Module.require (module.js:367:17) at require (internal/module.js:16:19) at module.exports (/__censored__/webpack/configFactory.js:216:3) at configProdClient (/__censored__/webpack/configProdClient.js:5:36) at Object. (/__censored__/webpack/configForEslintImportResolver.js:1:126) ``` --- tests/files/load-error-resolver.js | 2 +- tests/src/core/resolve.js | 9 +++++++-- utils/CHANGELOG.md | 3 +++ utils/resolve.js | 22 ++++++++++++++++++---- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/tests/files/load-error-resolver.js b/tests/files/load-error-resolver.js index aa9d33010..7cf7cfa1b 100644 --- a/tests/files/load-error-resolver.js +++ b/tests/files/load-error-resolver.js @@ -1 +1 @@ -throw new Error('TEST ERROR') +throw new SyntaxError('TEST SYNTAX ERROR') diff --git a/tests/src/core/resolve.js b/tests/src/core/resolve.js index 3bf46cd1a..1aa2071db 100644 --- a/tests/src/core/resolve.js +++ b/tests/src/core/resolve.js @@ -8,6 +8,11 @@ import * as fs from 'fs' import * as utils from '../utils' describe('resolve', function () { + // We don't want to test for a specific stack, just that it was there in the error message. + function replaceErrorStackForTest(str) { + return typeof str === 'string' ? str.replace(/(\n\s+at .+:\d+\)?)+$/, '\n') : str + } + it('throws on bad parameters', function () { expect(resolve.bind(null, null, null)).to.throw(Error) }) @@ -60,7 +65,7 @@ describe('resolve', function () { , Object.assign({}, testContext, { getFilename: function () { return utils.getFilename('exception.js') } }), )).to.equal(undefined) expect(testContextReports[0]).to.be.an('object') - expect(testContextReports[0].message).to.equal('Resolve error: foo-bar-resolver-v2 resolve test exception') + expect(replaceErrorStackForTest(testContextReports[0].message)).to.equal('Resolve error: foo-bar-resolver-v2 resolve test exception\n') expect(testContextReports[0].loc).to.eql({ line: 1, column: 0 }) testContextReports.length = 0 @@ -153,7 +158,7 @@ describe('resolve', function () { , Object.assign({}, testContext, { getFilename: function () { return utils.getFilename('exception.js') } }), )).to.equal(undefined) expect(testContextReports[0]).to.be.an('object') - expect(testContextReports[0].message).to.equal('Resolve error: TEST ERROR') + expect(replaceErrorStackForTest(testContextReports[0].message)).to.equal('Resolve error: SyntaxError: TEST SYNTAX ERROR\n') expect(testContextReports[0].loc).to.eql({ line: 1, column: 0 }) }) diff --git a/utils/CHANGELOG.md b/utils/CHANGELOG.md index 6a7fe6764..4573f794b 100644 --- a/utils/CHANGELOG.md +++ b/utils/CHANGELOG.md @@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Fixed - Uses createRequireFromPath to resolve loaders ([#1591], thanks [@arcanis]) +- report the error stack on a resolution error ([#599], thanks [@sompylasar]) ## v2.5.0 - 2019-12-07 @@ -72,6 +73,7 @@ Yanked due to critical issue with cache key resulting from #839. [#1166]: https://github.com/benmosher/eslint-plugin-import/issues/1166 [#1160]: https://github.com/benmosher/eslint-plugin-import/pull/1160 [#1035]: https://github.com/benmosher/eslint-plugin-import/issues/1035 +[#599]: https://github.com/benmosher/eslint-plugin-import/pull/599 [@hulkish]: https://github.com/hulkish [@timkraut]: https://github.com/timkraut @@ -81,3 +83,4 @@ Yanked due to critical issue with cache key resulting from #839. [@brettz9]: https://github.com/brettz9 [@JounQin]: https://github.com/JounQin [@arcanis]: https://github.com/arcanis +[@sompylasar]: https://github.com/sompylasar diff --git a/utils/resolve.js b/utils/resolve.js index 945ba4b6f..3dc15f022 100644 --- a/utils/resolve.js +++ b/utils/resolve.js @@ -13,6 +13,8 @@ const hashObject = require('./hash').hashObject const CASE_SENSITIVE_FS = !fs.existsSync(path.join(__dirname, 'reSOLVE.js')) exports.CASE_SENSITIVE_FS = CASE_SENSITIVE_FS +const ERROR_NAME = 'EslintPluginImportResolveError' + const fileExistsCache = new ModuleCache() // Polyfill Node's `Module.createRequireFromPath` if not present (added in Node v10.12.0) @@ -162,7 +164,9 @@ function resolverReducer(resolvers, map) { return map } - throw new Error('invalid resolver config') + const err = new Error('invalid resolver config') + err.name = ERROR_NAME + throw err } function getBaseDir(sourceFile) { @@ -175,10 +179,14 @@ function requireResolver(name, sourceFile) { tryRequire(path.resolve(getBaseDir(sourceFile), name)) if (!resolver) { - throw new Error(`unable to load resolver "${name}".`) + const err = new Error(`unable to load resolver "${name}".`) + err.name = ERROR_NAME + throw err } if (!isResolverValid(resolver)) { - throw new Error(`${name} with invalid interface loaded as resolver`) + const err = new Error(`${name} with invalid interface loaded as resolver`) + err.name = ERROR_NAME + throw err } return resolver @@ -210,8 +218,14 @@ function resolve(p, context) { ) } catch (err) { if (!erroredContexts.has(context)) { + // The `err.stack` string starts with `err.name` followed by colon and `err.message`. + // We're filtering out the default `err.name` because it adds little value to the message. + let errMessage = err.message + if (err.name !== ERROR_NAME && err.stack) { + errMessage = err.stack.replace(/^Error: /, '') + } context.report({ - message: `Resolve error: ${err.message}`, + message: `Resolve error: ${errMessage}`, loc: { line: 1, column: 0 }, }) erroredContexts.add(context)