-
Notifications
You must be signed in to change notification settings - Fork 668
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
Async exceptions in beforeMount not caught in unit tests #1503
Comments
I see the exception in the console, but the test is passing in the sandbox you shared. |
That said this fails on my local machine. Should this be getting caught in the test runner? Maybe relevant: vuejs/vue#10826 and a few others. I am not sure how we can catch this error in the runner from the test. |
Yeah, for some reason it passed in the sandbox, but fails locally too. I assume it's something with the way codesandbox is setup. Sorry for the confusion. I would expect it to be caught since I'm telling Jest to expect an assertion. And if you remove the async/await from |
I move the reproduction steps a git repo to make it easier to test. https://github.com/Yodablues/async-lifecycle-example There are 2 components, Basically, I want to to test that an exception gets thrown in an |
Thanks for this. I haven't found a way to catch this error - I'll try a little bit more... let me know if you come up with anything. VTU has some logic around catching errors, potential bug there:
|
Hey @Yodablues. I took a look into your reproduction repo. There was a conflict with the VTU version in I was able to catch the error in the This gives us a few options we could potentially pursue
You will also get this annoying error if you implement the above
Unfortunately, the below does not work due to how
But this is the behavior we would desire. I believe, fairly easily, we could implement something that could look like this:
We can support virtually any of the options present in the Global Config and handle this behavior accordingly. I think this would allow, in this case, the Sorry for the essay here, but I think in general the community could likely benefit from a feature like this. What do you think @lmiller1990 ? |
Great summary.
If there is an easy way to implement this feature via Also, we should see how VTU next handles this problem/if the same thing happens there. Vue 3 internals (and VTU next) is quite different, although the API is the same. |
@lmiller1990 Does VTU |
Yes VTU 2.x has it see here. Looks like it isn't documented. I made an issue for documenting it vuejs/test-utils-docs#49 I have no idea if Vue 3.x has custom errorHandler (probably does). We should find out if this is a problem in VTU 2.x and go from there. |
Sounds like a plan. I can put an example together that we talked about above using VTU |
@lmiller1990 finally had a chance to try this out with VTU The reprod link for this is here. The question now is, how do we want to support this functionality here? // @ts-ignore
import Async from "./Async.vue";
import { shallowMount } from '@vue/test-utils'
import { createApp } from 'vue'
describe("Test Error Handler Vue 3", () => {
it("Mounts a global errorhandler without issue", async (done) => {
const app = createApp({})
app.config.errorHandler = (err: any, vm, info) => {
if(info.includes("beforeMount")){
expect(err.message).toBe("test")
}
done()
}
shallowMount(Async, {
global: {
// @ts-ignore
config: {
errorHandler: app.config.errorHandler
}
}
});
});
}); |
Just to confirm: using the custom errorHandler strategy works fine in VTU V2? If we can implement it here easily with |
@lmiller1990 Correct. Besides the typings issue it worked great!
We should be able to add it relatively easy without a lot of effort. From what I can see, a) move the argument to the second argument space to implement the below RFC const localVue = createLocalVue({
errorHandler(){
// if provided, call this error handler, followed by the global error handler if one exists
},
optionMergeStrategies(){
// some implementation
}
// other global config options
}) I can (or anyone else if they want to!) start work on getting the feature implemented and open a PR with the proposed design/ RFC. Not really a true RFC, but probably for this use case it should be fine. And we can take it from there. What do you think? |
I have never seen anyone use the first argument to We could make this next release 1.1, since it has a both adds a new feature and has a "breaking" change (which is fine for an internal, undocumented API). |
@lmiller1990 @Yodablues I took a stab at implementing this feature in #1670. Lemme know what you all think! |
@lmiller1990 @Yodablues Should we close this issue out since the feature to solve this issue is now implemented in |
Version
1.0.0-beta.33
Reproduction link
https://codesandbox.io/s/white-dream-1vmod
Steps to reproduce
Create a new component that throws an exception in an async beforeMount. In my Test.vue component, I create a simple component that waits 500ms and then throws an exception.
Create a test that asserts exceptions are being thrown when mount/shallowMount are called.
In my test, I expect.assertions(1) to let Jest know to expect a single exception. Then I wrapped shallowMount in a try/catch and call done() in finally.
What is expected?
Exceptions would be caught in the test suite.
What is actually happening?
Exceptions are not caught and the test fails.
In non-async beforeMount, exceptions are caught, however they still get logged to the console.
The text was updated successfully, but these errors were encountered: