-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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
feat(errors): sync/async error handling for lifecycle hooks and v-on handlers (#7653, #6953) #8395
Conversation
Can we add error handling for v-on handlers to this? Both sync and async need to be caught for them. I have working code and tests for this here: https://github.com/coldino/vue/commits/event-handler-errors. It would need very minor updates to work with the changes in this PR. This will also address #6953. |
@coldino |
@enkot The changes will overlap significantly as I would switch to using your handlePromiseError function, so I will create a PR for your branch. Note that in addition to the ones you implemented, I also added async error handling for:
Currently there are only tests for the v-on invoked function async+sync errors. I don't have enough experience with the testing system to add tests for all of them. If you are able to assist with this we can have much greater coverage for async error catching. I will keep the hooks without tests in a separate commit so you can include/exclude as you choose. |
Improves the check for promise-like objects to ensure they are thenable. Includes tests.
I have only included the v-on checks in the PR as I didn't want to include changes with no tests and possibly delay this PR. |
Add handling for v-on normal+async errors (vuejs#6953)
…t cases for multiple event handlers
This comment has been minimized.
This comment has been minimized.
…handlers (vuejs#8395) close vuejs#6953, close vuejs#7653
Does it support async methods which is called from hooks like
I checked it with |
Update:
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
dev
branch for v2.x (or to a previous version branch), not themaster
branchfix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: