Skip to content

Commit

Permalink
fix(reporter): warning if stack trace contains generated code invocation
Browse files Browse the repository at this point in the history
For some projects, a preprocessor like TypeScript may run to downlevel
certain features to a lower ECMAScript target. e.g. a project may
consume Angular for example, which ships ES2020.

If TypeScript, ESBuild, Babel etc. is used to do this, they may inject
generated code which does not map to any original source. If any of
these helpers (the generated code) is then part of a stack trace, Karma
will incorrectly report an error for an unresolved source map position.

Generated code is valid within source maps and can be denoted as
mappings with a 1-variable-length mapping. See the source map spec:
https://sourcemaps.info/spec.html.

The warning for generated code is especially bad when the majority of
file paths, and the actually relevant-portions in the stack are
resolved properly. e.g.

Errors initially look like this without the source mapping processing
of Karma:

(See PR description as commit lint does not allow for long stack
traces..)

A helper function shows up in the stacktrace but has no original mapping as it is
purely generated by TypeScript/ESbuild etc. The following warning is
printed and pollutes the test output while the remaining stack trace
paths (as said before), have been remapped properly:

```
SourceMap position not found for trace: http://localhost:9877/base/angular_material/
  src/material/select/testing/unit_tests_bundle_spec.js:26:26
```

The resolved stacktrace looks like this after the transformation:

(see PR description as commit lint does not allow for long stack traces
here..)

More details on the scenario here:
https://gist.github.com/devversion/549d25915c2dc98a8896ba4408a1e27c.
  • Loading branch information
devversion authored and Jonathan Ginsburg committed Nov 4, 2021
1 parent 4c6f681 commit 4f23b14
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 2 deletions.
13 changes: 11 additions & 2 deletions lib/reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function createErrorFormatter (config, emitter, SourceMapConsumer) {
input = JSON.stringify(input, null, indentation)
}

let msg = input.replace(URL_REGEXP, function (_, prefix, path, __, ___, line, ____, column) {
let msg = input.replace(URL_REGEXP, function (stackTracePath, prefix, path, __, ___, line, ____, column) {
const normalizedPath = prefix === 'base/' ? `${basePath}/${path}` : path
const file = lastServedFiles.find((file) => file.path === normalizedPath)

Expand All @@ -64,12 +64,21 @@ function createErrorFormatter (config, emitter, SourceMapConsumer) {
const zeroBasedColumn = Math.max(0, (column || 1) - 1)
const original = getSourceMapConsumer(file.sourceMap).originalPositionFor({ line, column: zeroBasedColumn, bias })

// If there is no original position/source for the current stack trace path, then
// we return early with the formatted generated position. This handles the case of
// generated code which does not map to anything, see Case 1 of the source-map spec.
// https://sourcemaps.info/spec.html.
if (original.source === null) {
return PathUtils.formatPathMapping(path, line, column)
}

// Source maps often only have a local file name, resolve to turn into a full path if
// the path is not absolute yet.
const oneBasedOriginalColumn = original.column == null ? original.column : original.column + 1
return `${PathUtils.formatPathMapping(resolve(path, original.source), original.line, oneBasedOriginalColumn)} <- ${PathUtils.formatPathMapping(path, line, column)}`
} catch (e) {
log.warn(`SourceMap position not found for trace: ${input}`)
log.warn(`An unexpected error occurred while resolving the original position for: ${stackTracePath}`)
log.warn(e)
}
}

Expand Down
75 changes: 75 additions & 0 deletions test/unit/reporter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const loadFile = require('mocks').loadFile
const path = require('path')
const _ = require('lodash')
const sinon = require('sinon')
const logger = require('../../lib/logger')

const File = require('../../lib/file')

Expand Down Expand Up @@ -127,6 +128,8 @@ describe('reporter', () => {
describe('source maps', () => {
let originalPositionForCallCount = 0
let sourceMappingPath = null
let log
let logWarnStub

class MockSourceMapConsumer {
constructor (sourceMap) {
Expand All @@ -147,9 +150,36 @@ describe('reporter', () => {
}
}

class MockSourceMapConsumerWithParseError {
constructor () {
throw new Error('Fake parse error from source map consumer')
}
}

class MockSourceMapConsumerWithGeneratedCode {
constructor (sourceMap) {
this.source = sourceMap.content.replace('SOURCE MAP ', sourceMappingPath)
}

originalPositionFor () {
return {
source: null,
line: null,
column: null
}
}
}

beforeEach(() => {
originalPositionForCallCount = 0
sourceMappingPath = '/original/'

log = logger.create('reporter')
logWarnStub = sinon.spy(log, 'warn')
})

afterEach(() => {
logWarnStub.restore()
})

MockSourceMapConsumer.GREATEST_LOWER_BOUND = 1
Expand All @@ -170,6 +200,51 @@ describe('reporter', () => {
})
})

// Regression test for cases like: https://github.com/karma-runner/karma/pull/1098.
// Note that the scenario outlined in the PR should no longer surface due to a check
// ensuring that the line always is non-zero, but there could be other parsing errors.
it('should handle source map errors gracefully', (done) => {
formatError = m.createErrorFormatter({ basePath: '', hostname: 'localhost', port: 123 }, emitter,
MockSourceMapConsumerWithParseError)

const servedFiles = [new File('/a.js'), new File('/b.js')]
servedFiles[0].sourceMap = { content: 'SOURCE MAP a.js' }
servedFiles[1].sourceMap = { content: 'SOURCE MAP b.js' }

emitter.emit('file_list_modified', { served: servedFiles })

_.defer(() => {
const ERROR = 'at http://localhost:123/base/b.js:2:6'
expect(formatError(ERROR)).to.equal('at b.js:2:6\n')
expect(logWarnStub.callCount).to.equal(2)
expect(logWarnStub).to.have.been.calledWith('An unexpected error occurred while resolving the original position for: http://localhost:123/base/b.js:2:6')
expect(logWarnStub).to.have.been.calledWith(sinon.match({ message: 'Fake parse error from source map consumer' }))
done()
})
})

// Generated code can be added by e.g. TypeScript or Babel when it transforms
// native async/await to generators. Calls would then be wrapped with a helper
// that is generated and does not map to anything, so-called generated code that
// is allowed as case #1 in the source map spec.
it('should not warn for trace file portion for generated code', (done) => {
formatError = m.createErrorFormatter({ basePath: '', hostname: 'localhost', port: 123 }, emitter,
MockSourceMapConsumerWithGeneratedCode)

const servedFiles = [new File('/a.js'), new File('/b.js')]
servedFiles[0].sourceMap = { content: 'SOURCE MAP a.js' }
servedFiles[1].sourceMap = { content: 'SOURCE MAP b.js' }

emitter.emit('file_list_modified', { served: servedFiles })

_.defer(() => {
const ERROR = 'at http://localhost:123/base/b.js:2:6'
expect(formatError(ERROR)).to.equal('at b.js:2:6\n')
expect(logWarnStub.callCount).to.equal(0)
done()
})
})

it('should rewrite stack traces (when basePath is empty)', (done) => {
formatError = m.createErrorFormatter({ basePath: '', hostname: 'localhost', port: 123 }, emitter, MockSourceMapConsumer)
const servedFiles = [new File('/a.js'), new File('/b.js')]
Expand Down

0 comments on commit 4f23b14

Please sign in to comment.