-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Conversation
Thanks for taking the time to open a PR!
|
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
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
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 |
test was moved into reporter.errors.spec.js
in production/CI, the file is cypress_runner.js, so it doesn't work
it's covered by the isolated runner tests
There was a problem hiding this 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 👍
/** | ||
* 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. | ||
*/ |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// cy.fail(err) |
Hi @chrisbreiding is there an example of how to disable this? |
@zwass Yes, using the uncaught:exception event. |
Thank you! |
onunhandledrejection
as errors? #243User facing changelog
window.onerror
.uncaught:exception
event with the promise as the third argument.Additional details
Instead of use
window.onerror
to catch uncaught errors, we now usewindow.addEventListener('error')
. Before, if the app overwrotewindow.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 fromuncaught:exception
to get those tests to pass.PR Tasks
cypress-documentation
? Uncaught exception handling updates cypress-documentation#3615type definitions
?cypress.schema.json
?