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

Commit 8c2419d

Browse files
lgalfasolgalfaso
committed
feat($q): report promises with non rejection callback
Rejected promises that do not have a callback to handle the rejection report this to $exceptionHandler so they can be logged to the console. BREAKING CHANGE Tests that depend on specific order or number of messages in $exceptionHandler will need to handle rejected promises report. Closes: #13653 Closes: #7992
1 parent 986647a commit 8c2419d

File tree

12 files changed

+139
-38
lines changed

12 files changed

+139
-38
lines changed

‎src/ng/compile.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2494,7 +2494,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
24942494
childBoundTranscludeFn);
24952495
}
24962496
linkQueue = null;
2497-
});
2497+
}).then(noop, noop);
24982498

24992499
return function delayedNodeLinkFn(ignoreChildLinkFn, scope, node, rootElement, boundTranscludeFn) {
25002500
var childBoundTranscludeFn = boundTranscludeFn;

‎src/ng/interval.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,8 @@ function $IntervalProvider() {
188188
*/
189189
interval.cancel = function(promise) {
190190
if (promise && promise.$$intervalId in intervals) {
191+
// Interval cancels should not report as unhandled promise.
192+
intervals[promise.$$intervalId].promise.then(noop, noop);
191193
intervals[promise.$$intervalId].reject('canceled');
192194
$window.clearInterval(promise.$$intervalId);
193195
delete intervals[promise.$$intervalId];

‎src/ng/q.js

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,8 @@ function $$QProvider() {
243243
*/
244244
function qFactory(nextTick, exceptionHandler) {
245245
var $qMinErr = minErr('$q', TypeError);
246+
var queueSize = 0;
247+
var checkQueue = [];
246248

247249
/**
248250
* @ngdoc method
@@ -307,28 +309,67 @@ function qFactory(nextTick, exceptionHandler) {
307309
pending = state.pending;
308310
state.processScheduled = false;
309311
state.pending = undefined;
310-
for (var i = 0, ii = pending.length; i < ii; ++i) {
311-
deferred = pending[i][0];
312-
fn = pending[i][state.status];
313-
try {
314-
if (isFunction(fn)) {
315-
deferred.resolve(fn(state.value));
316-
} else if (state.status === 1) {
317-
deferred.resolve(state.value);
318-
} else {
319-
deferred.reject(state.value);
312+
try {
313+
for (var i = 0, ii = pending.length; i < ii; ++i) {
314+
state.pur = true;
315+
deferred = pending[i][0];
316+
fn = pending[i][state.status];
317+
try {
318+
if (isFunction(fn)) {
319+
deferred.resolve(fn(state.value));
320+
} else if (state.status === 1) {
321+
deferred.resolve(state.value);
322+
} else {
323+
deferred.reject(state.value);
324+
}
325+
} catch (e) {
326+
deferred.reject(e);
327+
exceptionHandler(e);
320328
}
321-
} catch (e) {
322-
deferred.reject(e);
323-
exceptionHandler(e);
329+
}
330+
} finally {
331+
--queueSize;
332+
if (queueSize === 0) {
333+
nextTick(processChecksFn());
334+
}
335+
}
336+
}
337+
338+
function processChecks() {
339+
while (!queueSize && checkQueue.length) {
340+
var toCheck = checkQueue.shift();
341+
if (!toCheck.pur) {
342+
toCheck.pur = true;
343+
var errorMessage = 'Possibly unhandled rejection: ';
344+
try {
345+
errorMessage += toJson(toCheck.value);
346+
} catch (e) {
347+
errorMessage += '[recursive value]';
348+
}
349+
exceptionHandler(errorMessage);
324350
}
325351
}
326352
}
327353

354+
function processChecksFn() {
355+
return function() { processChecks(); };
356+
}
357+
358+
function processQueueFn(state) {
359+
return function() { processQueue(state); };
360+
}
361+
328362
function scheduleProcessQueue(state) {
363+
if (!state.pending && state.status === 2 && !state.pur) {
364+
if (queueSize === 0 && checkQueue.length === 0) {
365+
nextTick(processChecksFn());
366+
}
367+
checkQueue.push(state);
368+
}
329369
if (state.processScheduled || !state.pending) return;
330370
state.processScheduled = true;
331-
nextTick(function() { processQueue(state); });
371+
++queueSize;
372+
nextTick(processQueueFn(state));
332373
}
333374

334375
function Deferred() {

‎src/ng/timeout.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ function $TimeoutProvider() {
8585
*/
8686
timeout.cancel = function(promise) {
8787
if (promise && promise.$$timeoutId in deferreds) {
88+
// Timeout cancels should not report an unhandled promise.
89+
deferreds[promise.$$timeoutId].promise.then(noop, noop);
8890
deferreds[promise.$$timeoutId].reject('canceled');
8991
delete deferreds[promise.$$timeoutId];
9092
return $browser.defer.cancel(promise.$$timeoutId);

‎src/ngMock/angular-mocks.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ angular.mock.$IntervalProvider = function() {
445445
promise = deferred.promise;
446446

447447
count = (angular.isDefined(count)) ? count : 0;
448-
promise.then(null, null, (!hasParams) ? fn : function() {
448+
promise.then(null, function() {}, (!hasParams) ? fn : function() {
449449
fn.apply(null, args);
450450
});
451451

@@ -505,6 +505,7 @@ angular.mock.$IntervalProvider = function() {
505505
});
506506

507507
if (angular.isDefined(fnIndex)) {
508+
repeatFns[fnIndex].deferred.promise.then(undefined, function() {});
508509
repeatFns[fnIndex].deferred.reject('canceled');
509510
repeatFns.splice(fnIndex, 1);
510511
return true;

‎src/ngResource/resource.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -702,12 +702,13 @@ angular.module('ngResource', ['ng']).
702702
return $q.reject(response);
703703
});
704704

705-
promise.finally(function() {
705+
promise = promise.finally(function(response) {
706706
value.$resolved = true;
707707
if (!isInstanceCall && cancellable) {
708708
value.$cancelRequest = angular.noop;
709709
timeoutDeferred = httpConfig.timeout = null;
710710
}
711+
return response;
711712
});
712713

713714
promise = promise.then(

‎test/ng/animateRunnerSpec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ describe("$$AnimateRunner", function() {
169169
var animationComplete = false;
170170
runner.finally(function() {
171171
animationComplete = true;
172-
});
172+
}).then(noop, noop);
173173
runner[method]();
174174
$rootScope.$digest();
175175
expect(animationComplete).toBe(true);

‎test/ng/compileSpec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1870,7 +1870,7 @@ describe('$compile', function() {
18701870
});
18711871
});
18721872

1873-
inject(function($compile, $templateCache, $rootScope, $exceptionHandler) {
1873+
inject(function($compile, $templateCache, $rootScope, $exceptionHandler, $timeout) {
18741874
// no root element
18751875
$templateCache.put('template.html', 'dada');
18761876
$compile('<p template></p>');

‎test/ng/httpSpec.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,7 +1031,7 @@ describe('$http', function() {
10311031

10321032
it('should $apply after error callback', function() {
10331033
$httpBackend.when('GET').respond(404);
1034-
$http({method: 'GET', url: '/some'});
1034+
$http({method: 'GET', url: '/some'}).then(noop, noop);
10351035
$httpBackend.flush();
10361036
expect($rootScope.$apply).toHaveBeenCalledOnce();
10371037
});
@@ -1432,7 +1432,7 @@ describe('$http', function() {
14321432

14331433
function doFirstCacheRequest(method, respStatus, headers) {
14341434
$httpBackend.expect(method || 'GET', '/url').respond(respStatus || 200, 'content', headers);
1435-
$http({method: method || 'GET', url: '/url', cache: cache});
1435+
$http({method: method || 'GET', url: '/url', cache: cache}).then(noop, noop);
14361436
$httpBackend.flush();
14371437
}
14381438

@@ -1640,7 +1640,7 @@ describe('$http', function() {
16401640

16411641
it('should preserve config object when rejecting from pending cache', function() {
16421642
$httpBackend.expect('GET', '/url').respond(404, 'content');
1643-
$http({method: 'GET', url: '/url', cache: cache, headers: {foo: 'bar'}});
1643+
$http({method: 'GET', url: '/url', cache: cache, headers: {foo: 'bar'}}).then(noop, noop);
16441644

16451645
$http({method: 'GET', url: '/url', cache: cache, headers: {foo: 'baz'}}).catch(callback);
16461646
$httpBackend.flush();

‎test/ng/qSpec.js

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
*/
3333

3434
describe('q', function() {
35-
var q, defer, deferred, promise, log;
35+
var q, defer, deferred, promise, log, exceptionHandlerCalls;
3636

3737
// The following private functions are used to help with logging for testing invocation of the
3838
// promise callbacks.
@@ -181,12 +181,23 @@ describe('q', function() {
181181
};
182182

183183

184+
function exceptionHandler(reason) {
185+
exceptionHandlerCalls.push(reason);
186+
}
187+
188+
189+
function exceptionHandlerStr() {
190+
return exceptionHandlerCalls.join('; ');
191+
}
192+
193+
184194
beforeEach(function() {
185-
q = qFactory(mockNextTick.nextTick, noop),
195+
q = qFactory(mockNextTick.nextTick, exceptionHandler),
186196
defer = q.defer;
187197
deferred = defer();
188198
promise = deferred.promise;
189199
log = [];
200+
exceptionHandlerCalls = [];
190201
mockNextTick.queue = [];
191202
});
192203

@@ -1586,6 +1597,8 @@ describe('q', function() {
15861597
var rejectedPromise = q.reject('rejected');
15871598
expect(rejectedPromise['finally']).not.toBeUndefined();
15881599
expect(rejectedPromise['catch']).not.toBeUndefined();
1600+
rejectedPromise.then(noop, noop);
1601+
mockNextTick.flush();
15891602
});
15901603
});
15911604

@@ -1995,15 +2008,15 @@ describe('q', function() {
19952008
it('should log exceptions thrown in a success callback and reject the derived promise',
19962009
function() {
19972010
var success1 = success(1, 'oops', true);
1998-
promise.then(success1).then(success(2), error(2));
2011+
promise.then(success1).then(success(2), error(2)).then(noop, noop);
19992012
syncResolve(deferred, 'done');
20002013
expect(logStr()).toBe('success1(done)->throw(oops); error2(oops)->reject(oops)');
20012014
expect(mockExceptionLogger.log).toEqual(['oops']);
20022015
});
20032016

20042017

20052018
it('should NOT log exceptions when a success callback returns rejected promise', function() {
2006-
promise.then(success(1, q.reject('rejected'))).then(success(2), error(2));
2019+
promise.then(success(1, q.reject('rejected'))).then(success(2), error(2)).then(noop, noop);
20072020
syncResolve(deferred, 'done');
20082021
expect(logStr()).toBe('success1(done)->{}; error2(rejected)->reject(rejected)');
20092022
expect(mockExceptionLogger.log).toEqual([]);
@@ -2012,15 +2025,15 @@ describe('q', function() {
20122025

20132026
it('should log exceptions thrown in a errback and reject the derived promise', function() {
20142027
var error1 = error(1, 'oops', true);
2015-
promise.then(null, error1).then(success(2), error(2));
2028+
promise.then(null, error1).then(success(2), error(2)).then(noop, noop);
20162029
syncReject(deferred, 'nope');
20172030
expect(logStr()).toBe('error1(nope)->throw(oops); error2(oops)->reject(oops)');
20182031
expect(mockExceptionLogger.log).toEqual(['oops']);
20192032
});
20202033

20212034

20222035
it('should NOT log exceptions when an errback returns a rejected promise', function() {
2023-
promise.then(null, error(1, q.reject('rejected'))).then(success(2), error(2));
2036+
promise.then(null, error(1, q.reject('rejected'))).then(success(2), error(2)).then(noop, noop);
20242037
syncReject(deferred, 'nope');
20252038
expect(logStr()).toBe('error1(nope)->{}; error2(rejected)->reject(rejected)');
20262039
expect(mockExceptionLogger.log).toEqual([]);
@@ -2029,7 +2042,7 @@ describe('q', function() {
20292042

20302043
it('should log exceptions throw in a progressack and stop propagation, but shoud NOT reject ' +
20312044
'the promise', function() {
2032-
promise.then(success(), error(), progress(1, 'failed', true)).then(null, error(1), progress(2));
2045+
promise.then(success(), error(), progress(1, 'failed', true)).then(null, error(1), progress(2)).then(noop, noop);
20332046
syncNotify(deferred, '10%');
20342047
expect(logStr()).toBe('progress1(10%)->throw(failed)');
20352048
expect(mockExceptionLogger.log).toEqual(['failed']);
@@ -2045,15 +2058,15 @@ describe('q', function() {
20452058
it('should log exceptions thrown in a success callback and reject the derived promise',
20462059
function() {
20472060
var success1 = success(1, 'oops', true);
2048-
q.when('hi', success1, error()).then(success(), error(2));
2061+
q.when('hi', success1, error()).then(success(), error(2)).then(noop, noop);
20492062
mockNextTick.flush();
20502063
expect(logStr()).toBe('success1(hi)->throw(oops); error2(oops)->reject(oops)');
20512064
expect(mockExceptionLogger.log).toEqual(['oops']);
20522065
});
20532066

20542067

20552068
it('should NOT log exceptions when a success callback returns rejected promise', function() {
2056-
q.when('hi', success(1, q.reject('rejected'))).then(success(2), error(2));
2069+
q.when('hi', success(1, q.reject('rejected'))).then(success(2), error(2)).then(noop, noop);
20572070
mockNextTick.flush();
20582071
expect(logStr()).toBe('success1(hi)->{}; error2(rejected)->reject(rejected)');
20592072
expect(mockExceptionLogger.log).toEqual([]);
@@ -2062,7 +2075,7 @@ describe('q', function() {
20622075

20632076
it('should log exceptions thrown in a errback and reject the derived promise', function() {
20642077
var error1 = error(1, 'oops', true);
2065-
q.when(q.reject('sorry'), success(), error1).then(success(), error(2));
2078+
q.when(q.reject('sorry'), success(), error1).then(success(), error(2)).then(noop, noop);
20662079
mockNextTick.flush();
20672080
expect(logStr()).toBe('error1(sorry)->throw(oops); error2(oops)->reject(oops)');
20682081
expect(mockExceptionLogger.log).toEqual(['oops']);
@@ -2071,7 +2084,7 @@ describe('q', function() {
20712084

20722085
it('should NOT log exceptions when an errback returns a rejected promise', function() {
20732086
q.when(q.reject('sorry'), success(), error(1, q.reject('rejected'))).
2074-
then(success(2), error(2));
2087+
then(success(2), error(2)).then(noop, noop);
20752088
mockNextTick.flush();
20762089
expect(logStr()).toBe('error1(sorry)->{}; error2(rejected)->reject(rejected)');
20772090
expect(mockExceptionLogger.log).toEqual([]);
@@ -2080,6 +2093,43 @@ describe('q', function() {
20802093
});
20812094

20822095

2096+
describe('when exceptionHandler is called', function() {
2097+
it('should log an unhandled rejected promise', function() {
2098+
var defer = q.defer();
2099+
defer.reject('foo');
2100+
mockNextTick.flush();
2101+
expect(exceptionHandlerStr()).toBe('Possibly unhandled rejection: "foo"');
2102+
});
2103+
2104+
2105+
it('should log a handled rejected promise on a promise without rejection callbacks', function() {
2106+
var defer = q.defer();
2107+
defer.promise.then(noop);
2108+
defer.reject('foo');
2109+
mockNextTick.flush();
2110+
expect(exceptionHandlerStr()).toBe('Possibly unhandled rejection: "foo"');
2111+
});
2112+
2113+
2114+
it('should not log a handled rejected promise', function() {
2115+
var defer = q.defer();
2116+
defer.promise.then(noop, noop);
2117+
defer.reject('foo');
2118+
mockNextTick.flush();
2119+
expect(exceptionHandlerStr()).toBe('');
2120+
});
2121+
2122+
2123+
it('should not log a handled rejected promise that is handled in a future tick', function() {
2124+
var defer = q.defer();
2125+
defer.promise.then(noop, noop);
2126+
defer.resolve(q.reject('foo'));
2127+
mockNextTick.flush();
2128+
expect(exceptionHandlerStr()).toBe('');
2129+
});
2130+
});
2131+
2132+
20832133
describe('when exceptionHandler rethrows exceptions, ', function() {
20842134
var originalLogExceptions, deferred, errorSpy, exceptionExceptionSpy;
20852135

‎test/ng/timeoutSpec.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ describe('$timeout', function() {
165165
expect($exceptionHandler.errors).toEqual([]);
166166

167167
$timeout.flush();
168-
expect($exceptionHandler.errors).toEqual(["Test Error"]);
168+
expect($exceptionHandler.errors).toEqual(["Test Error", 'Possibly unhandled rejection: "Test Error"']);
169169
}));
170170

171171

@@ -238,7 +238,7 @@ describe('$timeout', function() {
238238
$timeout(task2);
239239
promise3 = $timeout(task3, 333);
240240
promise4 = $timeout(333);
241-
promise3.then(task4);
241+
promise3.then(task4, noop);
242242

243243
$timeout.cancel(promise1);
244244
$timeout.cancel(promise3);

0 commit comments

Comments
 (0)