From 96bd60de6bd7d6f71211e79b33df564055e9cd2a Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Tue, 30 Sep 2025 14:46:24 -0500 Subject: [PATCH] fix: ensure that code frames within cy.origin point to the right line of the code (#32597) * fix: ensure that code frames within cy.origin point to the right line of the code * additional tests * additional tests * changelog * Apply suggestions from code review * Update system-tests/projects/e2e/cypress/e2e/cy_origin_error.cy.ts * fix tests * fix tests * clean up --- cli/CHANGELOG.md | 1 + packages/driver/src/cross-origin/origin_fn.ts | 5 +- .../driver/src/cy/commands/origin/index.ts | 5 +- packages/driver/src/cypress/chainer.ts | 6 +- packages/driver/src/cypress/cy.ts | 18 ++++- packages/driver/src/cypress/stack_utils.ts | 37 ++++++++++ .../test/unit/cypress/stack_utils.spec.ts | 69 +++++++++++++++++++ .../e2e/cypress/e2e/cy_origin_error.cy.ts | 26 +++++-- .../projects/e2e/cypress/support/util.js | 5 +- system-tests/test/cy_origin_error_spec.ts | 7 +- 10 files changed, 165 insertions(+), 14 deletions(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 67781dcc54..4245aefcf5 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -6,6 +6,7 @@ _Released 10/07/2025 (PENDING)_ **Bugfixes:** - Fixed a regression introduced in [`15.0.0`](https://docs.cypress.io/guides/references/changelog#15-0-0) where `dbus` connection error messages appear in docker containers when launching Cypress. Fixes [#32290](https://github.com/cypress-io/cypress/issues/32290). +- Fixed code frames in `cy.origin` so that failed commands will show the correct line/column within the corresponding spec file. Addressed in [#32597](https://github.com/cypress-io/cypress/pull/32597). **Misc:** diff --git a/packages/driver/src/cross-origin/origin_fn.ts b/packages/driver/src/cross-origin/origin_fn.ts index 3a6106886e..7869ea04d0 100644 --- a/packages/driver/src/cross-origin/origin_fn.ts +++ b/packages/driver/src/cross-origin/origin_fn.ts @@ -179,8 +179,11 @@ export const handleOriginFn = (Cypress: Cypress.Cypress, cy: $Cy) => { return } + const currentAssertionUserInvocationStack = cy.state('current').get('currentAssertionCommand')?.get('userInvocationStack') + const userInvocationStack = cy.state('current').get('userInvocationStack') + cy.stop() - Cypress.specBridgeCommunicator.toPrimary('queue:finished', { err }, { syncGlobals: true }) + Cypress.specBridgeCommunicator.toPrimary('queue:finished', { err, crossOriginUserInvocationStack: currentAssertionUserInvocationStack || userInvocationStack }, { syncGlobals: true }) }) // the name of this function is used to verify if privileged commands are diff --git a/packages/driver/src/cy/commands/origin/index.ts b/packages/driver/src/cy/commands/origin/index.ts index 4a903e5512..d714cbb093 100644 --- a/packages/driver/src/cy/commands/origin/index.ts +++ b/packages/driver/src/cy/commands/origin/index.ts @@ -118,8 +118,10 @@ export default (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy, state: State reject(err) } - const onQueueFinished = ({ err, subject, unserializableSubjectType }) => { + const onQueueFinished = ({ err, crossOriginUserInvocationStack, subject, unserializableSubjectType }) => { if (err) { + err.crossOriginUserInvocationStack = crossOriginUserInvocationStack + return _reject(err) } @@ -223,6 +225,7 @@ export default (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy, state: State autLocation: Cypress.state('autLocation')?.href, crossOriginCookies: Cypress.state('crossOriginCookies'), isProtocolEnabled: Cypress.state('isProtocolEnabled'), + originUserInvocationStack: userInvocationStack, }, config: preprocessConfig(Cypress.config()), env: preprocessEnv(Cypress.env()), diff --git a/packages/driver/src/cypress/chainer.ts b/packages/driver/src/cypress/chainer.ts index 869d57901b..482adf685b 100644 --- a/packages/driver/src/cypress/chainer.ts +++ b/packages/driver/src/cypress/chainer.ts @@ -22,10 +22,14 @@ export class $Chainer { $Chainer.prototype[key] = function (...args) { const privilegeVerification = Cypress.emitMap('command:invocation', { name: key, args }) - const userInvocationStack = $stackUtils.normalizedUserInvocationStack( + let userInvocationStack = $stackUtils.normalizedUserInvocationStack( (new this.specWindow.Error('command invocation stack')).stack, ) + if (cy.state('originUserInvocationStack')) { + userInvocationStack = $stackUtils.mergeCrossOriginUserInvocationStack(userInvocationStack, cy.state('originUserInvocationStack')) + } + // call back the original function with our new args // pass args an as array and not a destructured invocation fn(this, userInvocationStack, args, privilegeVerification) diff --git a/packages/driver/src/cypress/cy.ts b/packages/driver/src/cypress/cy.ts index 3b85f5cb55..e036c15095 100644 --- a/packages/driver/src/cypress/cy.ts +++ b/packages/driver/src/cypress/cy.ts @@ -390,7 +390,11 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert err.stack = $stackUtils.normalizedStack(err) - const userInvocationStack = $errUtils.getUserInvocationStack(err, this.state) + let userInvocationStack = err.crossOriginUserInvocationStack + + if (!userInvocationStack) { + userInvocationStack = $errUtils.getUserInvocationStack(err, this.state) + } err = $errUtils.enhanceStack({ err, @@ -740,7 +744,11 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert cy.linkSubject(chainer.chainerId, cy.state('chainerId')) } - const userInvocationStack = $stackUtils.captureUserInvocationStack(cy.specWindow.Error) + let userInvocationStack = $stackUtils.captureUserInvocationStack(cy.specWindow.Error) + + if (cy.state('originUserInvocationStack')) { + userInvocationStack = $stackUtils.mergeCrossOriginUserInvocationStack(userInvocationStack, cy.state('originUserInvocationStack')) + } callback(chainer, userInvocationStack, args, privilegeVerification, true) @@ -843,7 +851,11 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert cy.linkSubject(chainer.chainerId, cy.state('chainerId')) } - const userInvocationStack = $stackUtils.captureUserInvocationStack(cy.specWindow.Error) + let userInvocationStack = $stackUtils.captureUserInvocationStack(cy.specWindow.Error) + + if (cy.state('originUserInvocationStack')) { + userInvocationStack = $stackUtils.mergeCrossOriginUserInvocationStack(userInvocationStack, cy.state('originUserInvocationStack')) + } callback(chainer, userInvocationStack, args, privilegeVerification) diff --git a/packages/driver/src/cypress/stack_utils.ts b/packages/driver/src/cypress/stack_utils.ts index ae4875a4ea..f0907804fd 100644 --- a/packages/driver/src/cypress/stack_utils.ts +++ b/packages/driver/src/cypress/stack_utils.ts @@ -519,11 +519,47 @@ const normalizedUserInvocationStack = (userInvocationStack) => { || line.includes('Chainer.prototype[key]') || line.includes('cy.') || line.includes('$Chainer.') + // Remove cross origin stack lines + || line.includes('SpecBridgeCommunicator') + || (line.includes('at invokeOriginFn') && !line.includes('at eval')) }).join('\n') return normalizeStackIndentation(nonCypressStackLines) } +const mergeCrossOriginUserInvocationStack = (userInvocationStack: string, originUserInvocationStack: string) => { + // The method here is: + // 1. Grab the line/column from the first line of the origin user invocation stack + // 2. Add it to and replace the line/column of the first line of the user invocation stack + // + // Note: If any of our assumptions about the stack format are violated, this will just + // return the original user invocation stack to avoid breaking the stack trace. + const originStackLines = getStackLines(originUserInvocationStack) + const userStackLines = getStackLines(userInvocationStack) + + if (userStackLines.length === 0 || originStackLines.length === 0) return userInvocationStack + + // Note: chrome adds a parenthesis to the end of the stack line and firefox does not + const userStackMatch = userStackLines[0].match(/(\d+):(\d+)\)?$/) + + if (!userStackMatch) return userInvocationStack + + const userLine = Number(userStackMatch[1]) + const userColumn = Number(userStackMatch[2]) + + // Note: chrome adds a parenthesis to the end of the stack line and firefox does not + const originStackMatch = originStackLines[0].match(/(\d+):(\d+)\)?$/) + + if (!originStackMatch) return userInvocationStack + + const originLine = Number(originStackMatch[1]) + + // Note: chrome adds a parenthesis to the end of the stack line and firefox does not so we need to keep whatever we find + const newUserStackLine = `${originStackLines[0].replace(/(\d+:\d+)(\)?)$/, `${originLine + userLine - 1}:${userColumn}$2`)}` + + return userInvocationStack.replace(userStackLines[0], newUserStackLine) +} + export default { replacedStack, getCodeFrame, @@ -543,4 +579,5 @@ export default { stackWithUserInvocationStackSpliced, captureUserInvocationStack, getInvocationDetails, + mergeCrossOriginUserInvocationStack, } diff --git a/packages/driver/test/unit/cypress/stack_utils.spec.ts b/packages/driver/test/unit/cypress/stack_utils.spec.ts index f9d9970657..1a912e8890 100644 --- a/packages/driver/test/unit/cypress/stack_utils.spec.ts +++ b/packages/driver/test/unit/cypress/stack_utils.spec.ts @@ -65,4 +65,73 @@ describe('stack_utils', () => { }) } }) + + describe('normalizedUserInvocationStack', () => { + it('should remove cross origin stack lines', () => { + const userInvocationStack = ` at cy. [as prompt] (cypress:///../driver/src/cypress/cy.ts:657:86) + at eval (eval at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts), :2:16) + at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts:176:42) + at SpecBridgeCommunicator.eval (cypress:///../driver/src/cross-origin/origin_fn.ts:180:21)` + const normalizedUserInvocationStack = stack_utils.normalizedUserInvocationStack(userInvocationStack) + + expect(normalizedUserInvocationStack).toEqual(` at eval (eval at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts), :2:16)`) + }) + }) + + describe('mergeCrossOriginUserInvocationStack', () => { + it('should merge line numbers from origin stack into user stack', () => { + const userInvocationStack = ` at eval (eval at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts), :2:16)` + const originUserInvocationStack = ` at Context.eval (http://localhost:9500/__cypress/tests?p=cypress/e2e/run/cross-origin.cy.ts:14:12)` + + const result = stack_utils.mergeCrossOriginUserInvocationStack(userInvocationStack, originUserInvocationStack) + + // The first line should have line number 100 + 657 - 1 = 756, column should remain 86 + expect(result).toContain(' at Context.eval (http://localhost:9500/__cypress/tests?p=cypress/e2e/run/cross-origin.cy.ts:15:16)') + }) + + it('should handle different stack formats and preserve the rest of the stack', () => { + const userInvocationStack = ` at cy. [as click] (cypress:///../driver/src/cypress/cy.ts:10:20) + at eval (eval at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts), :2:16) + at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts:176:42) + at SpecBridgeCommunicator.eval (cypress:///../driver/src/cross-origin/origin_fn.ts:180:21)` + + const originUserInvocationStack = ` at cy. [as click] (cypress:///../driver/src/cypress/cy.ts:5:30) + at eval (eval at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts), :1:10)` + + const result = stack_utils.mergeCrossOriginUserInvocationStack(userInvocationStack, originUserInvocationStack) + + // Line should be 5 + 10 - 1 = 14, column should remain 20 + expect(result).toContain('cypress:///../driver/src/cypress/cy.ts:14:20') + // Rest of the stack should be preserved + expect(result).toContain('at eval (eval at invokeOriginFn') + expect(result).toContain('at invokeOriginFn') + expect(result).toContain('at SpecBridgeCommunicator.eval') + }) + + it('should handle edge case where origin line is 1', () => { + const userInvocationStack = ` at cy. [as click] (cypress:///../driver/src/cypress/cy.ts:3:15) + at eval (eval at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts), :2:16)` + + const originUserInvocationStack = ` at cy. [as click] (cypress:///../driver/src/cypress/cy.ts:1:25) + at eval (eval at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts), :1:10)` + + const result = stack_utils.mergeCrossOriginUserInvocationStack(userInvocationStack, originUserInvocationStack) + + // Line should be 1 + 3 - 1 = 3, column should remain 15 + expect(result).toContain('cypress:///../driver/src/cypress/cy.ts:3:15') + }) + + it('should handle edge case where user line is 1', () => { + const userInvocationStack = ` at cy. [as click] (cypress:///../driver/src/cypress/cy.ts:1:15) + at eval (eval at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts), :2:16)` + + const originUserInvocationStack = ` at cy. [as click] (cypress:///../driver/src/cypress/cy.ts:5:25) + at eval (eval at invokeOriginFn (cypress:///../driver/src/cross-origin/origin_fn.ts), :1:10)` + + const result = stack_utils.mergeCrossOriginUserInvocationStack(userInvocationStack, originUserInvocationStack) + + // Line should be 5 + 1 - 1 = 5, column should remain 15 + expect(result).toContain('cypress:///../driver/src/cypress/cy.ts:5:15') + }) + }) }) diff --git a/system-tests/projects/e2e/cypress/e2e/cy_origin_error.cy.ts b/system-tests/projects/e2e/cypress/e2e/cy_origin_error.cy.ts index 519b45e56e..85638a5ec5 100644 --- a/system-tests/projects/e2e/cypress/e2e/cy_origin_error.cy.ts +++ b/system-tests/projects/e2e/cypress/e2e/cy_origin_error.cy.ts @@ -19,8 +19,8 @@ describe('cy.origin errors', () => { }) verify('command failure', this, { - line: 16, - column: 8, + line: 17, + column: 10, message: 'Expected to find element', stack: ['cy_origin_error.cy.ts'], before () { @@ -37,8 +37,26 @@ describe('cy.origin errors', () => { }) verify('failure when using dependency', this, { - line: 32, - column: 8, + line: 35, + column: 10, + message: 'Expected to find element', + stack: ['cy_origin_error.cy.ts'], + before () { + cy.visit('/primary_origin.html') + }, + // Skip title validation here since the command is deep enough in the test that it does not show the command title + skipTitleValidation: true, + }) + + fail('failure when using assertion', this, () => { + cy.origin('http://www.foobar.com:4466', () => { + cy.get('#doesnotexist', { timeout: 1 }).should('exist') + }) + }) + + verify('failure when using assertion', this, { + line: 53, + column: 47, message: 'Expected to find element', stack: ['cy_origin_error.cy.ts'], before () { diff --git a/system-tests/projects/e2e/cypress/support/util.js b/system-tests/projects/e2e/cypress/support/util.js index c608d5337b..177456d9f8 100644 --- a/system-tests/projects/e2e/cypress/support/util.js +++ b/system-tests/projects/e2e/cypress/support/util.js @@ -24,6 +24,7 @@ export const verify = (title, ctx, options) => { column, message, stack, + skipTitleValidation, } = options const codeFrameFileRegex = new RegExp(`${Cypress.spec.relative}:${line}${column ? `:${column}` : ''}`) @@ -66,7 +67,9 @@ export const verify = (title, ctx, options) => { .invoke('text') .should('match', codeFrameFileRegex) - cy.get('.test-err-code-frame pre span').should('include.text', `fail('${title}',this,()=>`) + if (!skipTitleValidation) { + cy.get('.test-err-code-frame pre span').should('include.text', `fail('${title}',this,()=>`) + } cy.contains('.test-err-code-frame .runnable-err-file-path', openInIdePath.relative) }) diff --git a/system-tests/test/cy_origin_error_spec.ts b/system-tests/test/cy_origin_error_spec.ts index ac547e4076..a24c921b7e 100644 --- a/system-tests/test/cy_origin_error_spec.ts +++ b/system-tests/test/cy_origin_error_spec.ts @@ -34,7 +34,7 @@ describe('e2e cy.origin errors', () => { // keep the port the same to prevent issues with the snapshot port: PORT, spec: 'cy_origin_error.cy.ts', - expectedExitCode: 2, + expectedExitCode: 3, config: commonConfig, async onRun (exec) { const { stdout } = await exec() @@ -43,8 +43,9 @@ describe('e2e cy.origin errors', () => { expect(stdout).to.contain('Timed out retrying after 1ms: Expected to find element: `#doesnotexist`, but never found it.') // check to make sure stack trace contains the 'cy.origin' source - expect(stdout).to.contain('webpack://e2e/./cypress/e2e/cy_origin_error.cy.ts:16:7') - expect(stdout).to.contain('webpack://e2e/./cypress/e2e/cy_origin_error.cy.ts:32:7') + expect(stdout).to.contain('webpack://e2e/./cypress/e2e/cy_origin_error.cy.ts:17:9') + expect(stdout).to.contain('webpack://e2e/./cypress/e2e/cy_origin_error.cy.ts:35:9') + expect(stdout).to.contain('webpack://e2e/./cypress/e2e/cy_origin_error.cy.ts:53:46') }, }) })