From e6c206ce7f5c7a32059c4e7a7c36bdd0237cac1d Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 19 Apr 2016 20:18:52 +0100 Subject: [PATCH 1/3] 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 #14444 Closes #14463 --- src/ng/compile.js | 16 +++++++- test/ng/compileSpec.js | 86 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index fee065820bc4..e6b2970b8f82 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1296,11 +1296,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++; @@ -2461,7 +2469,11 @@ 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(); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 2b64f8ae8170..4a3066e6276b 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -1,6 +1,6 @@ 'use strict'; -describe('$compile', function() { +fdescribe('$compile', function() { function isUnknownElement(el) { return !!el.toString().match(/Unknown/); } @@ -4032,6 +4032,90 @@ 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('
')($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 + 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']); + }); + }); + + + 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('
')($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 + expect($exceptionHandler.errors[0]).toEqual([new Error('bad hook: 42'), new Error('bad hook: 84')]); + }); + }); }); }); From c57ed917b8d2f0ccee5bbb68ccb5427f81b8e957 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 19 Apr 2016 09:27:22 +0100 Subject: [PATCH 2/3] fix($compile): cope with `$onInit` hooks throwing Previously, if one of these hooks threw an error, then the compilation was terminated unceremoniously. Closes #14444 Closes #14463 --- src/ng/compile.js | 10 ++++++--- test/ng/compileSpec.js | 46 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index e6b2970b8f82..4e906a04f821 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1300,7 +1300,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { for (var i = 0, ii = onChangesQueue.length; i < ii; ++i) { try { onChangesQueue[i](); - } catch(e) { + } catch (e) { errors.push(e); } } @@ -2471,12 +2471,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (isFunction(controllerInstance.$onChanges)) { try { controllerInstance.$onChanges(controller.bindingInfo.initialChanges); - } catch(e) { + } 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() { diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 4a3066e6276b..f28ef2c489b3 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -1,6 +1,6 @@ 'use strict'; -fdescribe('$compile', function() { +describe('$compile', function() { function isUnknownElement(el) { return !!el.toString().match(/Unknown/); } @@ -3563,6 +3563,46 @@ fdescribe('$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('
')($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']); + }); + }); }); @@ -4113,7 +4153,9 @@ fdescribe('$compile', function() { $rootScope.$apply('a = 42'); // Both component's error should be logged - expect($exceptionHandler.errors[0]).toEqual([new Error('bad hook: 42'), new Error('bad hook: 84')]); + var errors = $exceptionHandler.errors.pop(); + expect(errors.pop()).toEqual(new Error('bad hook: 84')); + expect(errors.pop()).toEqual(new Error('bad hook: 42')); }); }); }); From caad0ad956a1c4ea490e0f3946e3b3dd92700edc Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 19 Apr 2016 20:49:45 +0100 Subject: [PATCH 3/3] 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 #14444 Closes #14463 --- test/ng/compileSpec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index f28ef2c489b3..a9ab748b2faf 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4115,7 +4115,8 @@ describe('$compile', function() { $rootScope.$apply('a = 42'); // The first component's error should be logged - expect($exceptionHandler.errors.pop()).toEqual([new Error('bad hook')]); + 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']);