Skip to content

Commit

Permalink
utils: [Fix] report the error stack on a resolution error
Browse files Browse the repository at this point in the history
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.<anonymous> (/__censored__/webpack/configForEslintImportResolver.js:1:126)
```
  • Loading branch information
sompylasar authored and ljharb committed Oct 2, 2016
1 parent b511da2 commit aed7a73
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 7 deletions.
2 changes: 1 addition & 1 deletion tests/files/load-error-resolver.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
throw new Error('TEST ERROR')
throw new SyntaxError('TEST SYNTAX ERROR')
9 changes: 7 additions & 2 deletions tests/src/core/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<stack-was-here>') : str
}

it('throws on bad parameters', function () {
expect(resolve.bind(null, null, null)).to.throw(Error)
})
Expand Down Expand Up @@ -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<stack-was-here>')
expect(testContextReports[0].loc).to.eql({ line: 1, column: 0 })

testContextReports.length = 0
Expand Down Expand Up @@ -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<stack-was-here>')
expect(testContextReports[0].loc).to.eql({ line: 1, column: 0 })
})

Expand Down
3 changes: 3 additions & 0 deletions utils/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
22 changes: 18 additions & 4 deletions utils/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit aed7a73

Please sign in to comment.