Any unhandled error causes acceptance tests to fail, making it impossible to test error states

[lolmaus]

Hi!

In our app, we catch errors in order to display flash messages to the user, letting them be aware of an error and offering a chance to retry.

We choose to re-throw the error after displaying the flash message. This is because we want the error to be caught by an error tracking service such as Sentry, Bugsnag, TrackJS, Raygun, etc.

The problem is that QUnit tests would crash on any uncaught error, making it impossible to test error reporting.

There are two userland workarounds from this issue:

1. Choose not to re-throw an error. I don't like this approach because it would require adding manual error tracking commands into every catch block.
2. Re-throwing conditionally, only in non-test environment. This is more or less OK, but I don't feel comfortable adding exceptions for tests. I believe, it's a slippery slope.

Previously, it was possible to override Ember.Test.adapter.exception and prevent tests from failing on uncaught assertions. The best part is that you could check for a condition (e. g. a certain error code or message) and re-throw an error if it was not the one you were expecting.

But according to ember-qunit source, it is no longer possible in modern Ember:

// Ember 2.17 and higher do not require the test adapter to have an `exception`
// method When `exception` is not present, the unhandled rejection is
// automatically re-thrown and will therefore hit QUnit's own global error
// handler (therefore appropriately causing test failure)

How do I prevent QUnit from crashing on unhandled errors? What is the modern replacement for overriding Ember.Test.adapter.exception?

[lolmaus]

Note that this is not a problem for ember-mocha. Mocha-driven tests do not crash when Ember Data rejects a promise.

[lolmaus]

Here's what I figured out:

1. QUnit is designed to fail tests on all errors, including unhandled rejected promises. Mocha won't do that unless you use an assertion against a rejected promise.

2. QUnit does not have an API to squelch those errors, making a test pass. There is assert.throws, but:

   • It can't be used with Cucumber-style tests, which execute a linear list of steps, and steps can't be nested.
   • It seems to be synchronous. If I get it right, you can't use it with async code and instead have to catch the failing promise.
   • But there is no access to Ember Data promises from an acceptance test.

3. Ember.onerror is an old and quite poorly documented feature of Ember:

   • It is mentioned once in the guides as a simple hook for Sentry/Bugsnag.
   • It's never mentioned in the API docs.
   • It does not have an export.
   • It does not offer pub/sub, meaning two addons can't use it.
   • ember-mocha does not seem to use it.
   • ember-qunit does use it (see below), but does not document it, keeping us clueless.

4. I had an impression that Ember.onerror is a candidate for deprecation and should be avoided. @amk221 kindly helped me understand that it has special behavior in ember-qunit: catching an error in Ember.onerror would prevent QUnit from failing a test!

5. At the same time, QUnit would ensure Ember.onerror is not catching all errors. In case it does, you should see a failing test case. To avoid this, you need to re-throw errors after catching them in Ember.onerror.

6. @amk221 pointed out that Ember.onerror only catches errors that are thrown within the Ember runloop. For some reason, this code:

   async someMethod() {
     try {
       await this.store.query(/*...*/);
     } catch (e) {
       throw e;
     }
   }

   ...would throw the error outside of the runloop, and Ember.onerror will ignore it!

   The solution is to wrap it with run from '@ember/runloop'. This is the first time I've had to use run in app code.

Now I have control over errors in tests again, which I lost after Ember.Test.adapter.exception was removed.

Big thanks to @amk221 for taking time and effort to help me figure this out.

---

Though my immediate problem is resolved, I'm gonna leave this issue open because:

• Ember.onerror behavior in tests needs to be documented.
• I would like to start a discussion over the future of Ember.onerror. We probably need an RFC to introduce a tool that can be imported and allows pub/sub, so that it can be used without overwriting other uses.

[amk221]

Just to follow up on the conversation @lolmaus and I had on discord

The pseudo code that helped was:

Ember.onerror = error => {
  if (/* i want to ignore the error */) {
    return;                                 // 1
  }

  if (Ember.testing) {
    throw error;                            // 2
  }
}

1. Be able to test error states in the test suite
2. Satisfy validateErrorHandler

[rwjblue]

FWIW, setupOnerror (from @ember/test-helpers) is specifically attempting to make that one off setup easier.

[rwjblue]

To specifically answer the original question:

How do I prevent QUnit from crashing on unhandled errors? What is the modern replacement for overriding Ember.Test.adapter.exception?

You would do something akin to:

import { setupOnerror } from '@ember/test-helpers';

test('....', async function(assert) {
  setupOnerror((error) => {
    // do nothing, throw, whatever you want
  });

  await visit('/whatever');
  // ...more stuff...
});

[amk221]

@rwjblue Thanks, that works great, but only if you use RSVP. Here is a basic example demonstrating the problem.

Imagine you have an app that used to have:

actions: {
  doSomethingAsync() {
    return this.service.doSomething().then(() => {
      this.flash.add('done');
    }).catch(error => {
      this.flash.add('failed');
      throw error;
    });
  }
}

...but now you want to modernise to use async/await...

@action
async doSomethingAsync() {
  try {
    await this.service.doSomething();
    this.flash.add('done');
  } catch(error) {
    this.flash.add('failed');
    throw error;
  }
}

setupOnerror is never called, and the opportunity to squelch the thrown error never happens, so the test fails.

[rwjblue]

I think it fully depends on the actual test in question. Generally speaking, there is no magic bullet for handling native promise rejections. The fact that it is possible in RSVP has been a convenient crutch for us for a while, but just doesn't match our future reality.

[lolmaus]

there is no magic bullet for handling native promise rejections

Please let me reiterate as I'm struggling to understand. Point out where I make a mistake or a false assumption.

The problem is that QUnit chooses to crash tests on all unhandled promise rejections (with exceptions mentioned above).

Given that QUnit is part of Ember apps by default (and the only real alternative, ember-mocha, is falling behind and not quite usable), we need a solution for this.

If the case of failed tests on unhandled native promises "works as intended", then we need guidelines how to deal with unhadled promises. E. g. should the developer always take care of catching all promises that they spawn?

It's not that obvious, because some async methods are hooks which are handled by Ember internals (sometimes too aggressively — you might not see the error at all, or it might be caught and rethrown elsewhere with an entirely irrelevant stack trace), but others like an action in @amk221's example are not handled automatically.

So two points here:

1. A typical Ember developer does not have a habit of catching promises because Ember guides never state that they should, and it works in most cases until it suddenly doesn't, and it's absolutely frustrating.
   If promises must never be left uncaught, guides need to be updated to develop this habit in Ember users.
2. How do we even handle promises? Take @amk221's example. What can be done to resolve the problem? The only thing I can think of is replacing throw error with something like this.metrics.trackError(error). Is that what we're supposed to do?

PS Thank you for looking into this matter.

[rwjblue]

@lolmaus - I'll try to reply to the various sections independently (as there are a few separate points you are making).

The problem is that QUnit chooses to crash tests on all unhandled promise rejections (with exceptions mentioned above).

FWIW, I think this is the correct stance for QUnit to take (and IIRC I implemented it waaaaaaay back in 2017 qunitjs/qunit#1241). Unhandled exceptions (and unhandled promise rejections) are bad, and in most circumstances point to an actual problem in the application.

If the case of failed tests on unhandled native promises "works as intended", then we need guidelines how to deal with unhandled promises

I totally agree with you here! We absolutely should document how to handle situations like @amk221's.

It's not that obvious, because some async methods are hooks which are handled by Ember internals (sometimes too aggressively — you might not see the error at all, or it might be caught and rethrown elsewhere with an entirely irrelevant stack trace), but others like an action in @amk221's example are not handled automatically.

I agree that it is unclear how Ember's internals handle unhandled rejections. I think we should update the documentation for all async route hooks to explicitly say what happens when an exception occurs. For example, if an exception occurs during model and you have an error substate or error action that rejection is automatically handled for you (this is why you don't commonly see the problem you are reporting for that category of code). I also think that doing that documentation will significantly help folks think about how exceptions / rejections are handled by the system (and therefore how they should be handled by their own code).

A typical Ember developer does not have a habit of catching promises because Ember guides never state that they should, and it works in most cases until it suddenly doesn't, and it's absolutely frustrating.

To reiterate, I agree with you. We should increase both API and guide documentation for exceptions/rejections and how application developers should handle those situations. FWIW, I do not think the answer is "never leave them uncaught" (as sometimes you really do intend for an error / rejection to bubble all the way up loudly).

How do we even handle promises? Take @amk221's example. What can be done to resolve the problem? The only thing I can think of is replacing throw error with something like this.metrics.trackError(error). Is that what we're supposed to do?

I am totally on board with the idea that we should identify specific use case / scenarios, and solve for them. That will help us know what to document (and where). Unfortunately, the example doesn't have enough information. What is triggering this action? What does the test look like? What kind of test? etc.

---

Ultimately, I think we should (mostly in order):

• attempt to summarize the cases that we are trying to work through (no, the general "unhandled rejections" bucket is not good enough 😝 )
• ensure that they are all possible to test
• determine what changes are needed (either here in ember-qunit, in @ember/test-helpers, ember-source, etc) to make those tests a tad less onerous (IMHO the tests making it clear that an unhandled rejection will be happening is totally fine)
• once we understand the scenarios, create detailed issues (or PRs 😉) for the API docs and guides that we think we need

[rwjblue]

Oh, also, I wanted to point out that this bit (from the original issue text) just isn't a factor here:

Previously, it was possible to override Ember.Test.adapter.exception and prevent tests from failing on uncaught assertions. The best part is that you could check for a condition (e. g. a certain error code or message) and re-throw an error if it was not the one you were expecting.

Anything that Ember.Test.adapter.exception would have caught is handleable (IMHO in a better way) with setupOnerror from @ember/test-helpers.

The conversation here recently is specifically about native promises (Ember.Test.adapter.exception could only ever catch RSVP promises and sync errors thrown from the run loop).

[amk221]

Thanks!

the example doesn't have enough information

I'll try and explain a bit more... I use the errors for flow control 🙈, e.g.

@action
attempt() {
  return this.validate()
   .then(() => this.save())
   .then(() => this.modal.close(();
}

this.validate throws, so that this.save never happens. It is that that is causing me grief. I realise it's frowned upon to use errors for flow control, so I could stop doing that. But I thought I'd let you know that's where my pain point was.

[rwjblue]

@amk221

I thought I'd let you know that's where my pain point was.

Gotcha. Where are you calling this.attempt() from? What kind of test is this?

Would you mind making a repo with a small demo?

[gossi]

I've also come across this issue today, where setupOnerror couldn't catch the exception. It slipped through QUnit and caused the test to fail. Good thing is I'm quite clear on what I did and have a temporary workaround.

The "expected behavior": I do abort a transition under a specific condition transition.abort().

Original Tests:

    const lock = confirmService.lockCurrentRoute();
    await visit('/route-b');
    assert.equal(currentURL(), '/route-a');

(I'm trying to visit route-b which is blocked). This resulted in:

Which then I wanted to test with:

    setupOnerror(function (error) {
        assert.ok(error); // or even more precise: assert.equal(error.name, 'TransitionAborted');
    });
    const lock = confirmService.lockCurrentRoute();
    await visit('/route-b');
    assert.equal(currentURL(), '/route-a');
    setupOnerror();

Here is the actual workaround:

    const lock = confirmService.lockCurrentRoute();
    try {
      await visit('/route-b');
    } catch (error) {
      console.log(error);
    }
    assert.equal(currentURL(), '/route-a');

With the log showing this:

Hope this brings some insights.

[amk221]

Would you mind making a repo with a small demo?

Sure: https://github.com/amk221/-ember-error-flow-control

[rreckonerr]

Just encountered the same issue while upgrading ember-qunit from 4.6.0 to 5.1.3

[MrChocolatine]

Same here, is this planned to be fixed/updated by any chance?

[jelhan]

I run into another case of errors, which aren't going through Ember.onerror and therefore can not be handled with setupOnerror in tests: Event listeners assigned with {{on}} modifier.

Assume we have this template-only component:

<button {{on "click" @onClick}} type="button">
  click me
</button>

Now we want to write a test for it, that it does not silently catch errors thrown by function passed to @onClick argument. My first attempt would look like this:

test('it does not catch errors thrown by @onClick handler', async function (assert) {
  const expectedError = new Error();

  setupOnerror((error) => {
    assert.step('error is thrown');
    assert.equal(error, expectedError);
  });

  this.set('clickHandler', () => {
    throw expectedError;
  });
  await render(hbs`<Button @onClick={{this.clickHandler}} />`);
  await click('button');

  assert.verifySteps(['error is thrown']);
});

But the error is not handled by Ember.onerror. Therefore setupOnerror is not working. Without monkey-patching QUnit's internals it is impossible to test this.

Please find a reproduction here: jelhan/reproduce-component-event-handler-throws-testing-issues@b950017...master It also contains a very hacky work-a-round, which requires monkey-patching QUnit.

This is driven by a real-world use case in Ember Bootstrap. A component provided by Ember Bootstrap silently catches errors of the event handler. This is considered a bug. Fixing the bug is easy. Adding tests for it is way more challenging. And even worse: Other existing tests, which tests behavior in case event handler rejects, are affected by that bug fix as well. So fixing the bug without adding tests is not an option. It would require even to disable existing tests. 😭

[rreckonerr]

Hi @rwjblue !
Any progress on this one?

[lolmaus]

Ran into this problem again with a test that intentionally returns HTTP 500 from a mocked API endpoint. QUnit chooses to crash a test on a rejected promise.

setupOnerror is not doing anything for this case.

An HTTP 500 error should be not an unexpected case, and this is how it is handled gracefully (simplified):

{{#if this.submitTask.isRunning}}
  Submitting...
{{else if this.subitTask.last.isError}}
  Something wrong happened:
  {{format-error this.submitTask.last.error}}
{{/if}}

I was able to resolve it like this.

Before:

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

After:

<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`
    }
  }

So this action only exists to wrap the Ember Concurrency task with a do-nothing try-catch block.

Apparently, the {{perform}} helper of Ember Concurrency should be considered faulty because it won't catch failed or cancelled tasks, causing tests to fail. 🤷‍♂️

I don't think that rejected promises should crash tests. Handling it requries too much runtime boilerplate that is absolutely unnecessary for production and only exists to please QUnit.

As for aborted transitions, I don't even know how to wrap them with a try-catch.

CC @simonihmig

[snewcomer]

https://github.com/snewcomer/ember-stateful-promise/blob/f151a254f1ad7a060e474abd551e686e83723b8f/tests/integration/components/playground-test.js#L13

This is what I did for a very specific use case of purposefully rejecting promises in the library.

[jamescdavis]

@snewcomer I came here to say https://api.qunitjs.com/extension/QUnit.onUncaughtException/, which is available and public API as of [email protected], works nicely.

FWIW, I've opened DefinitelyTyped/DefinitelyTyped#57770 to update the qunit types to include this hook.

[buschtoens]

I've started a PR to finally fix this: ember-test-helpers#1194. I would appreciate your thoughts!

[sandeep49m]

I am getting issue with if(Ember.testing ) { throw error} condition -
I have added the Ember.onerror in my App.js route, also there is one component in node-module where there is Ember.onerror is use with if(Ember.testing ) { throw error}. So when I generate the error it goes to inside the node-module component, whereas i expected it will trigger my App.js route.

Please can someone help me how to keep my App.js onerror function to trigger instead of the node-module component?

[NullVoxPopuli]

There are some techniques / platform APIs to catch errors: emberjs/ember-test-helpers#1128 (comment)

I don't know the timing of QUnit.onUncaughtException and the browser-based unhandledrejection event (what the above explores), but given that both these tools now exist, I think we can close this issue (as the problem isn't really anything wrong with ember-qunit, per se (feel free to disagree tho! 😅 I can be convinced!)).

There is a desire among a good few (many?) to have setupOnerror also listen to this type of error event, so maybe there is a PR in someone's future to add that capability? 🥳

[amk221]

It seems to me like setupOnerror is a poor mans try-catch. Why can't we do this:

test('something we know will fail', async function() {
  assert.expect(1);

  try {
    await visit('/');
  } catch(error) {
    // squelch, since we are testing an error scenario
  }
});

[jelhan]

What we should target for:

await assert.rejects(
  visit('/')
);

await assert.rejects(
  click('button')
);

But I fear there are architectural limitations in Ember.js itself, which prevents it for most use cases...

Last time I tried working around the limitations with QUnit.onUncaughtException and unhandlerejection browser event, I run into many issue. And it was breaking over time with upgrades of Ember, QUnit, and other libraries. Maybe it got better. But it's still far away from a good testing story for error cases.

[NullVoxPopuli]

Why can't we do this

This is actually how you test beforeModel transitions aborts 😅
See: emberjs/ember-test-helpers#332

which prevents it for most use cases...

yeah, we're hoping all the router-related limitations here go away when we get to Polaris