diff --git a/src/ng/compile.js b/src/ng/compile.js index df611d315c3d..67496c981e67 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3052,8 +3052,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { $compileNode.empty(); - $templateRequest(templateUrl) - .then(function(content) { + $templateRequest(templateUrl). + then(function(content) { var compileNode, tempTemplateAttrs, $template, childBoundTranscludeFn; content = denormalizeTemplate(content); @@ -3131,7 +3131,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { childBoundTranscludeFn); } linkQueue = null; - }).catch(noop); + }). + catch(function(error) { + if (error instanceof Error) { + $exceptionHandler(error); + } + }). + catch(noop); return function delayedNodeLinkFn(ignoreChildLinkFn, scope, node, rootElement, boundTranscludeFn) { var childBoundTranscludeFn = boundTranscludeFn; diff --git a/src/ng/q.js b/src/ng/q.js index 7b9de29b0d52..55829db5ffd4 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,18 +416,15 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) { } else { this.$$resolve(val); } - }, $$resolve: function(val) { - var then; var that = this; var done = false; try { - if ((isObject(val) || isFunction(val))) then = val && val.then; - if (isFunction(then)) { + if ((isObject(val) || isFunction(val)) && isFunction(val.then)) { this.promise.$$state.status = -1; - then.call(val, resolvePromise, rejectPromise, simpleBind(this, this.notify)); + val.then(resolvePromise, rejectPromise, simpleBind(this, this.notify)); } else { this.promise.$$state.value = val; this.promise.$$state.status = 1; @@ -436,7 +432,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..4bf426700ca2 100644 --- a/test/ng/templateRequestSpec.js +++ b/test/ng/templateRequestSpec.js @@ -115,19 +115,17 @@ describe('$templateRequest', function() { })); 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(); + inject(function($exceptionHandler, $httpBackend, $rootScope, $templateRequest) { + $httpBackend.expectGET('tpl.html').respond(404, '', {}, 'Not found'); - expect(function() { - $rootScope.$digest(); + $templateRequest('tpl.html').catch(noop); $httpBackend.flush(); - }).toThrowMinErr('$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 when the template is not found and ignoreRequestError is true', inject(function($rootScope, $templateRequest, $httpBackend) { 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); }); });