Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: give proper error message when err.stack is undefined #5313

Merged
merged 9 commits into from
Nov 8, 2019
2 changes: 2 additions & 0 deletions packages/driver/src/cypress/cy.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,8 @@ create = (specWindow, Cypress, Cookies, state, config, log) ->
## since this failed this means that a
## specific command failed and we should
## highlight it in red or insert a new command

err.name = err.name || 'CypressError'
errors.commandRunningFailed(err)

fail(err, state("runnable"))
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cypress/utils.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ module.exports = {
## because the browser has a cached
## dynamic stack getter that will
## not be evaluated later
stack = err.stack
stack = err.stack or ''

## preserve message
## and toString
Expand Down
89 changes: 89 additions & 0 deletions packages/server/__snapshots__/3_issue_1669_spec.coffee.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
exports['e2e issue 2891 passes 1'] = `

====================================================================================================

(Run Starting)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (issue_1669_spec.js) │
│ Searched: cypress/integration/issue_1669_spec.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: issue_1669_spec.js (1 of 1)


issue-1669 undefined err.stack in beforeEach hook
1) "before each" hook for "cy.setCookie should fail with correct error"


0 passing
1 failing

1) issue-1669 undefined err.stack in beforeEach hook "before each" hook for "cy.setCookie should fail with correct error":
CypressError: cy.setCookie() must be passed an RFC-6265-compliant cookie value. You passed:

\` bar\`

Because this error occurred during a 'before each' hook we are skipping the remaining tests in the current suite: 'issue-1669 undefined err.st...'
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line




(Results)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 1 │
│ Passing: 0 │
│ Failing: 1 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 1 │
│ Video: true │
│ Duration: X seconds │
│ Spec Ran: issue_1669_spec.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Screenshots)

- /XXX/XXX/XXX/cypress/screenshots/issue_1669_spec.js/issue-1669 undefined err.sta (1280x720)
ck in beforeEach hook -- cy.setCookie should fail with correct error -- before e
ach hook (failed).png


(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: /XXX/XXX/XXX/cypress/videos/issue_1669_spec.js.mp4 (X second)


====================================================================================================

(Run Finished)


Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✖ issue_1669_spec.js XX:XX 1 - 1 - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✖ 1 of 1 failed (100%) XX:XX 1 - 1 - -


`
15 changes: 15 additions & 0 deletions packages/server/test/e2e/3_issue_1669_spec.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
e2e = require("../support/helpers/e2e")
Fixtures = require("../support/helpers/fixtures")

describe "e2e issue 2891", ->
e2e.setup()

## https://github.com/cypress-io/cypress/issues/2891

it "passes", ->
e2e.exec(@, {
spec: "issue_1669_spec.js"
snapshot: true
browser: 'chrome'
expectedExitCode: 1
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
describe('issue-1669 undefined err.stack in beforeEach hook', () => {
beforeEach(() => {
cy.setCookie('foo', ' bar')
Copy link
Contributor

@flotwig flotwig Nov 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkucera if you merge in develop, I believe this was fixed to err correctly and no longer causes an err.stack === undefined, you may need to come up with a custom command to get the error you want

Also, just curious, is there a reason why is this an e2e test and not just a normal driver test that asserts on the error using cy.on('fail', (err) => {...})?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you catch the error in the 'fail' handler, it has not yet hit the logic that reads off .stack

I'll merge in develop, and try locally without these changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, it did pass without the src changes, so I changed the test that now fails without these changes

})

it('cy.setCookie should fail with correct error', () => {
expect(true).ok
})
})