The perform helper crashes QUnit tests when the task fails or is cancelled

[lolmaus]

The problem is that QUnit chooses to crash tests unconditionally on an unhandled rejected promise.

Say, I've got this basic setup:

  @dropTask
  @waitFor
  async _submitTask(): Promise<void> {
    return await this.product.save();
  }
  submitTask = taskFor(this._submitTask);

{{#if this.submitTask.isRunning}}
  Submitting...
{{else if this.subitTask.last.isError}}
  Something wrong happened:
  {{format-error this.submitTask.last.error}}
  <br>
  <button {{on "click" (perform this.submitTask)}}>Retry<button>
{{else}}
  <button {{on "click" (perform this.submitTask)}}>Submit<button>
{{/if}}

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:

    // ember-cli-mirage
    this.server.post('/products', { message: 'There was an error' }, 500);

    await click('button');

    assert.equal(
      find('.message').textContent.trim(),
      'Something wrong happened: "Ember Data Request POST /api/saved-configurations returned a 500'
    );

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:

  <button {{on "click" (perform this.submitTask)}}>Submit<button>

With this:

  <button {{on "click" this.submit}}>Submit<button>

  @action
  async submit(): Promise<void> {
    try {
      await this.submitTask.perform();
    } catch(e) {
      // We don't want tests to unconditionally crash when this happens.
      // The error will be handled by `submitTask.last.isError`
    }
  }

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

[maxfierke]

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.

[lolmaus]

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.

[maxfierke]

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: #409

[lolmaus]

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.

[maxfierke]

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 helper

Well, 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.

[lolmaus]

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:

https://ember-twiddle.com/29fdce356f214a97730791b20f28bccd?numColumns=2&openFiles=controllers.application%5C.js%2Ctests.acceptance.my-acceptance-test%5C.js

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. :)

[maxfierke]

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")

[lolmaus]

@maxfierke Thanks! 🙏