invisi/angular.js
1
0
Fork 0
Code Issues Pull Requests Actions Packages Releases Wiki Activity

feat($q): report promises with non rejection callback

Browse Source 
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

Unhandled rejected promises will be logged to $exceptionHandler.

Tests that depend on specific order or number of messages in $exceptionHandler
will need to handle rejected promises report.

Closes: #13653
Closes: #7992
This commit is contained in:
Lucas Mirelmann
2016-01-01 23:40:00 +01:00
parent 0ece2d5e0b
commit c9dffde1cb
13 changed files with 192 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/app/e2e/api-docs/service-pages.scenario.js
+2 -2
View File
@@ -10,7 +10,7 @@ describe("service pages", function() {
browser.get('build/docs/index.html#!/api/ng/service/$q');
providerLink = element.all(by.css('ol.api-profile-header-structure li a')).first();
expect(providerLink.getText()).not.toEqual('- $qProvider');
expect(providerLink.getText()).not.toEqual('- $compileProvider');
expect(providerLink.getAttribute('href')).not.toMatch(/api\/ng\/provider\/\$compileProvider/);
});
@@ -19,4 +19,4 @@ describe("service pages", function() {
expect(element.all(by.css('.input-arguments p em')).first().getText()).toContain('(default: 0)');
});
});
});
src/ng/compile.js
+1 -1
View File
@@ -2818,7 +2818,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
childBoundTranscludeFn);
}
linkQueue = null;
});
}).catch(noop);
return function delayedNodeLinkFn(ignoreChildLinkFn, scope, node, rootElement, boundTranscludeFn) {
var childBoundTranscludeFn = boundTranscludeFn;
src/ng/interval.js
+2
View File
@@ -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.catch(noop);
intervals[promise.$$intervalId].reject('canceled');
$window.clearInterval(promise.$$intervalId);
delete intervals[promise.$$intervalId];
src/ng/q.js
+93 -18
View File
@@ -216,21 +216,58 @@
*
* @returns {Promise} The newly created promise.
*/
/**
* @ngdoc provider
* @name $qProvider
*
* @description
*/
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 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;
}
};
}
/**
@@ -239,10 +276,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
@@ -307,28 +348,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() {
src/ng/timeout.js
+2
View File
@@ -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.catch(noop);
deferreds[promise.$$timeoutId].reject('canceled');
delete deferreds[promise.$$timeoutId];
return $browser.defer.cancel(promise.$$timeoutId);
src/ngMock/angular-mocks.js
Vendored
+2 -1
View File
@@ -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);
});
@@ -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;
src/ngResource/resource.js
+2 -1
View File
@@ -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(
test/ng/animateRunnerSpec.js
+1 -1
View File
@@ -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);
test/ng/httpSpec.js
+3 -3
View File
@@ -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();
});
@@ -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();
}
@@ -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();
test/ng/qSpec.js
+69 -11
View File
@@ -32,7 +32,7 @@
*/
describe('q', function() {
var q, defer, deferred, promise, log;
var q, q_no_error, defer, deferred, promise, log, exceptionHandlerCalls;
// The following private functions are used to help with logging for testing invocation of the
// promise callbacks.
@@ -181,12 +181,24 @@ describe('q', function() {
};
function exceptionHandler(reason) {
exceptionHandlerCalls.push(reason);
}
function exceptionHandlerStr() {
return exceptionHandlerCalls.join('; ');
}
beforeEach(function() {
q = qFactory(mockNextTick.nextTick, noop),
q = qFactory(mockNextTick.nextTick, exceptionHandler, true),
q_no_error = qFactory(mockNextTick.nextTick, exceptionHandler, false),
defer = q.defer;
deferred = defer();
promise = deferred.promise;
log = [];
exceptionHandlerCalls = [];
mockNextTick.queue = [];
});
@@ -1586,6 +1598,8 @@ describe('q', function() {
var rejectedPromise = q.reject('rejected');
expect(rejectedPromise['finally']).not.toBeUndefined();
expect(rejectedPromise['catch']).not.toBeUndefined();
rejectedPromise.catch(noop);
mockNextTick.flush();
});
});
@@ -1995,7 +2009,7 @@ describe('q', function() {
it('should log exceptions thrown in a success callback and reject the derived promise',
function() {
var success1 = success(1, 'oops', true);
promise.then(success1).then(success(2), error(2));
promise.then(success1).then(success(2), error(2)).catch(noop);
syncResolve(deferred, 'done');
expect(logStr()).toBe('success1(done)->throw(oops); error2(oops)->reject(oops)');
expect(mockExceptionLogger.log).toEqual(['oops']);
@@ -2003,7 +2017,7 @@ describe('q', function() {
it('should NOT log exceptions when a success callback returns rejected promise', function() {
promise.then(success(1, q.reject('rejected'))).then(success(2), error(2));
promise.then(success(1, q.reject('rejected'))).then(success(2), error(2)).catch(noop);
syncResolve(deferred, 'done');
expect(logStr()).toBe('success1(done)->{}; error2(rejected)->reject(rejected)');
expect(mockExceptionLogger.log).toEqual([]);
@@ -2012,7 +2026,7 @@ describe('q', function() {
it('should log exceptions thrown in a errback and reject the derived promise', function() {
var error1 = error(1, 'oops', true);
promise.then(null, error1).then(success(2), error(2));
promise.then(null, error1).then(success(2), error(2)).catch(noop);
syncReject(deferred, 'nope');
expect(logStr()).toBe('error1(nope)->throw(oops); error2(oops)->reject(oops)');
expect(mockExceptionLogger.log).toEqual(['oops']);
@@ -2020,7 +2034,7 @@ describe('q', function() {
it('should NOT log exceptions when an errback returns a rejected promise', function() {
promise.then(null, error(1, q.reject('rejected'))).then(success(2), error(2));
promise.then(null, error(1, q.reject('rejected'))).then(success(2), error(2)).catch(noop);
syncReject(deferred, 'nope');
expect(logStr()).toBe('error1(nope)->{}; error2(rejected)->reject(rejected)');
expect(mockExceptionLogger.log).toEqual([]);
@@ -2029,7 +2043,7 @@ describe('q', function() {
it('should log exceptions throw in a progressack and stop propagation, but shoud NOT reject ' +
'the promise', function() {
promise.then(success(), error(), progress(1, 'failed', true)).then(null, error(1), progress(2));
promise.then(success(), error(), progress(1, 'failed', true)).then(null, error(1), progress(2)).catch(noop);
syncNotify(deferred, '10%');
expect(logStr()).toBe('progress1(10%)->throw(failed)');
expect(mockExceptionLogger.log).toEqual(['failed']);
@@ -2045,7 +2059,7 @@ describe('q', function() {
it('should log exceptions thrown in a success callback and reject the derived promise',
function() {
var success1 = success(1, 'oops', true);
q.when('hi', success1, error()).then(success(), error(2));
q.when('hi', success1, error()).then(success(), error(2)).catch(noop);
mockNextTick.flush();
expect(logStr()).toBe('success1(hi)->throw(oops); error2(oops)->reject(oops)');
expect(mockExceptionLogger.log).toEqual(['oops']);
@@ -2053,7 +2067,7 @@ describe('q', function() {
it('should NOT log exceptions when a success callback returns rejected promise', function() {
q.when('hi', success(1, q.reject('rejected'))).then(success(2), error(2));
q.when('hi', success(1, q.reject('rejected'))).then(success(2), error(2)).catch(noop);
mockNextTick.flush();
expect(logStr()).toBe('success1(hi)->{}; error2(rejected)->reject(rejected)');
expect(mockExceptionLogger.log).toEqual([]);
@@ -2062,7 +2076,7 @@ describe('q', function() {
it('should log exceptions thrown in a errback and reject the derived promise', function() {
var error1 = error(1, 'oops', true);
q.when(q.reject('sorry'), success(), error1).then(success(), error(2));
q.when(q.reject('sorry'), success(), error1).then(success(), error(2)).catch(noop);
mockNextTick.flush();
expect(logStr()).toBe('error1(sorry)->throw(oops); error2(oops)->reject(oops)');
expect(mockExceptionLogger.log).toEqual(['oops']);
@@ -2071,7 +2085,7 @@ describe('q', function() {
it('should NOT log exceptions when an errback returns a rejected promise', function() {
q.when(q.reject('sorry'), success(), error(1, q.reject('rejected'))).
then(success(2), error(2));
then(success(2), error(2)).catch(noop);
mockNextTick.flush();
expect(logStr()).toBe('error1(sorry)->{}; error2(rejected)->reject(rejected)');
expect(mockExceptionLogger.log).toEqual([]);
@@ -2080,6 +2094,50 @@ describe('q', function() {
});
describe('when exceptionHandler is called', function() {
it('should log an unhandled rejected promise', function() {
var defer = q.defer();
defer.reject('foo');
mockNextTick.flush();
expect(exceptionHandlerStr()).toBe('Possibly unhandled rejection: foo');
});
it('should not log an unhandled rejected promise if disabled', function() {
var defer = q_no_error.defer();
defer.reject('foo');
expect(exceptionHandlerStr()).toBe('');
});
it('should log a handled rejected promise on a promise without rejection callbacks', function() {
var defer = q.defer();
defer.promise.then(noop);
defer.reject('foo');
mockNextTick.flush();
expect(exceptionHandlerStr()).toBe('Possibly unhandled rejection: foo');
});
it('should not log a handled rejected promise', function() {
var defer = q.defer();
defer.promise.catch(noop);
defer.reject('foo');
mockNextTick.flush();
expect(exceptionHandlerStr()).toBe('');
});
it('should not log a handled rejected promise that is handled in a future tick', function() {
var defer = q.defer();
defer.promise.catch(noop);
defer.resolve(q.reject('foo'));
mockNextTick.flush();
expect(exceptionHandlerStr()).toBe('');
});
});
describe('when exceptionHandler rethrows exceptions, ', function() {
var originalLogExceptions, deferred, errorSpy, exceptionExceptionSpy;
test/ng/timeoutSpec.js
+2 -2
View File
@@ -165,7 +165,7 @@ describe('$timeout', function() {
expect($exceptionHandler.errors).toEqual([]);
$timeout.flush();
expect($exceptionHandler.errors).toEqual(["Test Error"]);
expect($exceptionHandler.errors).toEqual(["Test Error", 'Possibly unhandled rejection: Test Error']);
}));
@@ -238,7 +238,7 @@ describe('$timeout', function() {
$timeout(task2);
promise3 = $timeout(task3, 333);
promise4 = $timeout(333);
promise3.then(task4);
promise3.then(task4, noop);
$timeout.cancel(promise1);
$timeout.cancel(promise3);
test/ngAnimate/animateCssSpec.js
+1
View File
@@ -1329,6 +1329,7 @@ describe("ngAnimate $animateCss", function() {
animator.end();
expect(element.data(ANIMATE_TIMER_KEY)).toBeUndefined();
$timeout.flush();
expect(function() {$timeout.verifyNoPendingTasks();}).not.toThrow();
}));
test/ngResource/resourceSpec.js
+12 -5
View File
@@ -1052,7 +1052,8 @@ describe("basic usage", function() {
it('should call the error callback if provided on non 2xx response', function() {
$httpBackend.expect('GET', '/CreditCard/123').respond(ERROR_CODE, ERROR_RESPONSE);
CreditCard.get({id:123}, callback, errorCB);
var ccs = CreditCard.get({id:123}, callback, errorCB);
ccs.$promise.then(noop, noop);
$httpBackend.flush();
expect(errorCB).toHaveBeenCalledOnce();
expect(callback).not.toHaveBeenCalled();
@@ -1062,7 +1063,8 @@ describe("basic usage", function() {
it('should call the error callback if provided on non 2xx response (without data)', function() {
$httpBackend.expect('GET', '/CreditCard').respond(ERROR_CODE, ERROR_RESPONSE);
CreditCard.get(callback, errorCB);
var ccs = CreditCard.get(callback, errorCB);
ccs.$promise.then(noop, noop);
$httpBackend.flush();
expect(errorCB).toHaveBeenCalledOnce();
expect(callback).not.toHaveBeenCalled();
@@ -1541,7 +1543,8 @@ describe('cancelling requests', function() {
}
});
CreditCard.get();
var ccs = CreditCard.get();
ccs.$promise.catch(noop);
$timeout.flush();
expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
@@ -1560,7 +1563,9 @@ describe('cancelling requests', function() {
}
});
CreditCard.get().$cancelRequest();
var ccs = CreditCard.get();
ccs.$promise.catch(noop);
ccs.$cancelRequest();
expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
CreditCard.get();
@@ -1578,7 +1583,9 @@ describe('cancelling requests', function() {
}
});
CreditCard.get().$cancelRequest();
var ccs = CreditCard.get();
ccs.$promise.catch(noop);
ccs.$cancelRequest();
expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
CreditCard.get();