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

Commit de54480

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 55b7f7d commit de54480

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++;
@@ -2478,10 +2486,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
24782486
forEach(elementControllers, function(controller) {
24792487
var controllerInstance = controller.instance;
24802488
if (isFunction(controllerInstance.$onChanges)) {
2481-
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
2489+
try {
2490+
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
2491+
} catch (e) {
2492+
$exceptionHandler(e);
2493+
}
24822494
}
24832495
if (isFunction(controllerInstance.$onInit)) {
2484-
controllerInstance.$onInit();
2496+
try {
2497+
controllerInstance.$onInit();
2498+
} catch (e) {
2499+
$exceptionHandler(e);
2500+
}
24852501
}
24862502
if (isFunction(controllerInstance.$onDestroy)) {
24872503
controllerScope.$on('$destroy', function callOnDestroyHook() {

test/ng/compileSpec.js

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

36563696

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

0 commit comments

Comments
 (0)