Skip to content

Commit

Permalink
[BUGFIX lts] Revert "[BUGFIX] Changed backburner's error handler to u…
Browse files Browse the repository at this point in the history
…se `dispatchError` instead of `onError`. This is so that backburner errors can be caught by the `Test.adapter`. Fixes #14864."

This reverts commit 196442d which
essentially made all error handling for things that are run-wrapped
async, dramatically impacting development ergonomics.

The originally reported issue is a _very real_ problem that we need to
guard against. To reproduce that issue, the following conditions must
exist:

* The application must implement `Ember.onerror` in a way that does not
  rethrow errors.
* Throw an error during anything that uses `run.join`. The example
scenario had a template like this:

```hbs
<button {{action 'throwsAnError'}}>Click me!</button>
```

To fix this error swallowing behavior, the commit being reverted made
all errors hit within the run loop use `dispatchError`, which (during
tests) has the default implementation of invoking `QUnit`'s
`assert.ok(false)`. Unfortunately, this meant that it is now impossible
to use a standard `try` / `catch` to catch errors thrown within anything
"run-wrapped".

For example, these patterns were no longer possible after the commit in
question:

```js
try {
  Ember.run(() => {
    throw new Error('This error should be catchable');
  });
} catch(e) {
  // this will never be hit during tests...
}
```

This ultimately breaks a large number of test suites that rely
(rightfully so!) on being able to do things like:

```js
module('service:foo-bar', function(hooks) {
  setupTest(hooks);

  hooks.beforeEach(() => {
    this.owner.register('service:whatever', Ember.Service.extend({
      someMethod(argumentHere) {
        Ember.assert('Some random argument validation here', !argumentHere);
      }
    });
  });

  test('throws when argumentHere is missing', function(assert) {
    let subject = this.owner.lookup('service:foo-bar');

    assert.throws(() => {
      run(() =>
        subject.someMethod());
    }, /random argument validation/);
  });
});
```

The ergonomics of breaking standard JS `try` / `catch` is too much, and
therefore the original commit is being reverted.
  • Loading branch information
rwjblue committed Nov 28, 2017
1 parent 2272e0e commit 8360825
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion packages/ember-metal/lib/error_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ let getStack = error => {
let onerror;
export const onErrorTarget = {
get onerror() {
return dispatchOverride || onerror;
return onerror;
}
};

Expand Down

0 comments on commit 8360825

Please sign in to comment.