Skip to content
Permalink
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
  • Loading branch information
petebacondarwin committed Apr 19, 2016
1 parent 55b7f7d commit de544807c23076b4d4eacd8f77682a9028980656
Showing with 146 additions and 3 deletions.
  1. +19 −3 src/ng/compile.js
  2. +127 −0 test/ng/compileSpec.js
@@ -1311,11 +1311,19 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
// We must run this hook in an apply since the $$postDigest runs outside apply
$rootScope.$apply(function() {
var errors = [];
for (var i = 0, ii = onChangesQueue.length; i < ii; ++i) {
onChangesQueue[i]();
try {
onChangesQueue[i]();
} catch (e) {
errors.push(e);
}
}
// Reset the queue to trigger a new schedule next time there is a change
onChangesQueue = undefined;
if (errors.length) {
throw errors;
}
});
} finally {
onChangesTtl++;
@@ -2478,10 +2486,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
forEach(elementControllers, function(controller) {
var controllerInstance = controller.instance;
if (isFunction(controllerInstance.$onChanges)) {
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
try {
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
} catch (e) {
$exceptionHandler(e);
}
}
if (isFunction(controllerInstance.$onInit)) {
controllerInstance.$onInit();
try {
controllerInstance.$onInit();
} catch (e) {
$exceptionHandler(e);
}
}
if (isFunction(controllerInstance.$onDestroy)) {
controllerScope.$on('$destroy', function callOnDestroyHook() {
@@ -3651,6 +3651,46 @@ describe('$compile', function() {
expect(Controller2.prototype.$onInit).toHaveBeenCalledOnce();
});
});

it('should continue to trigger other `$onInit` hooks if one throws an error', function() {
function ThrowingController() {
this.$onInit = function() {
throw new Error('bad hook');
};
}
function LoggingController($log) {
this.$onInit = function() {
$log.info('onInit');
};
}

angular.module('my', [])
.component('c1', {
controller: ThrowingController,
bindings: {'prop': '<'}
})
.component('c2', {
controller: LoggingController,
bindings: {'prop': '<'}
})
.config(function($exceptionHandlerProvider) {
// We need to test with the exceptionHandler not rethrowing...
$exceptionHandlerProvider.mode('log');
});

module('my');
inject(function($compile, $rootScope, $exceptionHandler, $log) {

// Setup the directive with bindings that will keep updating the bound value forever
element = $compile('<div><c1 prop="a"></c1><c2 prop="a"></c2>')($rootScope);

// The first component's error should be logged
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook'));

// The second component's hook should still be called
expect($log.info.logs.pop()).toEqual(['onInit']);
});
});
});


@@ -4122,6 +4162,93 @@ describe('$compile', function() {
expect($exceptionHandler.errors[0].toString()).toContain('[$compile:infchng] 10 $onChanges() iterations reached.');
});
});


it('should continue to trigger other `$onChanges` hooks if one throws an error', function() {
function ThrowingController() {
this.$onChanges = function(change) {
throw new Error('bad hook');
};
}
function LoggingController($log) {
this.$onChanges = function(change) {
$log.info('onChange');
};
}

angular.module('my', [])
.component('c1', {
controller: ThrowingController,
bindings: {'prop': '<'}
})
.component('c2', {
controller: LoggingController,
bindings: {'prop': '<'}
})
.config(function($exceptionHandlerProvider) {
// We need to test with the exceptionHandler not rethrowing...
$exceptionHandlerProvider.mode('log');
});

module('my');
inject(function($compile, $rootScope, $exceptionHandler, $log) {

// Setup the directive with bindings that will keep updating the bound value forever
element = $compile('<div><c1 prop="a"></c1><c2 prop="a"></c2>')($rootScope);

// The first component's error should be logged
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook'));

// The second component's changes should still be called
expect($log.info.logs.pop()).toEqual(['onChange']);

$rootScope.$apply('a = 42');

// The first component's error should be logged
var errors = $exceptionHandler.errors.pop();
expect(errors[0]).toEqual(new Error('bad hook'));

// The second component's changes should still be called
expect($log.info.logs.pop()).toEqual(['onChange']);
});
});


it('should collect up all `$onChanges` errors into one throw', function() {
function ThrowingController() {
this.$onChanges = function(change) {
throw new Error('bad hook: ' + this.prop);
};
}

angular.module('my', [])
.component('c1', {
controller: ThrowingController,
bindings: {'prop': '<'}
})
.config(function($exceptionHandlerProvider) {
// We need to test with the exceptionHandler not rethrowing...
$exceptionHandlerProvider.mode('log');
});

module('my');
inject(function($compile, $rootScope, $exceptionHandler, $log) {

// Setup the directive with bindings that will keep updating the bound value forever
element = $compile('<div><c1 prop="a"></c1><c1 prop="a * 2"></c1>')($rootScope);

// Both component's errors should be logged
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook: NaN'));
expect($exceptionHandler.errors.pop()).toEqual(new Error('bad hook: undefined'));

$rootScope.$apply('a = 42');

// Both component's error should be logged
var errors = $exceptionHandler.errors.pop();
expect(errors.pop()).toEqual(new Error('bad hook: 84'));
expect(errors.pop()).toEqual(new Error('bad hook: 42'));
});
});
});
});

0 comments on commit de54480

Please sign in to comment.
You can’t perform that action at this time.