Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 3749c85

Browse files
fix($compile): cope with $onChanges hooks throwing
Previously, if one of these hooks threw an error, then the compilation was terminated unceremoniously. In particular any other `$onChanges` hooks that had been scheduled were not executed, nor was the queue cleaned up properly. Closes #14444 Closes #14463
1 parent 886b59d commit 3749c85

File tree

2 files changed

+146
-3
lines changed

2 files changed

+146
-3
lines changed

src/ng/compile.js

+19-3
Original file line numberDiff line numberDiff line change
@@ -1311,11 +1311,19 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
13111311
}
13121312
// We must run this hook in an apply since the $$postDigest runs outside apply
13131313
$rootScope.$apply(function() {
1314+
var errors = [];
13141315
for (var i = 0, ii = onChangesQueue.length; i < ii; ++i) {
1315-
onChangesQueue[i]();
1316+
try {
1317+
onChangesQueue[i]();
1318+
} catch (e) {
1319+
errors.push(e);
1320+
}
13161321
}
13171322
// Reset the queue to trigger a new schedule next time there is a change
13181323
onChangesQueue = undefined;
1324+
if (errors.length) {
1325+
throw errors;
1326+
}
13191327
});
13201328
} finally {
13211329
onChangesTtl++;
@@ -2491,10 +2499,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
24912499
forEach(elementControllers, function(controller) {
24922500
var controllerInstance = controller.instance;
24932501
if (isFunction(controllerInstance.$onChanges)) {
2494-
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
2502+
try {
2503+
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
2504+
} catch (e) {
2505+
$exceptionHandler(e);
2506+
}
24952507
}
24962508
if (isFunction(controllerInstance.$onInit)) {
2497-
controllerInstance.$onInit();
2509+
try {
2510+
controllerInstance.$onInit();
2511+
} catch (e) {
2512+
$exceptionHandler(e);
2513+
}
24982514
}
24992515
if (isFunction(controllerInstance.$onDestroy)) {
25002516
controllerScope.$on('$destroy', function callOnDestroyHook() {

test/ng/compileSpec.js

+127
Original file line numberDiff line numberDiff line change
@@ -3650,6 +3650,46 @@ describe('$compile', function() {
36503650
expect(Controller2.prototype.$onInit).toHaveBeenCalledOnce();
36513651
});
36523652
});
3653+
3654+
it('should continue to trigger other `$onInit` hooks if one throws an error', function() {
3655+
function ThrowingController() {
3656+
this.$onInit = function() {
3657+
throw new Error('bad hook');
3658+
};
3659+
}
3660+
function LoggingController($log) {
3661+
this.$onInit = function() {
3662+
$log.info('onInit');
3663+
};
3664+
}
3665+
3666+
angular.module('my', [])
3667+
.component('c1', {
3668+
controller: ThrowingController,
3669+
bindings: {'prop': '<'}
3670+
})
3671+
.component('c2', {
3672+
controller: LoggingController,
3673+
bindings: {'prop': '<'}
3674+
})
3675+
.config(function($exceptionHandlerProvider) {
3676+
// We need to test with the exceptionHandler not rethrowing...
3677+
$exceptionHandlerProvider.mode('log');
3678+
});
3679+
3680+
module('my');
3681+
inject(function($compile, $rootScope, $exceptionHandler, $log) {
3682+
3683+
// Setup the directive with bindings that will keep updating the bound value forever
3684+
element = $compile('<div><c1 prop="a"></c1><c2 prop="a"></c2>')($rootScope);
3685+
3686+
// The first component's error should be logged
3687+
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook'));
3688+
3689+
// The second component's hook should still be called
3690+
expect($log.info.logs.pop()).toEqual(['onInit']);
3691+
});
3692+
});
36533693
});
36543694

36553695

@@ -4121,6 +4161,93 @@ describe('$compile', function() {
41214161
expect($exceptionHandler.errors[0].toString()).toContain('[$compile:infchng] 10 $onChanges() iterations reached.');
41224162
});
41234163
});
4164+
4165+
4166+
it('should continue to trigger other `$onChanges` hooks if one throws an error', function() {
4167+
function ThrowingController() {
4168+
this.$onChanges = function(change) {
4169+
throw new Error('bad hook');
4170+
};
4171+
}
4172+
function LoggingController($log) {
4173+
this.$onChanges = function(change) {
4174+
$log.info('onChange');
4175+
};
4176+
}
4177+
4178+
angular.module('my', [])
4179+
.component('c1', {
4180+
controller: ThrowingController,
4181+
bindings: {'prop': '<'}
4182+
})
4183+
.component('c2', {
4184+
controller: LoggingController,
4185+
bindings: {'prop': '<'}
4186+
})
4187+
.config(function($exceptionHandlerProvider) {
4188+
// We need to test with the exceptionHandler not rethrowing...
4189+
$exceptionHandlerProvider.mode('log');
4190+
});
4191+
4192+
module('my');
4193+
inject(function($compile, $rootScope, $exceptionHandler, $log) {
4194+
4195+
// Setup the directive with bindings that will keep updating the bound value forever
4196+
element = $compile('<div><c1 prop="a"></c1><c2 prop="a"></c2>')($rootScope);
4197+
4198+
// The first component's error should be logged
4199+
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook'));
4200+
4201+
// The second component's changes should still be called
4202+
expect($log.info.logs.pop()).toEqual(['onChange']);
4203+
4204+
$rootScope.$apply('a = 42');
4205+
4206+
// The first component's error should be logged
4207+
var errors = $exceptionHandler.errors.pop();
4208+
expect(errors[0]).toEqual(new Error('bad hook'));
4209+
4210+
// The second component's changes should still be called
4211+
expect($log.info.logs.pop()).toEqual(['onChange']);
4212+
});
4213+
});
4214+
4215+
4216+
it('should collect up all `$onChanges` errors into one throw', function() {
4217+
function ThrowingController() {
4218+
this.$onChanges = function(change) {
4219+
throw new Error('bad hook: ' + this.prop);
4220+
};
4221+
}
4222+
4223+
angular.module('my', [])
4224+
.component('c1', {
4225+
controller: ThrowingController,
4226+
bindings: {'prop': '<'}
4227+
})
4228+
.config(function($exceptionHandlerProvider) {
4229+
// We need to test with the exceptionHandler not rethrowing...
4230+
$exceptionHandlerProvider.mode('log');
4231+
});
4232+
4233+
module('my');
4234+
inject(function($compile, $rootScope, $exceptionHandler, $log) {
4235+
4236+
// Setup the directive with bindings that will keep updating the bound value forever
4237+
element = $compile('<div><c1 prop="a"></c1><c1 prop="a * 2"></c1>')($rootScope);
4238+
4239+
// Both component's errors should be logged
4240+
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook: NaN'));
4241+
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook: undefined'));
4242+
4243+
$rootScope.$apply('a = 42');
4244+
4245+
// Both component's error should be logged
4246+
var errors = $exceptionHandler.errors.pop();
4247+
expect(errors.pop()).toEqual(new Error('bad hook: 84'));
4248+
expect(errors.pop()).toEqual(new Error('bad hook: 42'));
4249+
});
4250+
});
41244251
});
41254252
});
41264253

0 commit comments

Comments
 (0)