diff --git a/lib/promises-aplus/promises-aplus-test-adapter.js b/lib/promises-aplus/promises-aplus-test-adapter.js index bcd16da7e70b..fdc243d72fa6 100644 --- a/lib/promises-aplus/promises-aplus-test-adapter.js +++ b/lib/promises-aplus/promises-aplus-test-adapter.js @@ -9,21 +9,21 @@ minErr, extend */ -/* eslint-disable no-unused-vars */ -var isFunction = function isFunction(value) {return typeof value === 'function';}; -var isPromiseLike = function isPromiseLike(obj) {return obj && isFunction(obj.then);}; -var isObject = function isObject(value) {return value != null && typeof value === 'object';}; -var isUndefined = function isUndefined(value) {return typeof value === 'undefined';}; +/* eslint-disable no-unused-vars */ +function isFunction(value) { return typeof value === 'function'; } +function isPromiseLike(obj) { return obj && isFunction(obj.then); } +function isObject(value) { return value !== null && typeof value === 'object'; } +function isUndefined(value) { return typeof value === 'undefined'; } -var minErr = function minErr(module, constructor) { +function minErr(module, constructor) { return function() { var ErrorConstructor = constructor || Error; throw new ErrorConstructor(module + arguments[0] + arguments[1]); }; -}; +} -var extend = function extend(dst) { +function extend(dst) { for (var i = 1, ii = arguments.length; i < ii; i++) { var obj = arguments[i]; if (obj) { @@ -35,18 +35,11 @@ var extend = function extend(dst) { } } return dst; -}; +} +/* eslint-enable */ var $q = qFactory(process.nextTick, function noopExceptionHandler() {}); exports.resolved = $q.resolve; exports.rejected = $q.reject; -exports.deferred = function() { - var deferred = $q.defer(); - - return { - promise: deferred.promise, - resolve: deferred.resolve, - reject: deferred.reject - }; -}; +exports.deferred = $q.defer; diff --git a/src/ng/compile.js b/src/ng/compile.js index df611d315c3d..d264d6689684 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3131,6 +3131,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { childBoundTranscludeFn); } linkQueue = null; + }).catch(function(error) { + if (error instanceof Error) { + $exceptionHandler(error); + } }).catch(noop); return function delayedNodeLinkFn(ignoreChildLinkFn, scope, node, rootElement, boundTranscludeFn) { diff --git a/src/ng/q.js b/src/ng/q.js index 7b9de29b0d52..b4568c607427 100644 --- a/src/ng/q.js +++ b/src/ng/q.js @@ -9,8 +9,8 @@ * A service that helps you run functions asynchronously, and use their return values (or exceptions) * when they are done processing. * - * This is an implementation of promises/deferred objects inspired by - * [Kris Kowal's Q](https://github.com/kriskowal/q). + * This is a [Promises/A+](https://promisesaplus.com/)-compliant implementation of promises/deferred + * objects inspired by [Kris Kowal's Q](https://github.com/kriskowal/q). * * $q can be used in two fashions --- one which is more similar to Kris Kowal's Q or jQuery's Deferred * implementations, and the other which resembles ES6 (ES2015) promises to some degree. @@ -366,7 +366,6 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) { } } catch (e) { deferred.reject(e); - exceptionHandler(e); } } } finally { @@ -417,7 +416,6 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) { } else { this.$$resolve(val); } - }, $$resolve: function(val) { @@ -425,7 +423,7 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) { var that = this; var done = false; try { - if ((isObject(val) || isFunction(val))) then = val && val.then; + if (isObject(val) || isFunction(val)) then = val.then; if (isFunction(then)) { this.promise.$$state.status = -1; then.call(val, resolvePromise, rejectPromise, simpleBind(this, this.notify)); @@ -436,7 +434,6 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) { } } catch (e) { rejectPromise(e); - exceptionHandler(e); } function resolvePromise(val) { diff --git a/src/ng/templateRequest.js b/src/ng/templateRequest.js index e7c1007beddc..1046f0223a92 100644 --- a/src/ng/templateRequest.js +++ b/src/ng/templateRequest.js @@ -60,53 +60,59 @@ function $TemplateRequestProvider() { * * @property {number} totalPendingRequests total amount of pending template requests being downloaded. */ - this.$get = ['$templateCache', '$http', '$q', '$sce', function($templateCache, $http, $q, $sce) { + this.$get = ['$exceptionHandler', '$templateCache', '$http', '$q', '$sce', + function($exceptionHandler, $templateCache, $http, $q, $sce) { - function handleRequestFn(tpl, ignoreRequestError) { - handleRequestFn.totalPendingRequests++; + function handleRequestFn(tpl, ignoreRequestError) { + handleRequestFn.totalPendingRequests++; - // We consider the template cache holds only trusted templates, so - // there's no need to go through whitelisting again for keys that already - // are included in there. This also makes Angular accept any script - // directive, no matter its name. However, we still need to unwrap trusted - // types. - if (!isString(tpl) || isUndefined($templateCache.get(tpl))) { - tpl = $sce.getTrustedResourceUrl(tpl); - } + // We consider the template cache holds only trusted templates, so + // there's no need to go through whitelisting again for keys that already + // are included in there. This also makes Angular accept any script + // directive, no matter its name. However, we still need to unwrap trusted + // types. + if (!isString(tpl) || isUndefined($templateCache.get(tpl))) { + tpl = $sce.getTrustedResourceUrl(tpl); + } - var transformResponse = $http.defaults && $http.defaults.transformResponse; + var transformResponse = $http.defaults && $http.defaults.transformResponse; - if (isArray(transformResponse)) { - transformResponse = transformResponse.filter(function(transformer) { - return transformer !== defaultHttpResponseTransform; - }); - } else if (transformResponse === defaultHttpResponseTransform) { - transformResponse = null; - } + if (isArray(transformResponse)) { + transformResponse = transformResponse.filter(function(transformer) { + return transformer !== defaultHttpResponseTransform; + }); + } else if (transformResponse === defaultHttpResponseTransform) { + transformResponse = null; + } - return $http.get(tpl, extend({ - cache: $templateCache, - transformResponse: transformResponse - }, httpOptions)) - .finally(function() { - handleRequestFn.totalPendingRequests--; - }) - .then(function(response) { - $templateCache.put(tpl, response.data); - return response.data; - }, handleError); + return $http.get(tpl, extend({ + cache: $templateCache, + transformResponse: transformResponse + }, httpOptions)) + .finally(function() { + handleRequestFn.totalPendingRequests--; + }) + .then(function(response) { + $templateCache.put(tpl, response.data); + return response.data; + }, handleError); - function handleError(resp) { - if (!ignoreRequestError) { - throw $templateRequestMinErr('tpload', 'Failed to load template: {0} (HTTP status: {1} {2})', - tpl, resp.status, resp.statusText); + function handleError(resp) { + if (!ignoreRequestError) { + resp = $templateRequestMinErr('tpload', + 'Failed to load template: {0} (HTTP status: {1} {2})', + tpl, resp.status, resp.statusText); + + $exceptionHandler(resp); + } + + return $q.reject(resp); } - return $q.reject(resp); } - } - handleRequestFn.totalPendingRequests = 0; + handleRequestFn.totalPendingRequests = 0; - return handleRequestFn; - }]; + return handleRequestFn; + } + ]; } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 492741a4496a..ef26f747f1e5 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -1854,17 +1854,18 @@ describe('$compile', function() { )); - it('should throw an error and clear element content if the template fails to load', inject( - function($compile, $httpBackend, $rootScope) { - $httpBackend.expect('GET', 'hello.html').respond(404, 'Not Found!'); - element = $compile('
content
')($rootScope); + it('should throw an error and clear element content if the template fails to load', + inject(function($compile, $exceptionHandler, $httpBackend, $rootScope) { + $httpBackend.expect('GET', 'hello.html').respond(404, 'Not Found!'); + element = $compile('
content
')($rootScope); - expect(function() { - $httpBackend.flush(); - }).toThrowMinErr('$compile', 'tpload', 'Failed to load template: hello.html'); - expect(sortedHtml(element)).toBe('
'); - } - )); + $httpBackend.flush(); + + expect(sortedHtml(element)).toBe('
'); + expect($exceptionHandler.errors[0].message).toMatch( + /^\[\$compile:tpload] Failed to load template: hello\.html/); + }) + ); it('should prevent multiple templates per element', function() { @@ -1878,13 +1879,15 @@ describe('$compile', function() { templateUrl: 'template.html' })); }); - inject(function($compile, $httpBackend) { + inject(function($compile, $exceptionHandler, $httpBackend) { $httpBackend.whenGET('template.html').respond('

template.html

'); - expect(function() { - $compile('
'); - $httpBackend.flush(); - }).toThrowMinErr('$compile', 'multidir', 'Multiple directives [async, sync] asking for template on: ' + - '
'); + + $compile('
'); + $httpBackend.flush(); + + expect($exceptionHandler.errors[0].message).toMatch(new RegExp( + '^\\[\\$compile:multidir] Multiple directives \\[async, sync] asking for ' + + 'template on:
')); }); }); @@ -2667,14 +2670,15 @@ describe('$compile', function() { ); it('should not allow more than one isolate/new scope creation per element regardless of `templateUrl`', - inject(function($httpBackend) { + inject(function($exceptionHandler, $httpBackend) { $httpBackend.expect('GET', 'tiscope.html').respond('
Hello, world !
'); - expect(function() { - compile('
'); - $httpBackend.flush(); - }).toThrowMinErr('$compile', 'multidir', 'Multiple directives [scopeB, tiscopeA] ' + - 'asking for new/isolated scope on:
'); + compile('
'); + $httpBackend.flush(); + + expect($exceptionHandler.errors[0].message).toMatch(new RegExp( + '^\\[\\$compile:multidir] Multiple directives \\[scopeB, tiscopeA] ' + + 'asking for new/isolated scope on:
')); }) ); @@ -8875,28 +8879,29 @@ describe('$compile', function() { '
' + '
', transclude: true - })); $compileProvider.directive('noTransBar', valueFn({ templateUrl: 'noTransBar.html', transclude: false - })); }); - inject(function($compile, $rootScope, $templateCache) { + inject(function($compile, $exceptionHandler, $rootScope, $templateCache) { $templateCache.put('noTransBar.html', '
' + // This ng-transclude is invalid. It should throw an error. '
' + '
'); - expect(function() { - element = $compile('
content
')($rootScope); - $rootScope.$apply(); - }).toThrowMinErr('ngTransclude', 'orphan', - 'Illegal use of ngTransclude directive in the template! No parent directive that requires a transclusion found. Element:
'); + element = $compile('
content
')($rootScope); + $rootScope.$digest(); + + expect($exceptionHandler.errors[0][1]).toBe('
'); + expect($exceptionHandler.errors[0][0].message).toMatch(new RegExp( + '^\\[ngTransclude:orphan] Illegal use of ngTransclude directive in the ' + + 'template! No parent directive that requires a transclusion found. Element: ' + + '
')); }); }); @@ -9706,12 +9711,15 @@ describe('$compile', function() { transclude: 'element' })); }); - inject(function($compile, $httpBackend) { + inject(function($compile, $exceptionHandler, $httpBackend) { $httpBackend.expectGET('template.html').respond('

template.html

'); + $compile('
'); - expect(function() { - $httpBackend.flush(); - }).toThrowMinErr('$compile', 'multidir', /Multiple directives \[first, second\] asking for transclusion on:

throw(oops); error2(oops)->reject(oops)'); - expect(mockExceptionLogger.log).toEqual(['oops']); + expect(mockExceptionLogger.log).toEqual([]); }); @@ -2095,12 +2095,13 @@ describe('q', function() { }); - it('should log exceptions thrown in a errback and reject the derived promise', function() { + it('should NOT log exceptions thrown in an errback but reject the derived promise', + function() { var error1 = error(1, 'oops', true); 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']); + expect(mockExceptionLogger.log).toEqual([]); }); @@ -2127,13 +2128,13 @@ describe('q', function() { describe('in when', function() { - it('should log exceptions thrown in a success callback and reject the derived promise', + it('should NOT log exceptions thrown in a success callback but reject the derived promise', function() { var success1 = success(1, 'oops', true); 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']); + expect(mockExceptionLogger.log).toEqual([]); }); @@ -2145,12 +2146,12 @@ describe('q', function() { }); - it('should log exceptions thrown in a errback and reject the derived promise', function() { + it('should NOT log exceptions thrown in a errback but reject the derived promise', function() { var error1 = error(1, 'oops', true); 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']); + expect(mockExceptionLogger.log).toEqual([]); }); @@ -2207,50 +2208,4 @@ describe('q', function() { expect(exceptionHandlerStr()).toBe(''); }); }); - - - describe('when exceptionHandler rethrows exceptions, ', function() { - var originalLogExceptions, deferred, errorSpy, exceptionExceptionSpy; - - beforeEach(function() { - // Turn off exception logging for these particular tests - originalLogExceptions = mockNextTick.logExceptions; - mockNextTick.logExceptions = false; - - // Set up spies - exceptionExceptionSpy = jasmine.createSpy('rethrowExceptionHandler') - .and.callFake(function rethrowExceptionHandler(e) { - throw e; - }); - errorSpy = jasmine.createSpy('errorSpy'); - - - q = qFactory(mockNextTick.nextTick, exceptionExceptionSpy); - deferred = q.defer(); - }); - - - afterEach(function() { - // Restore the original exception logging mode - mockNextTick.logExceptions = originalLogExceptions; - }); - - - it('should still reject the promise, when exception is thrown in success handler, even if exceptionHandler rethrows', function() { - deferred.promise.then(function() { throw new Error('reject'); }).then(null, errorSpy); - deferred.resolve('resolve'); - mockNextTick.flush(); - expect(exceptionExceptionSpy).toHaveBeenCalled(); - expect(errorSpy).toHaveBeenCalled(); - }); - - - it('should still reject the promise, when exception is thrown in error handler, even if exceptionHandler rethrows', function() { - deferred.promise.then(null, function() { throw new Error('reject again'); }).then(null, errorSpy); - deferred.reject('reject'); - mockNextTick.flush(); - expect(exceptionExceptionSpy).toHaveBeenCalled(); - expect(errorSpy).toHaveBeenCalled(); - }); - }); }); diff --git a/test/ng/templateRequestSpec.js b/test/ng/templateRequestSpec.js index b064b51875f7..5c741e5e52a6 100644 --- a/test/ng/templateRequestSpec.js +++ b/test/ng/templateRequestSpec.js @@ -114,49 +114,59 @@ describe('$templateRequest', function() { expect($templateCache.get('tpl.html')).toBe('matias'); })); - it('should throw an error when the template is not found', - inject(function($rootScope, $templateRequest, $httpBackend) { - - $httpBackend.expectGET('tpl.html').respond(404, '', {}, 'Not found'); - - $templateRequest('tpl.html'); - - $rootScope.$digest(); - - expect(function() { - $rootScope.$digest(); - $httpBackend.flush(); - }).toThrowMinErr('$compile', 'tpload', 'Failed to load template: tpl.html (HTTP status: 404 Not found)'); - })); - - it('should not throw when the template is not found and ignoreRequestError is true', - inject(function($rootScope, $templateRequest, $httpBackend) { + it('should call `$exceptionHandler` on request error', function() { + module(function($exceptionHandlerProvider) { + $exceptionHandlerProvider.mode('log'); + }); - $httpBackend.expectGET('tpl.html').respond(404); + inject(function($exceptionHandler, $httpBackend, $templateRequest) { + $httpBackend.expectGET('tpl.html').respond(404, '', {}, 'Not Found'); var err; - $templateRequest('tpl.html', true).catch(function(reason) { err = reason; }); - - $rootScope.$digest(); + $templateRequest('tpl.html').catch(function(reason) { err = reason; }); $httpBackend.flush(); - expect(err.status).toBe(404); - })); + expect(err.message).toMatch(new RegExp( + '^\\[\\$compile:tpload] Failed to load template: tpl\\.html ' + + '\\(HTTP status: 404 Not Found\\)')); + expect($exceptionHandler.errors[0].message).toMatch(new RegExp( + '^\\[\\$compile:tpload] Failed to load template: tpl\\.html ' + + '\\(HTTP status: 404 Not Found\\)')); + }); + }); - it('should not throw an error when the template is empty', - inject(function($rootScope, $templateRequest, $httpBackend) { + it('should not call `$exceptionHandler` on request error when `ignoreRequestError` is true', + function() { + module(function($exceptionHandlerProvider) { + $exceptionHandlerProvider.mode('log'); + }); - $httpBackend.expectGET('tpl.html').respond(''); + inject(function($exceptionHandler, $httpBackend, $templateRequest) { + $httpBackend.expectGET('tpl.html').respond(404); - $templateRequest('tpl.html'); + var err; + $templateRequest('tpl.html', true).catch(function(reason) { err = reason; }); + $httpBackend.flush(); - $rootScope.$digest(); + expect(err.status).toBe(404); + expect($exceptionHandler.errors).toEqual([]); + }); + } + ); - expect(function() { + it('should not call `$exceptionHandler` when the template is empty', + inject(function($exceptionHandler, $httpBackend, $rootScope, $templateRequest) { + $httpBackend.expectGET('tpl.html').respond(''); + + var onError = jasmine.createSpy('onError'); + $templateRequest('tpl.html').catch(onError); $rootScope.$digest(); $httpBackend.flush(); - }).not.toThrow(); - })); + + expect(onError).not.toHaveBeenCalled(); + expect($exceptionHandler.errors).toEqual([]); + }) + ); it('should accept empty templates and refuse null or undefined templates in cache', inject(function($rootScope, $templateRequest, $templateCache, $sce) { diff --git a/test/ngRoute/routeSpec.js b/test/ngRoute/routeSpec.js index dd325e10e580..9884a199f1d2 100644 --- a/test/ngRoute/routeSpec.js +++ b/test/ngRoute/routeSpec.js @@ -782,11 +782,20 @@ describe('$route', function() { }); inject(function($route, $location, $rootScope) { + var onError = jasmine.createSpy('onError'); + var onSuccess = jasmine.createSpy('onSuccess'); + + $rootScope.$on('$routeChangeError', onError); + $rootScope.$on('$routeChangeSuccess', onSuccess); + $location.path('/foo'); - expect(function() { - $rootScope.$digest(); - }).toThrowMinErr('$sce', 'insecurl', 'Blocked loading resource from url not allowed by ' + - '$sceDelegate policy. URL: http://example.com/foo.html'); + $rootScope.$digest(); + + expect(onSuccess).not.toHaveBeenCalled(); + expect(onError).toHaveBeenCalled(); + expect(onError.calls.mostRecent().args[3].message).toMatch(new RegExp( + '^\\[\\$sce:insecurl] Blocked loading resource from url not allowed by ' + + '\\$sceDelegate policy\\. URL: http:\\/\\/example\\.com\\/foo\\.html')); }); }); @@ -903,8 +912,7 @@ describe('$route', function() { it('should catch local factory errors', function() { var myError = new Error('MyError'); - module(function($routeProvider, $exceptionHandlerProvider) { - $exceptionHandlerProvider.mode('log'); + module(function($routeProvider) { $routeProvider.when('/locals', { resolve: { a: function($q) { @@ -914,10 +922,14 @@ describe('$route', function() { }); }); - inject(function($location, $route, $rootScope, $exceptionHandler) { + inject(function($location, $route, $rootScope) { + spyOn($rootScope, '$broadcast').and.callThrough(); + $location.path('/locals'); $rootScope.$digest(); - expect($exceptionHandler.errors).toEqual([myError]); + + expect($rootScope.$broadcast).toHaveBeenCalledWith( + '$routeChangeError', jasmine.any(Object), undefined, myError); }); }); }); @@ -1182,8 +1194,7 @@ describe('$route', function() { it('should broadcast `$routeChangeError` when redirectTo throws', function() { var error = new Error('Test'); - module(function($exceptionHandlerProvider, $routeProvider) { - $exceptionHandlerProvider.mode('log'); + module(function($routeProvider) { $routeProvider.when('/foo', {redirectTo: function() { throw error; }}); }); @@ -1196,7 +1207,6 @@ describe('$route', function() { var lastCallArgs = $rootScope.$broadcast.calls.mostRecent().args; expect(lastCallArgs[0]).toBe('$routeChangeError'); expect(lastCallArgs[3]).toBe(error); - expect($exceptionHandler.errors[0]).toBe(error); }); });