Merge pull request #262 from BackburnerJS/late-bound-onerror

Allow onError function to late bound, and recalculated on each flush
BackburnerJS · Aug 25, 2017 · c32ca81 · c32ca81
2 parents 883ce82 + 140d2ae
commit c32ca81
Show file tree

Hide file tree

Showing 3 changed files with 1,950 additions and 1,546 deletions.
diff --git a/lib/backburner/queue.ts b/lib/backburner/queue.ts
@@ -20,8 +20,6 @@ export default class Queue {
     this.name = name;
     this.options = options;
     this.globalOptions = globalOptions;
-
-    this.globalOptions.onError = getOnError(globalOptions);
   }
 
   public push(target, method, args, stack) {
@@ -56,8 +54,6 @@ export default class Queue {
     let method;
     let args;
     let errorRecordedForStack;
-    let onError = this.globalOptions.onError;
-    let invoke = onError ? this.invokeWithOnError : this.invoke;
 
     this.targetQueues = Object.create(null);
     let queueItems;
@@ -72,37 +68,42 @@ export default class Queue {
       before();
     }
 
-    for (let i = this.index; i < queueItems.length; i += 4) {
-      this.index += 4;
-
-      target                = queueItems[i];
-      method                = queueItems[i + 1];
-      args                  = queueItems[i + 2];
-      errorRecordedForStack = queueItems[i + 3]; // Debugging assistance
-
-      // method could have been nullified / canceled during flush
-      if (method !== null) {
-        //
-        //    ** Attention intrepid developer **
-        //
-        //    To find out the stack of this task when it was scheduled onto
-        //    the run loop, add the following to your app.js:
-        //
-        //    Ember.run.backburner.DEBUG = true; // NOTE: This slows your app, don't leave it on in production.
-        //
-        //    Once that is in place, when you are at a breakpoint and navigate
-        //    here in the stack explorer, you can look at `errorRecordedForStack.stack`,
-        //    which will be the captured stack when this job was scheduled.
-        //
-        //    One possible long-term solution is the following Chrome issue:
-        //       https://bugs.chromium.org/p/chromium/issues/detail?id=332624
-        //
-        invoke(target, method, args, onError, errorRecordedForStack);
-      }
+    let invoke;
+    if (queueItems.length > 0) {
+      let onError = getOnError(this.globalOptions);
+      invoke = onError ? this.invokeWithOnError : this.invoke;
+      for (let i = this.index; i < queueItems.length; i += 4) {
+        this.index += 4;
+
+        target                = queueItems[i];
+        method                = queueItems[i + 1];
+        args                  = queueItems[i + 2];
+        errorRecordedForStack = queueItems[i + 3]; // Debugging assistance
+
+        // method could have been nullified / canceled during flush
+        if (method !== null) {
+          //
+          //    ** Attention intrepid developer **
+          //
+          //    To find out the stack of this task when it was scheduled onto
+          //    the run loop, add the following to your app.js:
+          //
+          //    Ember.run.backburner.DEBUG = true; // NOTE: This slows your app, don't leave it on in production.
+          //
+          //    Once that is in place, when you are at a breakpoint and navigate
+          //    here in the stack explorer, you can look at `errorRecordedForStack.stack`,
+          //    which will be the captured stack when this job was scheduled.
+          //
+          //    One possible long-term solution is the following Chrome issue:
+          //       https://bugs.chromium.org/p/chromium/issues/detail?id=332624
+          //
+          invoke(target, method, args, onError, errorRecordedForStack);
+        }
 
-      if (this.index !== this._queueBeingFlushed.length &&
-        this.globalOptions.mustYield && this.globalOptions.mustYield()) {
-        return QUEUE_STATE.Pause;
+        if (this.index !== this._queueBeingFlushed.length &&
+          this.globalOptions.mustYield && this.globalOptions.mustYield()) {
+          return QUEUE_STATE.Pause;
+        }
       }
     }
 

diff --git a/tests/run-test.ts b/tests/run-test.ts
@@ -151,7 +151,7 @@ QUnit.test('onError set after start', function(assert) {
 });
 
 QUnit.test('onError with target and action', function(assert) {
-  assert.expect(2);
+  assert.expect(3);
 
   let target = {};
 
@@ -167,4 +167,14 @@ QUnit.test('onError with target and action', function(assert) {
   };
 
   bb.run(() => { throw new Error('QUnit.test error'); });
+
+  target['onerror'] = function() { };
+
+  bb.run(() => { throw new Error('QUnit.test error'); });
+
+  target['onerror'] = function(error) {
+    assert.equal('QUnit.test error', error.message);
+  };
+
+  bb.run(() => { throw new Error('QUnit.test error'); });
 });