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

fix($compile): do not swallow thrown errors in test

Browse Source 
In e13eeab, errors/rejections produced during fetching the template or compiling
an asynchronous directive, where overzealously silenced. This doesn't make any
difference in (most) production apps, where `$exceptionHandler` does not rethrow
the errors. In tests though (where `$exceptionHandler` rethrows by default), it
can unexpectedly "swallow" thrown errors.

This commit fixes it by removing the extraneous `.catch(noop)`, thus letting
errors thrown by `$exceptionHandler` to surface.

The changes in 'compileSpec.js' essentially revert the modifications that were
unnecessarily (and incorrectly) done in e13eeab (and also one incorrect
modification from [c22615c][1]).

[1]: https://github.com/angular/angular.js/commit/c22615cbfbaa7d1712e79b6bf2ace6eb41313bac#diff-348c2f3781ed66a24894c2046a52c628L2084

Fixes #15629

Closes #15631
This commit is contained in:
Georgios Kalpakas
2017-01-20 14:31:24 +02:00
parent 0ef193f1a3
commit 03dbd94cb8
2 changed files with 32 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ng/compile.js
+1 -1
View File
@@ -3156,7 +3156,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
if (error instanceof Error) {
$exceptionHandler(error);
}
}).catch(noop);
});
return function delayedNodeLinkFn(ignoreChildLinkFn, scope, node, rootElement, boundTranscludeFn) {
var childBoundTranscludeFn = boundTranscludeFn;
test/ng/compileSpec.js
+31 -33
View File
@@ -1881,15 +1881,14 @@ describe('$compile', function() {
it('should throw an error and clear element content if the template fails to load',
inject(function($compile, $exceptionHandler, $httpBackend, $rootScope) {
inject(function($compile, $httpBackend, $rootScope) {
$httpBackend.expect('GET', 'hello.html').respond(404, 'Not Found!');
element = $compile('<div><b class="hello">content</b></div>')($rootScope);
$httpBackend.flush();
expect(function() {
$httpBackend.flush();
}).toThrowMinErr('$compile', 'tpload', 'Failed to load template: hello.html');
expect(sortedHtml(element)).toBe('<div><b class="hello"></b></div>');
expect($exceptionHandler.errors[0]).toEqualMinErr('$compile', 'tpload',
'Failed to load template: hello.html');
})
);
@@ -1905,13 +1904,13 @@ describe('$compile', function() {
templateUrl: 'template.html'
}));
});
inject(function($compile, $exceptionHandler, $httpBackend) {
inject(function($compile, $httpBackend) {
$httpBackend.whenGET('template.html').respond('<p>template.html</p>');
$compile('<div><div class="sync async"></div></div>');
$httpBackend.flush();
expect($exceptionHandler.errors[0]).toEqualMinErr('$compile', 'multidir',
expect(function() {
$compile('<div><div class="sync async"></div></div>');
$httpBackend.flush();
}).toThrowMinErr('$compile', 'multidir',
'Multiple directives [async, sync] asking for template on: ' +
'<div class="sync async">');
});
@@ -2122,15 +2121,15 @@ describe('$compile', function() {
'multiple root elements': '<div></div><div></div>'
}, function(directiveTemplate) {
inject(function($compile, $templateCache, $rootScope, $exceptionHandler) {
inject(function($compile, $templateCache, $rootScope) {
$templateCache.put('template.html', directiveTemplate);
$compile('<p template></p>')($rootScope);
$rootScope.$digest();
expect($exceptionHandler.errors.pop()).toEqualMinErr('$compile', 'tplrt',
'Template for directive \'template\' must have exactly one root element. ' +
'template.html'
);
expect(function() {
$compile('<p template></p>')($rootScope);
$rootScope.$digest();
}).toThrowMinErr('$compile', 'tplrt',
'Template for directive \'template\' must have exactly one root element. ' +
'template.html');
});
});
@@ -2657,13 +2656,13 @@ describe('$compile', function() {
);
it('should not allow more than one isolate/new scope creation per element regardless of `templateUrl`',
inject(function($exceptionHandler, $httpBackend) {
inject(function($httpBackend) {
$httpBackend.expect('GET', 'tiscope.html').respond('<div>Hello, world !</div>');
compile('<div class="tiscope-a; scope-b"></div>');
$httpBackend.flush();
expect($exceptionHandler.errors[0]).toEqualMinErr('$compile', 'multidir',
expect(function() {
compile('<div class="tiscope-a; scope-b"></div>');
$httpBackend.flush();
}).toThrowMinErr('$compile', 'multidir',
'Multiple directives [scopeB, tiscopeA] asking for new/isolated scope on: ' +
'<div class="tiscope-a; scope-b ng-scope">');
})
@@ -9069,18 +9068,17 @@ describe('$compile', function() {
}));
});
inject(function($compile, $exceptionHandler, $rootScope, $templateCache) {
inject(function($compile, $rootScope, $templateCache) {
$templateCache.put('noTransBar.html',
'<div>' +
// This ng-transclude is invalid. It should throw an error.
'<div class="bar" ng-transclude></div>' +
'</div>');
element = $compile('<div trans-foo>content</div>')($rootScope);
$rootScope.$digest();
expect($exceptionHandler.errors[0][1]).toBe('<div class="bar" ng-transclude="">');
expect($exceptionHandler.errors[0][0]).toEqualMinErr('ngTransclude', 'orphan',
expect(function() {
element = $compile('<div trans-foo>content</div>')($rootScope);
$rootScope.$digest();
}).toThrowMinErr('ngTransclude', 'orphan',
'Illegal use of ngTransclude directive in the template! ' +
'No parent directive that requires a transclusion found. ' +
'Element: <div class="bar" ng-transclude="">');
@@ -9893,13 +9891,13 @@ describe('$compile', function() {
transclude: 'element'
}));
});
inject(function($compile, $exceptionHandler, $httpBackend) {
inject(function($compile, $httpBackend) {
$httpBackend.expectGET('template.html').respond('<p second>template.html</p>');
$compile('<div template first></div>');
$httpBackend.flush();
expect($exceptionHandler.errors[0]).toEqualMinErr('$compile', 'multidir',
expect(function() {
$compile('<div template first></div>');
$httpBackend.flush();
}).toThrowMinErr('$compile', 'multidir',
'Multiple directives [first, second] asking for transclusion on: <p ');
});
});