Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix($q): call reject() even if $exceptionHandler rethrows
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
petebacondarwin committed Jul 31, 2013
1 parent a644ca7 commit 664526d
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/ng/q.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,17 +237,17 @@ function qFactory(nextTick, exceptionHandler) {
try {
result.resolve((callback || defaultCallback)(value));
} catch(e) {
exceptionHandler(e);
result.reject(e);
exceptionHandler(e);
}
};

var wrappedErrback = function(reason) {
try {
result.resolve((errback || defaultErrback)(reason));
} catch(e) {
exceptionHandler(e);
result.reject(e);
exceptionHandler(e);
}
};

Expand Down
51 changes: 50 additions & 1 deletion test/ng/qSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
}
});
}
Expand Down Expand Up @@ -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();
});
});
});

0 comments on commit 664526d

Please sign in to comment.