Closed lolmaus closed 2 years ago
Yeah, I think this is the case in ember-mocha as well. There's some general questions in ember-concurrency around error handling about whether we should throw errors when the error states are used, but this behavior is I guess by-design too. Would recommend using the setupOnerror
handler from @ember/test-helpers
to stub out the global error handler in the given test and use it to assert the specific error raised, or less preferably, make it a no-op. That should resolve the tests failing.
Would recommend using the
setupOnerror
handler from@ember/test-helpers
to stub out the global error handler
I had tried it like this:
debugger;
setupOnerror((error) => {
debugger;
});
I can see only the first debugger
firing. Then the test fails, and the second debugger
is never hit.
I guess this may depend on how/where you're using it
You could also try something like the swallow-error
helper in the description of this issue: https://github.com/machty/ember-concurrency/issues/409
I guess this may depend on how/where you're using it
Well, I have described exactly how I'm using it.
You could also try something like the
swallow-error
helper
Well, the point of this issue is to discus the possiblity of including this functionality into the perform
helper.
Is there even a case where you do want tests to fail when a {{perform}}
-activated task is cancelled? The very fact of using a template helper indicates that you're not trying to handle the error/cancellation from where you invoke it, but rather handle it via .last.isError
or even not handle at all.
I guess this may depend on how/where you're using it
Well, I have described exactly how I'm using it.
The snippet provided lacks context and doesn't make a runnable example I can faithfully reproduce, so it's difficult for me to infer from it why setupOnerror
might not be catching the error
You could also try something like the
swallow-error
helperWell, the point of this issue is to discus the possiblity of including this functionality into the
perform
helper.
ah, I missed that ask at the bottom. This is definitely something we could do in the {{perform}}
and probably should provide. I'll try to get to it this week. Would also accept a PR w/ tests if you have one ready.
Is there even a case where you do want tests to fail when a
{{perform}}
-activated task is cancelled? The very fact of using a template helper indicates that you're not trying to handle the error/cancellation from where you invoke it, but rather handle it via.last.isError
or even not handle at all.
if someone isn't using last.error
and things to check error state, we'd probably want errors to be raised, otherwise we're silently allowing them, which might be unexpected/undefined behavior. This is the tricky bit. ember-concurrency should have an opinion about this or not, but as-yet it allows both, and so both cases require a little bit of hoop-jumping. It would be great if we could detect whether last.error
is used or referenced or something. Maybe in 3.x we can change this behavior and require specifying how errors will be handled.
The snippet provided lacks context and doesn't make a runnable example I can faithfully reproduce, so it's difficult for me to infer from it why setupOnerror might not be catching the error
I've tried replicating it on Twiddle, and setupOnerror
does work there:
Not sure why that's not the case for me. Maybe because we're two Ember versions ahead, IDK...
if someone isn't using last.error and things to check error state, we'd probably want errors to be raised, otherwise we're silently allowing them, which might be unexpected/undefined behavior
It could be op-in:
<button
{{on "click" (perform this.submitTask "arg1" "arg2" catch=true)}}
>
It would be great if we could detect whether last.error is used or referenced or something
That would be black magic. Someone will be doomed to have a hair-pulling frustration over it. :)
yeah, I think opt-in is probably the way to go. will probably do something like throwOnError=false
with a nice useful error message when an error is thrown (something like "if you're handling errors elsewhere, pass throwOnError=false
to the perform
helper")
@maxfierke Thanks! 🙏
The problem is that QUnit chooses to crash tests unconditionally on an unhandled rejected promise.
Say, I've got this basic setup:
This is perfectly valid code: it adheres to Ember Concurrency guidelines and it works flawlessly in production.
The problem happens when I try to test the failed state:
This is also a perfectly valid test, yet QUnit chooses to crash it because the promise generated by the
perform
helper has been rejected and not handled.The workaround is to replace this:
With this:
This is a huge bummer, because this runtime boilerplate is absolutely unnecessary for production and only exists to please QUnit.
I believe this is an issue with QUnit and has nothing to do with Ember Concurrency. Yet, @rwjblue has confirmed that this QUnit behavior is intended and that QUnit was deliberately modified to behave like that.
Given that this QUnit behavior is official, it basically renders the
perform
helper faulty as it would crash the tests for failed and cancelled EC tasks.I would like to start a discussion regarding what can be done to mitigate this. E. g. can the
perform
helper internally catch the promise returned by the task?CC @simonihmig