setupOnError not working

[amk221]

Here are two test cases.

One where a component errors in constructor, and another where it errors in response to a click.

The first can be caught by setupOnError, and ignored in the test suite. 👍🏻
The second is not caught by setupOnError, and so the test fails. 👎🏻

[jfdnc]

Interestingly, this causes your failing test here to respect setupOnerror and pass:

<button type="button" {{on "click" (action this.check)}}> // using `action` template helper
  check
</button>

with the example invocation binding via @action window object onerror called

using (action helper invokes error in the correct context

[jfdnc]

@amk221 Looks like this is a known issue, there is some discussion in the latter part of this thread in the ember-qunit repo, which it looks like you were part of but there has been more discussion since your last comment there. Seems to be currently unresolved.

[buschtoens]

I got fed up with this long-standing issue, so I decided to invest a bit of time and do some detective work. 🕵️😄

This occurs, when an error is thrown outside of the Ember run loop. setupOnerror works by hooking into Ember.onerror, which in turn is called by Backburner (Ember's backing run loop implementation) when a task in the run loop throws.

The error then propagates to the global window.onerror handler or window.onunhandledrejection, if it's async.

It still shows up as a global failure in your QUnit test, because QUnit catches these via QUnit.onUncaughtException, like @snewcomer correctly pointed out in the issue that you linked: emberjs/ember-qunit#592 (comment)

Running outside of a run loop can easily happen by accident, for instance when you manually add an event listener to an element and don't wrap the listener in run().

import { run } from '@ember/runloop';

element.addEventListener('click', () => {
  run(() => {
    throw Error('caught by `setupOnerror`');
  });
});

element.addEventListener('click', () => {
  throw Error('NOT caught by `setupOnerror`');
})

And in fact, if you change your check action to throw the error inside a run loop, the test passes!

import Component from '@glimmer/component';
import { action } from '@ember/object';
import { run } from '@ember/runloop';

export default class MyComponent extends Component {
  @action
  check() {
    run(() => {
      throw new Error('check failed');
    });
  }
}

Now the Ember Guides claim that

For basic Ember app development scenarios, you don't need to understand the run loop or use it directly. All common paths are paved nicely for you and don't require working with the run loop.

This would lead me to believe that using {{on "click" this.check}} and the click test helper should just work™ out of the box. So why doesn't it?

Ember transparently uses Glimmer's {{on}} modifier. Glimmer itself is not integrated with Backburner nor the Ember runloop. It registers the passed in listener as-is without wrapping it in run() first.

I would consider this is a bug, because it violates the promise made in the Ember Guides that you don't need to concern yourself with run when using Ember's standard tools. It doesn't really matter for real-world apps, because Ember will helpfully wrap calls that schedule tasks into a run loop queue into an implicit autorun loop, if they are not called from inside a run loop. The only negative effect of this is slightly worse performance.

This only really becomes an issue in tests, because Ember.onerror is not set up to handle errors thrown outside of a run loop, like explained above.

But the trouble doesn't stop there. Regular "native" DOM events initiated by a user action / the browser are handled asynchronously as opposed to synthetic events, i.e. events triggered manually via dispatchEvent:

Unlike "native" events, which are fired by the browser and invoke event handlers asynchronously via the event loop, dispatchEvent() invokes event handlers synchronously. All applicable event handlers are called and return before dispatchEvent() returns.

click and any other DOM test helpers firing events use dispatchEvent via fireEvent. And since dispatchEvent triggers the event listeners synchronously, one would hope that, even if setupOnerror doesn't work, the error would at least propagate through to click:

try {
  await click('.some-element-that-throws-on-click');
} catch (error) {
  console.log(error); // This should log the error from the click handler.
}

Alas, that also doesn't work: #310, #453, #440

But why? Even though dispatchEvent calls all listeners synchronously, it doesn't propagate any of the errors they might throw. So there again, the only option is to handle them in a global error handler. As of now, that's window.onerror, which QUnit hooks into and then fails the test with the out of band global failure, like explained above.

Ideally though, the handler would be wrapped in a run loop, so that Ember.onerror can catch it instead. That would allow for handling it via setupOnerror, but the click helper still wouldn't throw, because it doesn't hook into Ember.onerror to watch for errors thrown during the execution of the event listeners.

[amk221]

What's the situation with this? It's happening more and more.

eslint-plugin-ember recently added a no-runloop rule.

We were using run(() => {} to temporarily fix this.

[NullVoxPopuli]

Don't use runloop to 'get around' this.

You can use try/catch for exceptions from click events, or assert.throws.

:) <3

setupOnerror is only for runloop stuff, which is why putting stuff in runloop queues makes errors show up there.

It's a separate discussion to have setupOnerror also listen to window error events (where errors during click would end up).
That could be nice!

[amk221]

How are we supposed to test error scenarios (unhappy paths) in acceptance tests though? They cause the tests to fail, which is why we use setupOnError to squelch it.

[NullVoxPopuli]

Can you provide a minimal repro of your specific example?

It's totally valid atm to use setupOnerror for rendering errors and assert.throws for others, atd try/catch for others - which should cover your bases)

[amk221]

Sure, amk221/-ember-unhappy-path-test@2797b67

It's related to this emberjs/ember-qunit#592 as well

[NullVoxPopuli]

there are no failing tests on the main branch :-\

[amk221]

It’s commented as to why

[NullVoxPopuli]

gotchya, well, I discovered some things, so thank you <3

It seems this is related to a limitation in The Platform for certain styles of throwing errors.

I made a demo here to demonstrate (and to remove all possible ember/qunit suspicions)

https://jsbin.com/jakecim/edit?html,output

In particular, when catching errors (event listening, etc), "uncaught (in promise)" errors always reach the "root" / console / browser, and it doesn't seem there is a way to "stopPropagation", as you would with click event handlers.

This seems to back up the experience of others as well:

• https://stackoverflow.com/questions/62100577/event-preventdefault-does-not-seem-to-work-async-context

  After a few experiments I seem to find that the only way I can find to prevent this is to swallow the error (using try...catch) at some point, preferably at the highest async call in the stack (or rather in the sequence of calls of async functions), if that can be determined.

So, in the JSBin demo, I've added a checkbox (enabled by default) which makes the click-handler async/await (so that the click handler does not lose track of the async stack), and then all 3 scenarios (async, promise chain, and sync), are correctly caught and don't bubble up to the window -- unchecking this checkboxe reproduces the behavior you're experiencing in your app -- I believe this solution is in alignment with what others have discovered as well (the stackoverflow, for example).

I do see a bit of varying behavior between Chrome and Firefox though (which is in alignment with the stackoverflow post as well). In particular, Chrome doesn't seem to "reduntantly" print the error in the the jsbin demo. So it's possible that this JSBin has the solution ya'll are looking for, if you only want to test against Chrome(based) browsers.

that solution is probably this:

window.addEventListener('unhandledrejection', (event) => {
  console.info('unhandledrejection handler called');
  event.preventDefault()
}); 

specifically, event.preventDefault() (but again, only in chrome(based) browsers (also, I don't know if this is a bug in FireFox or not (the difference in behavior)))

I'm going to close this issue, as I don't think there is anything better we can do with setupOnerror.

If someone can come up with some way "preventDefault" / "stopPropagation" on error-event handlers, at some point in the future, let's open up a new issue and discuss there (or better, a PR! ✨✨✨ ) -- as I do agree with ya'll that it'd be great to have some unified way to test/assert/capture errors in tests.

[amk221]

Thanks for the investigation.

Where does that leave us though? It's impossible to test certain things, consider:

export default class extends Component {
  constructor() {
    super(...arguments);

    assert('@foo must be provided', 'foo' in this.args);
  }
}

test('foo', async function(assert) {
  assert.expect(1);

  setupOnerror((error) => {
    if (
      error.message === 'Assertion Failed: @foo must be provided'
    ) {
      assert.step('ensures correct usage');
      return;
    }

    throw error;
  });

  await render(hbs`<MyComponent />`);

  assert.verifySteps(['ensures correct usage']);
});

[mansona]

@amk221 I wonder about a test like that though 🤔 the implementation of assert is that it it won't run in a production build so that it's less strict and would throw fewer errors for end-users. assert itself is a helpful tool for developers to identify problems during development and not something that we should be testing.

I understand that this could just be used for an illustration but the reason I am asking is because it could be that you should be testing at a different layer rather than trying to "catch" exceptions thrown in constructors of components 🤔

[amk221]

Yeh, I know they are stripped, it's to help guide developers. But yes, for example purposes only. There are still scenarios that are untestable.

[NullVoxPopuli]

It's impossible to test certain things
There are still scenarios that are emberjs/ember-qunit#592 (comment).

This isn't true -- #1128 (comment) provided ways to capture all sorts of errors, exceptions, from anywhere.

Now, if you mean with setupOnerror, then sure. Let's explore improving setupOnerror for users of chrome-based browsers 🎉

[amk221]

This isn't true -- #1128 (comment) provided ways to capture all sorts of errors, exceptions, from anywhere.

🤔 I must be missing something glaringly obvious.... Because nothing I've seen can get the acceptance tests to pass once the error reaches the window. Neither the tools from @ember/test-helpers, nor Qunit, nor browser native

I'm happy to wash my hands of setupOnerror, if there was a solution that isn't a "probably" solution, but an actual one.

[NullVoxPopuli]

Made an example: amk221/-ember-unhappy-path-test#1

There is another example of how to do this here: qunitjs/qunit#1419 (comment) -- albeit, not contextually ergonomic (nor does it actually work -- as QUnit's properties are readonly (only QUnit.config is mutable)).

So, I've opened this issue on the QUnit repo, where I think we should figure out some solution for QUnit v3. qunitjs/qunit#1736

Maybe it's qunit that needs to provide setupOnError / resetOnError (or similar) for handling this situation.