From 85fd26935ea504f1e486b3c1e4778789f1145589 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 22 Nov 2017 11:03:21 -0500 Subject: [PATCH] [BUGFIX lts] Revert "[BUGFIX] Changed backburner's error handler to use `dispatchError` instead of `onError`. This is so that backburner errors can be caught by the `Test.adapter`. Fixes #14864." This reverts commit 196442dc17b2511495b68087b1c5e817c70d62da 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 ``` 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. --- packages/ember-metal/lib/error_handler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ember-metal/lib/error_handler.js b/packages/ember-metal/lib/error_handler.js index 2ebdb29bae9..72ce2c47412 100644 --- a/packages/ember-metal/lib/error_handler.js +++ b/packages/ember-metal/lib/error_handler.js @@ -16,7 +16,7 @@ let getStack = error => { let onerror; export const onErrorTarget = { get onerror() { - return dispatchOverride || onerror; + return onerror; } };