From d59027c40ed73fa9e114706d0c5a885785311dec Mon Sep 17 00:00:00 2001
From: Pete Bacon Darwin <pete@bacondarwin.com>
Date: Wed, 31 Jul 2013 10:09:23 +0100
Subject: [PATCH] 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.
---
 src/ng/q.js      |  4 ++--
 test/ng/qSpec.js | 51 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/src/ng/q.js b/src/ng/q.js
index 2b75b3c82839..bf235640cc72 100644
--- a/src/ng/q.js
+++ b/src/ng/q.js
@@ -215,8 +215,8 @@ function qFactory(nextTick, exceptionHandler) {
             try {
               result.resolve((callback || defaultCallback)(value));
             } catch(e) {
-              exceptionHandler(e);
               result.reject(e);
+              exceptionHandler(e);
             }
           };
 
@@ -224,8 +224,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 941b4f2ea63d..085918effc2b 100644
--- a/test/ng/qSpec.js
+++ b/test/ng/qSpec.js
@@ -101,6 +101,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) {
@@ -110,7 +111,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);
+            }
           }
         });
       }
@@ -836,4 +839,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();
+    });
+  });
 });