alternative stack trace approach (#2119)

* reimpl getDefinitionLineAndUri

* disable current stack trace filtering

* filter stack traces without modifying error

* rename file

* rename file

* add advice about source maps

* WIP feature for transpiled stack traces

* improve feature for transpiled stack traces

* skip source-mapping scenarios when instrumenting with nyc

* update CHANGELOG.md

Author: davidjgoss

CHANGELOG.md

@@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 Please see [CONTRIBUTING.md](https://github.com/cucumber/cucumber/blob/master/CONTRIBUTING.md) on how to contribute to Cucumber.
 
 ## [Unreleased]
+## Changed
+- Handle stack traces without V8-specific modification ([#2119](https://github.com/cucumber/cucumber-js/pull/2119))
 
 ## [8.7.0] - 2022-10-17
 ### Deprecated

docs/transpiling.md

@@ -57,3 +57,14 @@ If you are using babel with [@babel/preset-typescript](https://babeljs.io/docs/e
 ### ESM
 
 See [ESM](./esm.md) for general advice on using loaders for transpilation in ESM projects.
+
+### Source maps
+
+Source maps are used to ensure accurate source references and stack traces in Cucumber's reporting, by giving traceability from a transpiled piece of code back to the original source code.
+
+Just-in-time transpilers like `ts-node` and `@babel/register` have sensible default configuration that emits source maps and enables them in the runtime environment, so you shouldn't have to do anything in order for source maps to work.
+
+If you're using step definition code that's _already_ transpiled (maybe because it's a shared library) then you'll need to:
+
+1. Ensure source maps are emitted by your transpiler. You can verify by checking for a comment starting with `//# sourceMappingURL=` at the end of your transpiled file(s).
+2. Ensure source maps are enabled at runtime. Node.js supports this natively via [the `--enable-source-maps` flag](https://nodejs.org/docs/latest/api/cli.html#--enable-source-maps).

features/stack_traces.feature

@@ -0,0 +1,51 @@
+@spawn
+@source-mapping
+Feature: Stack traces
+  Background:
+    Given a file named "features/a.feature" with:
+      """
+      Feature: some feature
+        Scenario: some scenario
+          Given a passing step
+          And a failing step
+      """
+
+  Rule: Source maps are respected when dealing with transpiled support code
+
+    Just-in-time transpilers like `@babel/register` and `ts-node` emit source maps with
+    the transpiled code. Cucumber users expect stack traces to point to the line and column
+    in the original source file when there is an error.
+
+    Background:
+      Given a file named "features/steps.ts" with:
+      """
+      import { Given } from '@cucumber/cucumber'
+
+      interface Something {
+        field1: string
+        field2: string
+      }
+
+      Given('a passing step', function() {})
+
+      Given('a failing step', function() {
+        throw new Error('boom')
+      })
+      """
+
+    Scenario: commonjs
+      When I run cucumber-js with `--require-module ts-node/register --require features/steps.ts`
+      Then the output contains the text:
+      """
+      /features/steps.ts:11:9
+      """
+      And it fails
+
+    Scenario: esm
+      Given my env includes "{\"NODE_OPTIONS\":\"--loader ts-node/esm\"}"
+      When I run cucumber-js with `--import features/steps.ts`
+      Then the output contains the text:
+      """
+      /features/steps.ts:11:9
+      """
+      And it fails

features/support/world.ts

@@ -41,10 +41,14 @@ export class World {
   parseEnvString(str: string): NodeJS.ProcessEnv {
     const result: NodeJS.ProcessEnv = {}
     if (doesHaveValue(str)) {
-      str
-        .split(/\s+/)
-        .map((keyValue) => keyValue.split('='))
-        .forEach((pair) => (result[pair[0]] = pair[1]))
+      try {
+        Object.assign(result, JSON.parse(str))
+      } catch {
+        str
+          .split(/\s+/)
+          .map((keyValue) => keyValue.split('='))
+          .forEach((pair) => (result[pair[0]] = pair[1]))
+      }
     }
     return result
   }

package-lock.json

@@ -195,7 +195,6 @@
     "node": "12 || 14 || 16 || 17 || 18"
   },
   "dependencies": {
-    "@cspotcode/source-map-support": "^0.8.0",
     "@cucumber/ci-environment": "9.1.0",
     "@cucumber/cucumber-expressions": "16.0.0",
     "@cucumber/gherkin": "24.1.0",
@@ -213,6 +212,7 @@
     "debug": "^4.3.4",
     "duration": "^0.2.2",
     "durations": "^3.4.2",
+    "error-stack-parser": "^2.1.4",
     "figures": "^3.2.0",
     "glob": "^7.1.6",
     "has-ansi": "^4.0.1",
@@ -226,7 +226,6 @@
     "progress": "^2.0.3",
     "resolve-pkg": "^2.0.0",
     "semver": "7.3.8",
-    "stack-chain": "^2.0.0",
     "string-argv": "^0.3.1",
     "strip-ansi": "6.0.1",
     "supports-color": "^8.1.1",
@@ -311,7 +310,7 @@
     "prepublishOnly": "rm -rf lib && npm run build-local",
     "pretest-coverage": "npm run build-local",
     "pretypes-test": "npm run build-local",
-    "test-coverage": "nyc --silent mocha 'src/**/*_spec.ts' 'compatibility/**/*_spec.ts' && nyc --silent --no-clean node bin/cucumber.js && nyc report --reporter=lcov",
+    "test-coverage": "nyc --silent mocha 'src/**/*_spec.ts' 'compatibility/**/*_spec.ts' && nyc --silent --no-clean node bin/cucumber.js --tags \"not @source-mapping\" && nyc report --reporter=lcov",
     "test": "npm run lint && npm run types-test && npm run unit-test && npm run cck-test && npm run feature-test",
     "types-test": "tsd",
     "unit-test": "mocha 'src/**/*_spec.ts'",

package.json

@@ -0,0 +1,38 @@
+import path from 'path'
+import { valueOrDefault } from './value_checker'
+import { StackFrame } from 'error-stack-parser'
+
+const projectRootPath = path.join(__dirname, '..')
+const projectChildDirs = ['src', 'lib', 'node_modules']
+
+export function isFileNameInCucumber(fileName: string): boolean {
+  return projectChildDirs.some((dir) =>
+    fileName.startsWith(path.join(projectRootPath, dir))
+  )
+}
+
+export function filterStackTrace(frames: StackFrame[]): StackFrame[] {
+  if (isErrorInCucumber(frames)) {
+    return frames
+  }
+  const index = frames.findIndex((x) => isFrameInCucumber(x))
+  if (index === -1) {
+    return frames
+  }
+  return frames.slice(0, index)
+}
+
+function isErrorInCucumber(frames: StackFrame[]): boolean {
+  const filteredFrames = frames.filter((x) => !isFrameInNode(x))
+  return filteredFrames.length > 0 && isFrameInCucumber(filteredFrames[0])
+}
+
+function isFrameInCucumber(frame: StackFrame): boolean {
+  const fileName = valueOrDefault(frame.getFileName(), '')
+  return isFileNameInCucumber(fileName)
+}
+
+function isFrameInNode(frame: StackFrame): boolean {
+  const fileName = valueOrDefault(frame.getFileName(), '')
+  return !fileName.includes(path.sep)
+}

src/filter_stack_trace.ts

@@ -0,0 +1,22 @@
+import { format } from 'assertion-error-formatter'
+import errorStackParser from 'error-stack-parser'
+import { filterStackTrace } from '../filter_stack_trace'
+
+export function formatError(error: Error, filterStackTraces: boolean): string {
+  let filteredStack: string
+  if (filterStackTraces) {
+    try {
+      filteredStack = filterStackTrace(errorStackParser.parse(error))
+        .map((f) => f.source)
+        .join('\n')
+    } catch {
+      // if we weren't able to parse and filter, we'll settle for the original
+    }
+  }
+  return format(error, {
+    colorFns: {
+      errorStack: (stack: string) =>
+        filteredStack ? `\n${filteredStack}` : stack,
+    },
+  })
+}

src/runtime/format_error.ts

@@ -2,7 +2,6 @@ import * as messages from '@cucumber/messages'
 import { IdGenerator } from '@cucumber/messages'
 import { EventEmitter } from 'events'
 import { EventDataCollector } from '../formatter/helpers'
-import StackTraceFilter from '../stack_trace_filter'
 import { ISupportCodeLibrary } from '../support_code_library_builder/types'
 import { assembleTestCases } from './assemble_test_cases'
 import { retriesForPickle, shouldCauseFailure } from './helpers'
@@ -40,7 +39,6 @@ export default class Runtime implements IRuntime {
   private readonly newId: IdGenerator.NewId
   private readonly options: IRuntimeOptions
   private readonly pickleIds: string[]
-  private readonly stackTraceFilter: StackTraceFilter
   private readonly supportCodeLibrary: ISupportCodeLibrary
   private success: boolean
   private runTestRunHooks: RunsTestRunHooks
@@ -59,7 +57,6 @@ export default class Runtime implements IRuntime {
     this.newId = newId
     this.options = options
     this.pickleIds = pickleIds
-    this.stackTraceFilter = new StackTraceFilter()
     this.supportCodeLibrary = supportCodeLibrary
     this.success = true
     this.runTestRunHooks = makeRunTestRunHooks(
@@ -85,6 +82,7 @@ export default class Runtime implements IRuntime {
       testCase,
       retries,
       skip,
+      filterStackTraces: this.options.filterStacktraces,
       supportCodeLibrary: this.supportCodeLibrary,
       worldParameters: this.options.worldParameters,
     })
@@ -95,9 +93,6 @@ export default class Runtime implements IRuntime {
   }
 
   async start(): Promise<boolean> {
-    if (this.options.filterStacktraces) {
-      this.stackTraceFilter.filter()
-    }
     const testRunStarted: messages.Envelope = {
       testRunStarted: {
         timestamp: this.stopwatch.timestamp(),
@@ -132,9 +127,6 @@ export default class Runtime implements IRuntime {
       },
     }
     this.eventBroadcaster.emit('envelope', testRunFinished)
-    if (this.options.filterStacktraces) {
-      this.stackTraceFilter.unfilter()
-    }
     return this.success
   }
 }