fix(ivy): don't mask errors by calling lifecycle hooks after a crash (#31244)

The Angular runtime frequently calls into user code (for example, when
writing to a property binding). Since user code can throw errors, calls to
it are frequently wrapped in a try-finally block. In Ivy, the following
pattern is common:

```typescript
enterView();
try {
  callUserCode();
} finally {
  leaveView();
}
```

This has a significant problem, however: `leaveView` has a side effect: it
calls any pending lifecycle hooks that might've been scheduled during the
current round of change detection. Generally it's a bad idea to run
lifecycle hooks after the application has crashed. The application is in an
inconsistent state - directives may not be instantiated fully, queries may
not be resolved, bindings may not have been applied, etc. Invariants that
the app code relies upon may not hold. Further crashes or broken behavior
are likely.

Frequently, lifecycle hooks are used to make assertions about these
invariants. When these assertions fail, they will throw and "swallow" the
original error, making debugging of the problem much more difficult.

This commit modifies `leaveView` to understand whether the application is
currently crashing, via a parameter `safeToRunHooks`. This parameter is set
by modifying the above pattern:

```typescript
enterView();
let safeToRunHooks = false;
try {
  callUserCode();
  safeToRunHooks = true;
} finally {
  leaveView(..., safeToRunHooks);
}
```

If `callUserCode` crashes, then `safeToRunHooks` will never be set to `true`
and `leaveView` won't call any further user code. The original error will
then propagate back up the stack and be reported correctly. A test is added
to verify this behavior.

PR Close #31244

integration/_payload-limits.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime": 1440,
-        "main": 14664,
+        "main": 14847,
         "polyfills": 43567
       }
     }

packages/core/src/render3/component.ts

@@ -136,6 +136,9 @@ export function renderComponent<T>(
 
   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<T>(
     rootView[FLAGS] &= ~LViewFlags.CreationMode;
     resetPreOrderHookFlags(rootView);
     refreshDescendantViews(rootView);  // update mode pass
+    safeToRunHooks = true;
   } finally {
-    leaveView(oldView);
+    leaveView(oldView, safeToRunHooks);
     if (rendererFactory.end) rendererFactory.end();
   }
 

packages/core/src/render3/component_ref.ts

@@ -177,6 +177,9 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
 
     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<T> extends viewEngine_ComponentFactory<T> {
 
       addToViewTree(rootLView, componentView);
       refreshDescendantViews(rootLView);
+      safeToRunHooks = true;
     } finally {
-      leaveView(oldLView);
+      leaveView(oldLView, safeToRunHooks);
     }
 
     const componentRef = new ComponentRef(

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);
 }

packages/core/src/render3/instructions/shared.ts

@@ -390,6 +390,8 @@ export function renderEmbeddedTemplate<T>(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<T>(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<T>(
   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<T>(
     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<T>(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<T>(hostView: LView, component: T) {
     if (!creationMode || hostTView.staticViewQueries) {
       executeViewQueryFn(RenderFlags.Update, hostTView, component);
     }
+    safeToRunHooks = true;
   } finally {
-    leaveView(oldView);
+    leaveView(oldView, safeToRunHooks);
   }
 }
 

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);

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: '<div [dir]="3"></div>',
+    })
+    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');
+  });
 });

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);
       }
     });
   });