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

Commit d59027c

Browse files
petebacondarwinbtford
authored andcommitted
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.
1 parent c197c2a commit d59027c

File tree

2 files changed

+52
-3
lines changed

2 files changed

+52
-3
lines changed

src/ng/q.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -215,17 +215,17 @@ function qFactory(nextTick, exceptionHandler) {
215215
try {
216216
result.resolve((callback || defaultCallback)(value));
217217
} catch(e) {
218-
exceptionHandler(e);
219218
result.reject(e);
219+
exceptionHandler(e);
220220
}
221221
};
222222

223223
var wrappedErrback = function(reason) {
224224
try {
225225
result.resolve((errback || defaultErrback)(reason));
226226
} catch(e) {
227-
exceptionHandler(e);
228227
result.reject(e);
228+
exceptionHandler(e);
229229
}
230230
};
231231

test/ng/qSpec.js

+50-1
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ describe('q', function() {
101101
mockNextTick.queue.push(task);
102102
},
103103
queue: [],
104+
logExceptions: true,
104105
flush: function() {
105106
if (!mockNextTick.queue.length) throw new Error('Nothing to be flushed!');
106107
while (mockNextTick.queue.length) {
@@ -110,7 +111,9 @@ describe('q', function() {
110111
try {
111112
task();
112113
} catch(e) {
113-
dump('exception in mockNextTick:', e, e.name, e.message, task);
114+
if ( mockNextTick.logExceptions ) {
115+
dump('exception in mockNextTick:', e, e.name, e.message, task);
116+
}
114117
}
115118
});
116119
}
@@ -836,4 +839,50 @@ describe('q', function() {
836839
});
837840
});
838841
});
842+
843+
844+
describe('when exceptionHandler rethrows exceptions, ', function() {
845+
var originalLogExceptions, deferred, errorSpy, exceptionExceptionSpy;
846+
847+
beforeEach(function() {
848+
// Turn off exception logging for these particular tests
849+
originalLogExceptions = mockNextTick.logExceptions;
850+
mockNextTick.logExceptions = false;
851+
852+
// Set up spies
853+
exceptionExceptionSpy = jasmine.createSpy('rethrowExceptionHandler')
854+
.andCallFake(function rethrowExceptionHandler(e) {
855+
throw e;
856+
});
857+
errorSpy = jasmine.createSpy('errorSpy');
858+
859+
860+
q = qFactory(mockNextTick.nextTick, exceptionExceptionSpy);
861+
deferred = q.defer();
862+
});
863+
864+
865+
afterEach(function() {
866+
// Restore the original exception logging mode
867+
mockNextTick.logExceptions = originalLogExceptions;
868+
});
869+
870+
871+
it('should still reject the promise, when exception is thrown in success handler, even if exceptionHandler rethrows', function() {
872+
deferred.promise.then(function() { throw 'reject'; }).then(null, errorSpy);
873+
deferred.resolve('resolve');
874+
mockNextTick.flush();
875+
expect(exceptionExceptionSpy).toHaveBeenCalled();
876+
expect(errorSpy).toHaveBeenCalled();
877+
});
878+
879+
880+
it('should still reject the promise, when exception is thrown in success handler, even if exceptionHandler rethrows', function() {
881+
deferred.promise.then(null, function() { throw 'reject again'; }).then(null, errorSpy);
882+
deferred.reject('reject');
883+
mockNextTick.flush();
884+
expect(exceptionExceptionSpy).toHaveBeenCalled();
885+
expect(errorSpy).toHaveBeenCalled();
886+
});
887+
});
839888
});

0 commit comments

Comments
 (0)