diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 35c68b565d..7a79c70b08 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime": 1440, - "main": 14664, + "main": 14847, "polyfills": 43567 } } diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index ed6cd447e7..41e0007709 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -136,6 +136,9 @@ export function renderComponent( const oldView = enterView(rootView, null); let component: T; + + // Will become true if the `try` block executes with no errors. + let safeToRunHooks = false; try { if (rendererFactory.begin) rendererFactory.begin(); const componentView = createRootComponentView( @@ -149,8 +152,9 @@ export function renderComponent( rootView[FLAGS] &= ~LViewFlags.CreationMode; resetPreOrderHookFlags(rootView); refreshDescendantViews(rootView); // update mode pass + safeToRunHooks = true; } finally { - leaveView(oldView); + leaveView(oldView, safeToRunHooks); if (rendererFactory.end) rendererFactory.end(); } diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 13da2b898d..a3d8ad2131 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -177,6 +177,9 @@ export class ComponentFactory extends viewEngine_ComponentFactory { let component: T; let tElementNode: TElementNode; + + // Will become true if the `try` block executes with no errors. + let safeToRunHooks = false; try { const componentView = createRootComponentView( hostRNode, this.componentDef, rootLView, rendererFactory, renderer); @@ -199,8 +202,9 @@ export class ComponentFactory extends viewEngine_ComponentFactory { addToViewTree(rootLView, componentView); refreshDescendantViews(rootLView); + safeToRunHooks = true; } finally { - leaveView(oldLView); + leaveView(oldLView, safeToRunHooks); } const componentRef = new ComponentRef( diff --git a/packages/core/src/render3/instructions/embedded_view.ts b/packages/core/src/render3/instructions/embedded_view.ts index e805c59143..94d6d2f1b3 100644 --- a/packages/core/src/render3/instructions/embedded_view.ts +++ b/packages/core/src/render3/instructions/embedded_view.ts @@ -140,6 +140,9 @@ export function ɵɵembeddedViewEnd(): void { refreshDescendantViews(lView); // update mode pass const lContainer = lView[PARENT] as LContainer; ngDevMode && assertLContainerOrUndefined(lContainer); - leaveView(lContainer[PARENT] !); + // It's always safe to run hooks here, as `leaveView` is not called during the 'finally' block + // of a try-catch-finally statement, so it can never be reached while unwinding the stack due to + // an error being thrown. + leaveView(lContainer[PARENT] !, /* safeToRunHooks */ true); setPreviousOrParentTNode(viewHost !, false); } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 41690362cc..3d215c4ad2 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -390,6 +390,8 @@ export function renderEmbeddedTemplate(viewToRender: LView, tView: TView, con // This is a root view inside the view tree tickRootContext(getRootContext(viewToRender)); } else { + // Will become true if the `try` block executes with no errors. + let safeToRunHooks = false; try { setPreviousOrParentTNode(null !, true); @@ -404,8 +406,9 @@ export function renderEmbeddedTemplate(viewToRender: LView, tView: TView, con viewToRender[TVIEW].firstTemplatePass = false; refreshDescendantViews(viewToRender); + safeToRunHooks = true; } finally { - leaveView(oldView !); + leaveView(oldView !, safeToRunHooks); setPreviousOrParentTNode(_previousOrParentTNode, _isParent); } } @@ -417,6 +420,9 @@ export function renderComponentOrTemplate( const oldView = enterView(hostView, hostView[T_HOST]); const normalExecutionPath = !getCheckNoChangesMode(); const creationModeIsActive = isCreationMode(hostView); + + // Will become true if the `try` block executes with no errors. + let safeToRunHooks = false; try { if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) { rendererFactory.begin(); @@ -434,11 +440,12 @@ export function renderComponentOrTemplate( resetPreOrderHookFlags(hostView); templateFn && executeTemplate(hostView, templateFn, RenderFlags.Update, context); refreshDescendantViews(hostView); + safeToRunHooks = true; } finally { if (normalExecutionPath && !creationModeIsActive && rendererFactory.end) { rendererFactory.end(); } - leaveView(oldView); + leaveView(oldView, safeToRunHooks); } } @@ -1724,6 +1731,8 @@ export function checkView(hostView: LView, component: T) { const templateFn = hostTView.template !; const creationMode = isCreationMode(hostView); + // Will become true if the `try` block executes with no errors. + let safeToRunHooks = false; try { resetPreOrderHookFlags(hostView); creationMode && executeViewQueryFn(RenderFlags.Create, hostTView, component); @@ -1733,8 +1742,9 @@ export function checkView(hostView: LView, component: T) { if (!creationMode || hostTView.staticViewQueries) { executeViewQueryFn(RenderFlags.Update, hostTView, component); } + safeToRunHooks = true; } finally { - leaveView(oldView); + leaveView(oldView, safeToRunHooks); } } diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index 582d77e7bc..4d89ee54ac 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -465,17 +465,20 @@ export function resetComponentState() { * the direction of traversal (up or down the view tree) a bit clearer. * * @param newView New state to become active + * @param safeToRunHooks Whether the runtime is in a state where running lifecycle hooks is valid. + * This is not always the case (for example, the application may have crashed and `leaveView` is + * being executed while unwinding the call stack). */ -export function leaveView(newView: LView): void { +export function leaveView(newView: LView, safeToRunHooks: boolean): void { const tView = lView[TVIEW]; if (isCreationMode(lView)) { lView[FLAGS] &= ~LViewFlags.CreationMode; } else { try { resetPreOrderHookFlags(lView); - executeHooks( - lView, tView.viewHooks, tView.viewCheckHooks, checkNoChangesMode, - InitPhaseState.AfterViewInitHooksToBeRun, undefined); + safeToRunHooks && executeHooks( + lView, tView.viewHooks, tView.viewCheckHooks, checkNoChangesMode, + InitPhaseState.AfterViewInitHooksToBeRun, undefined); } finally { // Views are clean and in update mode after being checked, so these bits are cleared lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass); diff --git a/packages/core/test/acceptance/integration_spec.ts b/packages/core/test/acceptance/integration_spec.ts index 2a59030167..cd4942877e 100644 --- a/packages/core/test/acceptance/integration_spec.ts +++ b/packages/core/test/acceptance/integration_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import {CommonModule} from '@angular/common'; -import {Component, ContentChild, Directive, ElementRef, EventEmitter, HostBinding, HostListener, Input, OnInit, Output, QueryList, TemplateRef, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; +import {Component, ContentChild, Directive, ElementRef, EventEmitter, HostBinding, HostListener, Input, NgModule, OnInit, Output, QueryList, TemplateRef, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; import {TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; @@ -1737,4 +1737,34 @@ describe('acceptance integration tests', () => { expect(clicks).toBe(1); }); + it('should not mask errors thrown during lifecycle hooks', () => { + @Directive({ + selector: '[dir]', + inputs: ['dir'], + }) + class Dir { + get dir(): any { return null; } + + set dir(value: any) { throw new Error('this error is expected'); } + } + + @Component({ + template: '
', + }) + class Cmp { + ngAfterViewInit(): void { + // This lifecycle hook should never run, since attempting to bind to Dir's input will throw + // an error. If the runtime continues to run lifecycle hooks after that error, then it will + // execute this hook and throw this error, which will mask the real problem. This test + // verifies this don't happen. + throw new Error('this error is unexpected'); + } + } + + TestBed.configureTestingModule({ + declarations: [Cmp, Dir], + }); + const fixture = TestBed.createComponent(Cmp); + expect(() => fixture.detectChanges()).toThrowError('this error is expected'); + }); }); diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index 3ccc9ab7a2..1cf4569217 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -607,6 +607,7 @@ describe('di', () => { null, createTView(-1, null, 1, 0, null, null, null, null), null, LViewFlags.CheckAlways, null, null, {} as any, {} as any); const oldView = enterView(contentView, null); + let safeToRunHooks = false; try { const parentTNode = getOrCreateTNode(contentView[TVIEW], null, 0, TNodeType.Element, null, null); @@ -617,8 +618,9 @@ describe('di', () => { const injector = getOrCreateNodeInjectorForNode(parentTNode, contentView); expect(injector).not.toEqual(-1); + safeToRunHooks = true; } finally { - leaveView(oldView); + leaveView(oldView, safeToRunHooks); } }); });