Skip to content

Commit 5813f4f

Browse files
fix($compile): cope with $onChanges hooks throwing
Previously, if one of these hooks threw an error, then any other `$onChanges` hooks that had been scheduled were not executed, nor was the queue cleaned up properly. Closes angular#14444
1 parent eb9f7d4 commit 5813f4f

File tree

2 files changed

+99
-3
lines changed

2 files changed

+99
-3
lines changed

src/ng/compile.js

+14-2
Original file line numberDiff line numberDiff line change
@@ -1296,11 +1296,19 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
12961296
}
12971297
// We must run this hook in an apply since the $$postDigest runs outside apply
12981298
$rootScope.$apply(function() {
1299+
var errors = [];
12991300
for (var i = 0, ii = onChangesQueue.length; i < ii; ++i) {
1300-
onChangesQueue[i]();
1301+
try {
1302+
onChangesQueue[i]();
1303+
} catch(e) {
1304+
errors.push(e);
1305+
}
13011306
}
13021307
// Reset the queue to trigger a new schedule next time there is a change
13031308
onChangesQueue = undefined;
1309+
if (errors.length) {
1310+
throw errors;
1311+
}
13041312
});
13051313
} finally {
13061314
onChangesTtl++;
@@ -2461,7 +2469,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
24612469
forEach(elementControllers, function(controller) {
24622470
var controllerInstance = controller.instance;
24632471
if (isFunction(controllerInstance.$onChanges)) {
2464-
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
2472+
try {
2473+
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
2474+
} catch(e) {
2475+
$exceptionHandler(e);
2476+
}
24652477
}
24662478
if (isFunction(controllerInstance.$onInit)) {
24672479
controllerInstance.$onInit();

test/ng/compileSpec.js

+85-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
describe('$compile', function() {
3+
fdescribe('$compile', function() {
44
function isUnknownElement(el) {
55
return !!el.toString().match(/Unknown/);
66
}
@@ -4032,6 +4032,90 @@ describe('$compile', function() {
40324032
expect($exceptionHandler.errors[0].toString()).toContain('[$compile:infchng] 10 $onChanges() iterations reached.');
40334033
});
40344034
});
4035+
4036+
4037+
it('should continue to trigger other `$onChanges` hooks if one throws an error', function() {
4038+
function ThrowingController() {
4039+
this.$onChanges = function(change) {
4040+
throw new Error('bad hook');
4041+
};
4042+
}
4043+
function LoggingController($log) {
4044+
this.$onChanges = function(change) {
4045+
$log.info('onChange');
4046+
};
4047+
}
4048+
4049+
angular.module('my', [])
4050+
.component('c1', {
4051+
controller: ThrowingController,
4052+
bindings: {'prop': '<'}
4053+
})
4054+
.component('c2', {
4055+
controller: LoggingController,
4056+
bindings: {'prop': '<'}
4057+
})
4058+
.config(function($exceptionHandlerProvider) {
4059+
// We need to test with the exceptionHandler not rethrowing...
4060+
$exceptionHandlerProvider.mode('log');
4061+
});
4062+
4063+
module('my');
4064+
inject(function($compile, $rootScope, $exceptionHandler, $log) {
4065+
4066+
// Setup the directive with bindings that will keep updating the bound value forever
4067+
element = $compile('<div><c1 prop="a"></c1><c2 prop="a"></c2>')($rootScope);
4068+
4069+
// The first component's error should be logged
4070+
expect($exceptionHandler.errors.pop().toString()).toEqual('Error: bad hook');
4071+
4072+
// The second component's changes should still be called
4073+
expect($log.info.logs.pop()).toEqual(['onChange']);
4074+
4075+
$rootScope.$apply('a = 42');
4076+
4077+
// The first component's error should be logged
4078+
expect($exceptionHandler.errors[0].toString()).toEqual('Error: bad hook');
4079+
4080+
// The second component's changes should still be called
4081+
expect($log.info.logs.pop()).toEqual(['onChange']);
4082+
});
4083+
});
4084+
4085+
4086+
it('should collect up all `$onChanges` errors into one throw', function() {
4087+
function ThrowingController() {
4088+
this.$onChanges = function(change) {
4089+
throw new Error('bad hook: ' + this.prop);
4090+
};
4091+
}
4092+
4093+
angular.module('my', [])
4094+
.component('c1', {
4095+
controller: ThrowingController,
4096+
bindings: {'prop': '<'}
4097+
})
4098+
.config(function($exceptionHandlerProvider) {
4099+
// We need to test with the exceptionHandler not rethrowing...
4100+
$exceptionHandlerProvider.mode('log');
4101+
});
4102+
4103+
module('my');
4104+
inject(function($compile, $rootScope, $exceptionHandler, $log) {
4105+
4106+
// Setup the directive with bindings that will keep updating the bound value forever
4107+
element = $compile('<div><c1 prop="a"></c1><c1 prop="a * 2"></c1>')($rootScope);
4108+
4109+
// Both component's errors should be logged
4110+
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook: NaN'));
4111+
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook: undefined'));
4112+
4113+
$rootScope.$apply('a = 42');
4114+
4115+
// Both component's error should be logged
4116+
expect($exceptionHandler.errors[0]).toEqual([new Error('bad hook: 42'), new Error('bad hook: 84')]);
4117+
});
4118+
});
40354119
});
40364120
});
40374121

0 commit comments

Comments
 (0)