Skip to content

Commit

Permalink
Fix #536: For Resolve error report err.stack
Browse files Browse the repository at this point in the history
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`.

See #536

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 committed Oct 29, 2018
1 parent db471a8 commit 931363f
Show file tree
Hide file tree
Showing 3 changed files with 24 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 @@ -145,7 +150,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
20 changes: 16 additions & 4 deletions utils/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ function resolverReducer(resolvers, map) {
return map
}

throw new Error('invalid resolver config')
const err = new Error('invalid resolver config')
err.name = 'EslintPluginImportResolveError'
throw err
}

function getBaseDir(sourceFile) {
Expand All @@ -159,10 +161,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 = 'EslintPluginImportResolveError'
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 = 'EslintPluginImportResolveError'
throw err
}

return resolver
Expand Down Expand Up @@ -194,8 +200,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 !== 'EslintPluginImportResolveError' && 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 931363f

Please sign in to comment.