Skip to content

Commit

Permalink
fix($q): reject should catch & forward exceptions thrown in errback
Browse files Browse the repository at this point in the history
  • Loading branch information
IgorMinar committed Aug 22, 2013
1 parent 7d188d6 commit 487e535
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 3 deletions.
7 changes: 6 additions & 1 deletion src/ng/q.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,12 @@ function qFactory(nextTick, exceptionHandler) {
then: function(callback, errback) {
var result = defer();
nextTick(function() {
result.resolve((isFunction(errback) ? errback : defaultErrback)(reason));
try {
result.resolve((isFunction(errback) ? errback : defaultErrback)(reason));
} catch(e) {
result.reject(e);
exceptionHandler(e);
}
});
return result.promise;
}
Expand Down
24 changes: 22 additions & 2 deletions test/ng/qSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,15 @@ describe('q', function() {
syncResolve(deferred, rejectedPromise.then());
expect(log).toEqual(['error(rejected)->reject(rejected)']);
});


it('should catch exceptions thrown in errback and forward them to derived promises', function() {
var rejectedPromise = q.reject('rejected');
rejectedPromise.then(null, error('Broken', 'catch me!', true)).
then(null, error('Affected'))
mockNextTick.flush();
expect(log).toEqual(['errorBroken(rejected)->throw(catch me!)', 'errorAffected(catch me!)->reject(catch me!)']);
});
});


Expand Down Expand Up @@ -1401,6 +1410,17 @@ describe('q', function() {
});


describe('in reject', function() {
it('should log exceptions thrown in errback of a rejected promise', function() {
var rejectedPromise = q.reject('rejected');
rejectedPromise.then(null, error('Broken', 'catch me!', true));
mockNextTick.flush();
expect(logStr()).toBe('errorBroken(rejected)->throw(catch me!)');
expect(mockExceptionLogger.log).toEqual(['catch me!']);
});
});


describe('in when', function() {
it('should log exceptions thrown in a success callback and reject the derived promise',
function() {
Expand Down Expand Up @@ -1460,7 +1480,7 @@ describe('q', function() {
deferred = q.defer();
});


afterEach(function() {
// Restore the original exception logging mode
mockNextTick.logExceptions = originalLogExceptions;
Expand All @@ -1474,7 +1494,7 @@ describe('q', function() {
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);
Expand Down

1 comment on commit 487e535

@mhevery
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Please sign in to comment.