fix($q): call `reject()` even if `$exceptionHandler` rethrows

Normally $exceptionHandler doesn't throw an exception.  It is normally
used just for logging and so on.  But if an application developer
implemented a version that did throw an exception then $q would never
have called reject() when converting an exception thrown inside a `then`
handler into a rejected promise.

2 files changed, 52 insertions, 3 deletions

diff --git a/src/ng/q.js b/src/ng/q.js
index fe05b37f..b68660d5 100644
--- a/src/ng/q.js
+++ b/src/ng/q.js
@@ -237,8 +237,8 @@ function qFactory(nextTick, exceptionHandler) {
             try {
               result.resolve((callback || defaultCallback)(value));
             } catch(e) {
-              exceptionHandler(e);
               result.reject(e);
+              exceptionHandler(e);
             }
           };
 
@@ -246,8 +246,8 @@ function qFactory(nextTick, exceptionHandler) {
             try {
               result.resolve((errback || defaultErrback)(reason));
             } catch(e) {
-              exceptionHandler(e);
               result.reject(e);
+              exceptionHandler(e);
             }
           };
 
diff --git a/test/ng/qSpec.js b/test/ng/qSpec.js
index 316f8add..6d08cb15 100644
--- a/test/ng/qSpec.js
+++ b/test/ng/qSpec.js
@@ -160,6 +160,7 @@ describe('q', function() {
       mockNextTick.queue.push(task);
     },
     queue: [],
+    logExceptions: true,
     flush: function() {
       if (!mockNextTick.queue.length) throw new Error('Nothing to be flushed!');
       while (mockNextTick.queue.length) {
@@ -169,7 +170,9 @@ describe('q', function() {
           try {
             task();
           } catch(e) {
-            dump('exception in mockNextTick:', e, e.name, e.message, task);
+            if ( mockNextTick.logExceptions ) {
+              dump('exception in mockNextTick:', e, e.name, e.message, task);
+            }
           }
         });
       }
@@ -1393,4 +1396,50 @@ describe('q', function() {
       });
     });
   });
+
+
+  describe('when exceptionHandler rethrows exceptions, ', function() {
+    var originalLogExceptions, deferred, errorSpy, exceptionExceptionSpy;
+
+    beforeEach(function() {
+      // Turn off exception logging for these particular tests
+      originalLogExceptions = mockNextTick.logExceptions;
+      mockNextTick.logExceptions = false;
+
+      // Set up spies
+      exceptionExceptionSpy = jasmine.createSpy('rethrowExceptionHandler')
+      .andCallFake(function rethrowExceptionHandler(e) {
+        throw e;
+      });
+      errorSpy = jasmine.createSpy('errorSpy');
+
+
+      q = qFactory(mockNextTick.nextTick, exceptionExceptionSpy);
+      deferred = q.defer();
+    });
+
+    
+    afterEach(function() {
+      // Restore the original exception logging mode
+      mockNextTick.logExceptions = originalLogExceptions;
+    });
+
+
+    it('should still reject the promise, when exception is thrown in success handler, even if exceptionHandler rethrows', function() {
+      deferred.promise.then(function() { throw 'reject'; }).then(null, errorSpy);
+      deferred.resolve('resolve');
+      mockNextTick.flush();
+      expect(exceptionExceptionSpy).toHaveBeenCalled();
+      expect(errorSpy).toHaveBeenCalled();
+    });
+    
+
+    it('should still reject the promise, when exception is thrown in success handler, even if exceptionHandler rethrows', function() {
+      deferred.promise.then(null, function() { throw 'reject again'; }).then(null, errorSpy);
+      deferred.reject('reject');
+      mockNextTick.flush();
+      expect(exceptionExceptionSpy).toHaveBeenCalled();
+      expect(errorSpy).toHaveBeenCalled();
+    });
+  });
 });