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(); +    }); +  });  }); | 
