fix(reporter): warning if stack trace contains generated code invocation

devversion · Jonathan Ginsburg · commit 4f23b14d3e77 · 2021-11-04T14:21:45.000-06:00
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.
diff --git a/lib/reporter.js b/lib/reporter.js
@@ -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)
 
@@ -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)
         }
       }
 
diff --git a/test/unit/reporter.spec.js b/test/unit/reporter.spec.js
@@ -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')
 
@@ -127,6 +128,8 @@ describe('reporter', () => {
     describe('source maps', () => {
       let originalPositionForCallCount = 0
       let sourceMappingPath = null
+      let log
+      let logWarnStub
 
       class MockSourceMapConsumer {
         constructor (sourceMap) {
@@ -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
@@ -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')]
