diff options
| author | Pete Bacon Darwin | 2013-07-31 10:09:23 +0100 |
|---|---|---|
| committer | Brian Ford | 2013-08-12 16:23:37 -0700 |
| commit | d59027c40ed73fa9e114706d0c5a885785311dec (patch) | |
| tree | 05ef864ab0430122ecefb6adfe76fce1240d0f56 | |
| parent | c197c2aa277d7f68e3faa18fa67cf58072423233 (diff) | |
| download | angular.js-d59027c40ed73fa9e114706d0c5a885785311dec.tar.bz2 | |
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.
| -rw-r--r-- | src/ng/q.js | 4 | ||||
| -rw-r--r-- | 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 2b75b3c8..bf235640 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 941b4f2e..085918ef 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(); + }); + }); }); |
