Skip to content

Commit

Permalink
feat($q): report promises with non rejection callback
Browse files Browse the repository at this point in the history
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: angular#13653
Closes: angular#7992
  • Loading branch information
lgalfaso committed Mar 21, 2016
1 parent c9b4251 commit bfdfc7e
Show file tree
Hide file tree
Showing 12 changed files with 184 additions and 43 deletions.
2 changes: 1 addition & 1 deletion src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -2716,7 +2716,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
childBoundTranscludeFn);
}
linkQueue = null;
});
}).then(noop, noop);

return function delayedNodeLinkFn(ignoreChildLinkFn, scope, node, rootElement, boundTranscludeFn) {
var childBoundTranscludeFn = boundTranscludeFn;
Expand Down
2 changes: 2 additions & 0 deletions src/ng/interval.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ function $IntervalProvider() {
*/
interval.cancel = function(promise) {
if (promise && promise.$$intervalId in intervals) {
// Interval cancels should not report as unhandled promise.
intervals[promise.$$intervalId].promise.then(noop, noop);
intervals[promise.$$intervalId].reject('canceled');
$window.clearInterval(promise.$$intervalId);
delete intervals[promise.$$intervalId];
Expand Down
105 changes: 87 additions & 18 deletions src/ng/q.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,20 +217,51 @@
* @returns {Promise} The newly created promise.
*/
function $QProvider() {

var errorOnUnhandledRejections = true;
this.$get = ['$rootScope', '$exceptionHandler', function($rootScope, $exceptionHandler) {
return qFactory(function(callback) {
$rootScope.$evalAsync(callback);
}, $exceptionHandler);
}, $exceptionHandler, errorOnUnhandledRejections);
}];

/**
* @ngdoc method
* @name $qProvider#errorOnUnhandledRejections
* @kind function
*
* @description
* Retrieves or overrides on whether to generate an error when a rejected promise is not handled.
*
* @param {boolean=} value Whether to generate an error when a rejected promise is not handled.
* @returns {boolean|ng.$qProvider} Current value when called without a new value or self for
* chaining otherwise.
*/
this.errorOnUnhandledRejections = function(value) {
if (isDefined(value)) {
errorOnUnhandledRejections = value;
return this;
} else {
return errorOnUnhandledRejections;
}
};
}

function $$QProvider() {
var errorOnUnhandledRejections = true;
this.$get = ['$browser', '$exceptionHandler', function($browser, $exceptionHandler) {
return qFactory(function(callback) {
$browser.defer(callback);
}, $exceptionHandler);
}, $exceptionHandler, errorOnUnhandledRejections);
}];

this.errorOnUnhandledRejections = function(value) {
if (isDefined(value)) {
errorOnUnhandledRejections = value;
return this;
} else {
return errorOnUnhandledRejections;
}
};
}

/**
Expand All @@ -239,10 +270,14 @@ function $$QProvider() {
* @param {function(function)} nextTick Function for executing functions in the next turn.
* @param {function(...*)} exceptionHandler Function into which unexpected exceptions are passed for
* debugging purposes.
@ param {=boolean} errorOnUnhandledRejections Whether an error should be generated on unhandled
* promises rejections.
* @returns {object} Promise manager.
*/
function qFactory(nextTick, exceptionHandler) {
function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) {
var $qMinErr = minErr('$q', TypeError);
var queueSize = 0;
var checkQueue = [];

/**
* @ngdoc method
Expand Down Expand Up @@ -307,28 +342,62 @@ function qFactory(nextTick, exceptionHandler) {
pending = state.pending;
state.processScheduled = false;
state.pending = undefined;
for (var i = 0, ii = pending.length; i < ii; ++i) {
deferred = pending[i][0];
fn = pending[i][state.status];
try {
if (isFunction(fn)) {
deferred.resolve(fn(state.value));
} else if (state.status === 1) {
deferred.resolve(state.value);
} else {
deferred.reject(state.value);
try {
for (var i = 0, ii = pending.length; i < ii; ++i) {
state.pur = true;
deferred = pending[i][0];
fn = pending[i][state.status];
try {
if (isFunction(fn)) {
deferred.resolve(fn(state.value));
} else if (state.status === 1) {
deferred.resolve(state.value);
} else {
deferred.reject(state.value);
}
} catch (e) {
deferred.reject(e);
exceptionHandler(e);
}
} catch (e) {
deferred.reject(e);
exceptionHandler(e);
}
} finally {
--queueSize;
if (errorOnUnhandledRejections && queueSize === 0) {
nextTick(processChecksFn());
}
}
}

function processChecks() {
while (!queueSize && checkQueue.length) {
var toCheck = checkQueue.shift();
if (!toCheck.pur) {
toCheck.pur = true;
var errorMessage = 'Possibly unhandled rejection: ' + toDebugString(toCheck.value);
exceptionHandler(errorMessage);
}
}
}

function processChecksFn() {
return function() { processChecks(); };
}

function processQueueFn(state) {
return function() { processQueue(state); };
}

function scheduleProcessQueue(state) {
if (errorOnUnhandledRejections && !state.pending && state.status === 2 && !state.pur) {
if (queueSize === 0 && checkQueue.length === 0) {
nextTick(processChecksFn());
}
checkQueue.push(state);
}
if (state.processScheduled || !state.pending) return;
state.processScheduled = true;
nextTick(function() { processQueue(state); });
++queueSize;
nextTick(processQueueFn(state));
}

function Deferred() {
Expand Down
2 changes: 2 additions & 0 deletions src/ng/timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ function $TimeoutProvider() {
*/
timeout.cancel = function(promise) {
if (promise && promise.$$timeoutId in deferreds) {
// Timeout cancels should not report an unhandled promise.
deferreds[promise.$$timeoutId].promise.then(noop, noop);
deferreds[promise.$$timeoutId].reject('canceled');
delete deferreds[promise.$$timeoutId];
return $browser.defer.cancel(promise.$$timeoutId);
Expand Down
3 changes: 2 additions & 1 deletion src/ngMock/angular-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ angular.mock.$IntervalProvider = function() {
promise = deferred.promise;

count = (angular.isDefined(count)) ? count : 0;
promise.then(null, null, (!hasParams) ? fn : function() {
promise.then(null, function() {}, (!hasParams) ? fn : function() {
fn.apply(null, args);
});

Expand Down Expand Up @@ -505,6 +505,7 @@ angular.mock.$IntervalProvider = function() {
});

if (angular.isDefined(fnIndex)) {
repeatFns[fnIndex].deferred.promise.then(undefined, function() {});
repeatFns[fnIndex].deferred.reject('canceled');
repeatFns.splice(fnIndex, 1);
return true;
Expand Down
3 changes: 2 additions & 1 deletion src/ngResource/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -706,13 +706,14 @@ angular.module('ngResource', ['ng']).
return $q.reject(response);
});

promise['finally'](function() {
promise = promise['finally'](function(response) {
value.$resolved = true;
if (!isInstanceCall && cancellable) {
value.$cancelRequest = angular.noop;
$timeout.cancel(numericTimeoutPromise);
timeoutDeferred = numericTimeoutPromise = httpConfig.timeout = null;
}
return response;
});

promise = promise.then(
Expand Down
2 changes: 1 addition & 1 deletion test/ng/animateRunnerSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ describe("$$AnimateRunner", function() {
var animationComplete = false;
runner.finally(function() {
animationComplete = true;
});
}).then(noop, noop);
runner[method]();
$rootScope.$digest();
expect(animationComplete).toBe(true);
Expand Down
6 changes: 3 additions & 3 deletions test/ng/httpSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,7 @@ describe('$http', function() {

it('should $apply after error callback', function() {
$httpBackend.when('GET').respond(404);
$http({method: 'GET', url: '/some'});
$http({method: 'GET', url: '/some'}).catch(noop);
$httpBackend.flush();
expect($rootScope.$apply).toHaveBeenCalledOnce();
});
Expand Down Expand Up @@ -1432,7 +1432,7 @@ describe('$http', function() {

function doFirstCacheRequest(method, respStatus, headers) {
$httpBackend.expect(method || 'GET', '/url').respond(respStatus || 200, 'content', headers);
$http({method: method || 'GET', url: '/url', cache: cache});
$http({method: method || 'GET', url: '/url', cache: cache}).catch(noop);
$httpBackend.flush();
}

Expand Down Expand Up @@ -1640,7 +1640,7 @@ describe('$http', function() {

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

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

0 comments on commit bfdfc7e

Please sign in to comment.