-
-
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
fix(errorHandler): async error handling for watchers #9484
Conversation
I'm waiting for this. |
no need to squash your commits, we already do when merging 😉 |
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.
Hey, sorry for the delay and thank you for this!
Could you add a test case in the errorCaptured.spec.js
file as well? The test should pass already as the invokeWithErrorHandling
already handles it, but it will be nice to have a test case for it 😀
The rest seems to be good!
@posva Thank you for the review, i added some extra test case to the |
@posva - Is there an ETA for this merge ? |
I just spent half an hour trying to figure out why my error handler wasn't getting called because I assumed all callback handling was already updated to support async functions. Would be really nice if this could be merged. |
Great work @hello-brsd! Until this is merged, I'm having to write an interim solution using axios interceptors along with vuex. 😬 @posva Thanks for approving the PR. Any updates on merge ETA? |
Sorry, I cannot provide any ETA 😓 |
Any updates? |
I would also like to know what the status is on this because I've been bitten by this multiple times by now, and the fix seems very straightforward. I see Vue 2.x hasn't had a release with anything besides bug fixes for a while, so is this something you would even consider merging anymore? Or is all the focus on Vue 3 now? |
Any updates on this? Our error handling would be greatly improved by this because we use a lot of async watches in our code. I'd really like to avoid having to resort to workarounds! |
Can this please be reviewed? |
@OzCroot Looks like this is already approved a while ago, @posva any newer ETA for this to land? Documentation for
and there is a
Overall one would expect from documentation that async watchers would work with global error handler, but currently it is not so 🙂 |
Here`s a fix I came up with until this PR is merged.
|
Added async error handling for watchers and immediate watchers
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:
Fixes #9625
Fixes #10009