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: Improve uncaught error handling #14826

Merged
merged 34 commits into from
Mar 1, 2021

Conversation

chrisbreiding
Copy link
Contributor

@chrisbreiding chrisbreiding commented Jan 29, 2021

User facing changelog

  • Cypress now catches uncaught errors and fails the test even if the app-under-test has defined window.onerror.
  • Cypress now fails tests if there is an unhandled promise rejection in the app-under-test. Unhandled rejections will trigger the uncaught:exception event with the promise as the third argument.

Additional details

Instead of use window.onerror to catch uncaught errors, we now use window.addEventListener('error'). Before, if the app overwrote window.onerror, we would no longer catch any uncaught errors, but that's no longer the case.

We also now use window.addEventListener('unhandledrejection') to catch and fail if there's an unhandled promise rejection.

How has the user experience changed?

If a user's app overwrote window.onerror or had unhandled rejections, tests will fail that trigger errors/unhandled rejections. They will need to fix those errors in their app or return false from uncaught:exception to get those tests to pass.

PR Tasks

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jan 29, 2021

Thanks for taking the time to open a PR!

@chrisbreiding chrisbreiding changed the title fix: Improve async error handling fix: Improve uncaught error handling Feb 12, 2021
@chrisbreiding chrisbreiding marked this pull request as draft February 12, 2021 15:28
We now wrap timers in the AUT because if we do it in the
driver/top frame, any uncaught errors thrown in the timer callbacks
get picked up by the top frame's 'error' handler instead of the AUT's.
We need to wrap the timer callbacks in the AUT itself for errors to
propagate properly.
- refactor/consolidate the logic and the tests
- add promise to uncaught:exception if it's an unhandled rejection
@cypress
Copy link

cypress bot commented Feb 23, 2021



Test summary

9349 0 118 3Flakiness 1


Run details

Project cypress
Status Passed
Commit d52707e
Started Feb 26, 2021 7:36 PM
Ended Feb 26, 2021 7:46 PM
Duration 10:43 💡
OS Linux Debian - 10.5
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

cypress/integration/retries.ui.spec.js Flakiness
1 runner/cypress retries.ui.spec > opens attempt on each attempt failure for the screenshot, and closes after test passes

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@chrisbreiding chrisbreiding marked this pull request as ready for review February 26, 2021 14:27
Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

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

Changes make sense to me, code looks good too 👍

Comment on lines +1 to +8
/**
* This is the entry point for the script that gets injected into
* the AUT. It gets bundled on its own and injected into the <head>
* of the AUT by `packages/proxy`.
*
* If adding to this bundle, try to keep it light and free of
* dependencies.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm happy that this is no longer embedded as a string, good stuff

// cause an 'uncaught:exception' event. This error will be caught in
// top.onerror with stack as 5th argument.
// fail the test
// cy.fail(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// cy.fail(err)

@zwass
Copy link

zwass commented Jun 15, 2021

Hi @chrisbreiding is there an example of how to disable this?

@chrisbreiding
Copy link
Contributor Author

@zwass Yes, using the uncaught:exception event.

@zwass
Copy link

zwass commented Jun 15, 2021

Thank you!

@cypress-io cypress-io locked as resolved and limited conversation to collaborators Jun 15, 2021
@chrisbreiding chrisbreiding deleted the TR-119-async-error-handling branch April 5, 2022 18:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants