fix: Error codeframes in retry failures display the code that produce…

…d the error (#30962)

* change how user invocation stacks are prepared, for more flexibility

* build darwin and persist artifacts for this branch

* changelog

* add implicit dependencies to @packages/extension so things build in the right order

* note about binary testing for error_utils and stack_utils

* changelog

* Update packages/driver/test/unit/cypress/err_utils.spec.ts

Co-authored-by: Bill Glesias <[email protected]>

---------

Co-authored-by: Bill Glesias <[email protected]>

.circleci/workflows.yml

@@ -33,7 +33,7 @@ mainBuildFilters: &mainBuildFilters
         - /^release\/\d+\.\d+\.\d+$/
         # use the following branch as well to ensure that v8 snapshot cache updates are fully tested
         - 'update-v8-snapshot-cache-on-develop'
-        - 'chore/browser_spike'
+        - 'cacie/30927/retry-error-codeframe'
 
 # usually we don't build Mac app - it takes a long time
 # but sometimes we want to really confirm we are doing the right thing
@@ -44,6 +44,7 @@ macWorkflowFilters: &darwin-workflow-filters
     - equal: [ develop, << pipeline.git.branch >> ]
     # use the following branch as well to ensure that v8 snapshot cache updates are fully tested
     - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ]
+    - equal: [ 'cacie/30927/retry-error-codeframe', << pipeline.git.branch >> ]
     - equal: [ 'chore/browser_spike', << pipeline.git.branch >> ]
     - matches:
         pattern: /^release\/\d+\.\d+\.\d+$/
@@ -154,7 +155,7 @@ commands:
           name: Set environment variable to determine whether or not to persist artifacts
           command: |
             echo "Setting SHOULD_PERSIST_ARTIFACTS variable"
-            echo 'if ! [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "chore/browser_spike" ]]; then
+            echo 'if ! [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "cacie/30927/retry-error-codeframe" ]]; then
                 export SHOULD_PERSIST_ARTIFACTS=true
             fi' >> "$BASH_ENV"
   # You must run `setup_should_persist_artifacts` command and be using bash before running this command

cli/CHANGELOG.md

@@ -5,6 +5,7 @@ _Released 2/11/2025 (PENDING)_
 
 **Bugfixes:**
 
+- Fixed a regression introduced in [`14.0.0`](https://docs.cypress.io/guides/references/changelog#14-0-0) where error codeframes in the runner UI were not populated with the correct data in failed retry attempts. Fixes [#30927](https://github.com/cypress-io/cypress/issues/30927).
 - All commands performed in `after` and `afterEach` hooks will now correctly retry when a test fails. Commands that are actions like `.click()` and `.type()` will now perform the action in this situation also. Fixes [#2831](https://github.com/cypress-io/cypress/issues/2831).
 - Fixed an issue in Cypress [`14.0.0`](https://docs.cypress.io/guides/references/changelog#14-0-0) where privileged commands did not run correctly when a spec file or support file contained characters that required encoding. Fixes [#30933](https://github.com/cypress-io/cypress/issues/30933).
 

packages/driver/src/cypress/error_utils.ts

@@ -1,5 +1,13 @@
 // See: ./errorScenarios.md for details about error messages and stack traces
 
+// NOTE: If you modify the logic relating to this file, ensure the
+// UI for error code frames works as expected with the binary. This includes each
+// browser, as well as e2e and CT testing types. Stack patterns differ in Chrome
+// between the binary and dev mode, so Cy in Cy tests cannot catch them proactively.
+
+// Various stack patterns are saved as scenario fixtures in ./driver/test
+// to prevent regressions.
+
 import chai from 'chai'
 import _ from 'lodash'
 import $dom from '../dom'
@@ -136,7 +144,7 @@ const getUserInvocationStack = (err, state) => {
   // command errors and command assertion errors (default assertion or cy.should)
   // have the invocation stack attached to the current command
   // prefer err.userInvocation stack if it's been set
-  let userInvocationStack = err.userInvocationStack || state('currentAssertionUserInvocationStack')
+  let userInvocationStack: string | undefined = err.userInvocationStack || state('currentAssertionUserInvocationStack')
 
   // if there is no user invocation stack from an assertion or it is the default
   // assertion, meaning it came from a command (e.g. cy.get), prefer the
@@ -153,14 +161,29 @@ const getUserInvocationStack = (err, state) => {
     userInvocationStack = withInvocationStack?.get('userInvocationStack')
   }
 
-  if (!userInvocationStack) return
+  if (!userInvocationStack) return undefined
 
-  // In CT with vite, the user invocation stack includes internal cypress code, so clean it up
+  // In some environments, additional codepoints are included in the stack prior
+  // to the first userland codepoint.
+  const internalCodepointIdentifier = [
+    '/__cypress/runner', // binary environments and most dev environments
+    'cypress:///../driver', // webpack CT with a dev build
+  ].find((identifier) => {
+    return userInvocationStack?.includes(identifier)
+  })
 
-  // remove lines that are included _prior_ to the first userland line
-  userInvocationStack = $stackUtils.stackWithLinesDroppedFromMarker(userInvocationStack, '/__cypress', true)
+  // removes lines in the invocation stack above the first userland line. If one
+  // of the cypress codepoint identifiers is not present in the stack trace,
+  // the first line will be a userland codepoint, so no dropping is necessary.
+  userInvocationStack = internalCodepointIdentifier ? _.dropWhile(
+    userInvocationStack.split('\n'),
+    (stackLine) => {
+      return stackLine.includes(internalCodepointIdentifier)
+    },
+  ).join('\n') : userInvocationStack
 
-  // remove lines that are included _after and including_ the replacement marker
+  // remove lines that are included _after and including_ the replacement marker -
+  // these are also internal to cypress, and unimportant for the user invocation stack
   userInvocationStack = $stackUtils.stackPriorToReplacementMarker(userInvocationStack)
 
   if (
@@ -170,6 +193,8 @@ const getUserInvocationStack = (err, state) => {
   ) {
     return userInvocationStack
   }
+
+  return undefined
 }
 
 const modifyErrMsg = (err, newErrMsg, cb) => {

packages/driver/src/cypress/stack_utils.ts

@@ -1,4 +1,13 @@
 // See: ./errorScenarios.md for details about error messages and stack traces
+
+// NOTE: If you modify the logic relating to this file, ensure the
+// UI for error code frames works as expected with the binary. This includes each
+// browser, as well as e2e and CT testing types. Stack patterns differ in Chrome
+// between the binary and dev mode, so Cy in Cy tests cannot catch them proactively.
+
+// Various stack patterns are saved as scenario fixtures in ./driver/test
+// to prevent regressions.
+
 import _ from 'lodash'
 import path from 'path'
 import errorStackParser from 'error-stack-parser'

packages/driver/test/unit/cypress/err_utils.spec.ts

@@ -0,0 +1,67 @@
+/**
+ * @vitest-environment jsdom
+ */
+import { vi, describe, it, expect, beforeEach } from 'vitest'
+
+// source_map_utils must be included in order for vite to mock it, even
+// if it isn't referenced.
+// eslint-disable-next-line
+import source_map_utils from '../../../src/cypress/source_map_utils'
+import errUtils from '../../../src/cypress/error_utils'
+import stackFrameFixture from './__fixtures__/getUserInvocationStack_stackFrames.json'
+
+vi.mock('../../../src/cypress/source_map_utils', () => {
+  return {
+    default: {
+      getSourcePosition: vi.fn(),
+    },
+  }
+})
+
+describe('err_utils', () => {
+  beforeEach(() => {
+    // @ts-expect-error
+    global.Cypress = {
+      config: vi.fn(),
+    }
+
+    vi.resetAllMocks()
+  })
+
+  describe('getUserInvocationStack', () => {
+    const { invocationFile, line, column, scenarios } = stackFrameFixture
+
+    let stack: string
+
+    class MockError {
+      name = 'CypressError'
+      get userInvocationStack () {
+        return stack
+      }
+    }
+
+    const state = () => undefined
+
+    for (const scenario of scenarios) {
+      const { browser, build, testingType, stack: scenarioStack } = scenario
+
+      describe(`${browser}:${build}:${testingType}`, () => {
+        beforeEach(() => {
+          stack = scenarioStack
+        })
+
+        it('returns the userInvocationStack with no leading internal cypress codeframes', () => {
+          const invocationStack = errUtils.getUserInvocationStack(new MockError(), state)
+
+          expect(invocationStack).not.toBeUndefined()
+
+          const [first, second] = (invocationStack as string).split('\n')
+
+          const invocationFrame = second ?? first
+
+          expect(invocationFrame).toContain(`${invocationFile}:${line}:${column}`)
+        })
+      })
+    }
+  })
+})

packages/driver/test/unit/cypress/stack_utils.spec.ts

@@ -5,7 +5,7 @@ import { vi, describe, it, expect, beforeEach } from 'vitest'
 
 import source_map_utils from '../../../src/cypress/source_map_utils'
 import stack_utils from '../../../src/cypress/stack_utils'
-import stackFrameFixture from './__fixtures__/spec_stackframes.json'
+import stackFrameFixture from './__fixtures__/getInvocationDetails_spec_stackframes.json'
 
 vi.mock('../../../src/cypress/source_map_utils', () => {
   return {

packages/extension/package.json

@@ -44,5 +44,10 @@
     "lib",
     "theme"
   ],
-  "nx": {}
+  "nx": {
+    "implicitDependencies": [
+      "@packages/server",
+      "@packages/socket"
+    ]
+  }
 }