Improve error boundary handling for unmounted subtrees

A passive effect's cleanup function may throw after an unmount. In that event, React should still call the nearest error boundary (even if that error boundary may have also been unmounted) so that it can log the error.

Prior to this commit, there were several potential problems:
* If the nearest error boundary was the root of the unmounted subtree, React sometimes threw an uncaught runtime error trying to access a property on a null  field.
* As a result, React never called  on this boundary.
* Even if React did find the boundary, it didn't schedule any follow up work because the pointer to the root had been disconnected.

This commit makes a few improvements:
* React now defers nulling out the  field until after passive effects have been flushed, to avoid the uncaught runtime error.
* React will call the nearested (unmounted) error boundary's  method.
* If no unmounted boundary was found, React will call the nearest (mounted) error boundary's  method in the new reconciler fork only.

Author: Brian Vaughn

packages/react-reconciler/src/ReactFiberCommitWork.new.js

@@ -166,13 +166,13 @@ function safelyCallComponentWillUnmount(current, instance) {
     );
     if (hasCaughtError()) {
       const unmountError = clearCaughtError();
-      captureCommitPhaseError(current, unmountError);
+      captureCommitPhaseError(current, unmountError, null);
     }
   } else {
     try {
       callComponentWillUnmountWithTimer(current, instance);
     } catch (unmountError) {
-      captureCommitPhaseError(current, unmountError);
+      captureCommitPhaseError(current, unmountError, null);
     }
   }
 }
@@ -185,13 +185,13 @@ function safelyDetachRef(current: Fiber) {
         invokeGuardedCallback(null, ref, null, null);
         if (hasCaughtError()) {
           const refError = clearCaughtError();
-          captureCommitPhaseError(current, refError);
+          captureCommitPhaseError(current, refError, null);
         }
       } else {
         try {
           ref(null);
         } catch (refError) {
-          captureCommitPhaseError(current, refError);
+          captureCommitPhaseError(current, refError, null);
         }
       }
     } else {
@@ -200,18 +200,22 @@ function safelyDetachRef(current: Fiber) {
   }
 }
 
-export function safelyCallDestroy(current: Fiber, destroy: () => void) {
+export function safelyCallDestroy(
+  current: Fiber,
+  destroy: () => void,
+  nearestMountedAncestor: Fiber | null,
+) {
   if (__DEV__) {
     invokeGuardedCallback(null, destroy, null);
     if (hasCaughtError()) {
       const error = clearCaughtError();
-      captureCommitPhaseError(current, error);
+      captureCommitPhaseError(current, error, nearestMountedAncestor);
     }
   } else {
     try {
       destroy();
     } catch (error) {
-      captureCommitPhaseError(current, error);
+      captureCommitPhaseError(current, error, nearestMountedAncestor);
     }
   }
 }
@@ -877,10 +881,10 @@ function commitUnmount(
                   current.mode & ProfileMode
                 ) {
                   startLayoutEffectTimer();
-                  safelyCallDestroy(current, destroy);
+                  safelyCallDestroy(current, destroy, null);
                   recordLayoutEffectDuration(current);
                 } else {
-                  safelyCallDestroy(current, destroy);
+                  safelyCallDestroy(current, destroy, null);
                 }
               }
             }

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

@@ -2356,14 +2356,14 @@ function commitBeforeMutationEffects(firstChild: Fiber) {
       invokeGuardedCallback(null, commitBeforeMutationEffectsImpl, null, fiber);
       if (hasCaughtError()) {
         const error = clearCaughtError();
-        captureCommitPhaseError(fiber, error);
+        captureCommitPhaseError(fiber, error, null);
       }
       resetCurrentDebugFiberInDEV();
     } else {
       try {
         commitBeforeMutationEffectsImpl(fiber);
       } catch (error) {
-        captureCommitPhaseError(fiber, error);
+        captureCommitPhaseError(fiber, error, null);
       }
     }
     fiber = fiber.sibling;
@@ -2453,14 +2453,14 @@ function commitMutationEffects(
       );
       if (hasCaughtError()) {
         const error = clearCaughtError();
-        captureCommitPhaseError(fiber, error);
+        captureCommitPhaseError(fiber, error, null);
       }
       resetCurrentDebugFiberInDEV();
     } else {
       try {
         commitMutationEffectsImpl(fiber, root, renderPriorityLevel);
       } catch (error) {
-        captureCommitPhaseError(fiber, error);
+        captureCommitPhaseError(fiber, error, null);
       }
     }
     fiber = fiber.sibling;
@@ -2556,13 +2556,13 @@ function commitMutationEffectsDeletions(
       );
       if (hasCaughtError()) {
         const error = clearCaughtError();
-        captureCommitPhaseError(childToDelete, error);
+        captureCommitPhaseError(childToDelete, error, null);
       }
     } else {
       try {
         commitDeletion(root, childToDelete, renderPriorityLevel);
       } catch (error) {
-        captureCommitPhaseError(childToDelete, error);
+        captureCommitPhaseError(childToDelete, error, null);
       }
     }
   }
@@ -2604,14 +2604,14 @@ function commitLayoutEffects(
       );
       if (hasCaughtError()) {
         const error = clearCaughtError();
-        captureCommitPhaseError(fiber, error);
+        captureCommitPhaseError(fiber, error, null);
       }
       resetCurrentDebugFiberInDEV();
     } else {
       try {
         commitLayoutEffectsImpl(fiber, root, committedLanes);
       } catch (error) {
-        captureCommitPhaseError(fiber, error);
+        captureCommitPhaseError(fiber, error, null);
       }
     }
     fiber = fiber.sibling;
@@ -2750,7 +2750,7 @@ function flushPassiveMountEffectsImpl(fiber: Fiber): void {
           if (hasCaughtError()) {
             invariant(fiber !== null, 'Should be working on an effect.');
             const error = clearCaughtError();
-            captureCommitPhaseError(fiber, error);
+            captureCommitPhaseError(fiber, error, null);
           }
         } else {
           try {
@@ -2771,7 +2771,7 @@ function flushPassiveMountEffectsImpl(fiber: Fiber): void {
             }
           } catch (error) {
             invariant(fiber !== null, 'Should be working on an effect.');
-            captureCommitPhaseError(fiber, error);
+            captureCommitPhaseError(fiber, error, null);
           }
         }
       }
@@ -2790,7 +2790,7 @@ function flushPassiveUnmountEffects(firstChild: Fiber): void {
     if (deletions !== null) {
       for (let i = 0; i < deletions.length; i++) {
         const fiberToDelete = deletions[i];
-        flushPassiveUnmountEffectsInsideOfDeletedTree(fiberToDelete);
+        flushPassiveUnmountEffectsInsideOfDeletedTree(fiberToDelete, fiber);
 
         // Now that passive effects have been processed, it's safe to detach lingering pointers.
         detachFiberAfterEffects(fiberToDelete);
@@ -2816,7 +2816,11 @@ function flushPassiveUnmountEffects(firstChild: Fiber): void {
       case Block: {
         const primaryEffectTag = fiber.effectTag & Passive;
         if (primaryEffectTag !== NoEffect) {
-          flushPassiveUnmountEffectsImpl(fiber, HookPassive | HookHasEffect);
+          flushPassiveUnmountEffectsImpl(
+            fiber,
+            HookPassive | HookHasEffect,
+            null,
+          );
         }
       }
     }
@@ -2827,6 +2831,7 @@ function flushPassiveUnmountEffects(firstChild: Fiber): void {
 
 function flushPassiveUnmountEffectsInsideOfDeletedTree(
   fiberToDelete: Fiber,
+  nearestMountedAncestor: Fiber,
 ): void {
   if ((fiberToDelete.subtreeTag & PassiveStaticSubtreeTag) !== NoSubtreeTag) {
     // If any children have passive effects then traverse the subtree.
@@ -2835,7 +2840,10 @@ function flushPassiveUnmountEffectsInsideOfDeletedTree(
     // since that would not cover passive effects in siblings.
     let child = fiberToDelete.child;
     while (child !== null) {
-      flushPassiveUnmountEffectsInsideOfDeletedTree(child);
+      flushPassiveUnmountEffectsInsideOfDeletedTree(
+        child,
+        nearestMountedAncestor,
+      );
       child = child.sibling;
     }
   }
@@ -2846,7 +2854,11 @@ function flushPassiveUnmountEffectsInsideOfDeletedTree(
       case ForwardRef:
       case SimpleMemoComponent:
       case Block: {
-        flushPassiveUnmountEffectsImpl(fiberToDelete, HookPassive);
+        flushPassiveUnmountEffectsImpl(
+          fiberToDelete,
+          HookPassive,
+          nearestMountedAncestor,
+        );
       }
     }
   }
@@ -2857,6 +2869,7 @@ function flushPassiveUnmountEffectsImpl(
   // Tags to check for when deciding whether to unmount. e.g. to skip over
   // layout effects
   hookEffectTag: HookEffectTag,
+  nearestMountedAncestor: Fiber | null,
 ): void {
   const updateQueue: FunctionComponentUpdateQueue | null = (fiber.updateQueue: any);
   const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null;
@@ -2877,10 +2890,10 @@ function flushPassiveUnmountEffectsImpl(
             fiber.mode & ProfileMode
           ) {
             startPassiveEffectTimer();
-            safelyCallDestroy(fiber, destroy);
+            safelyCallDestroy(fiber, destroy, nearestMountedAncestor);
             recordPassiveEffectDuration(fiber);
           } else {
-            safelyCallDestroy(fiber, destroy);
+            safelyCallDestroy(fiber, destroy, nearestMountedAncestor);
           }
         }
       }
@@ -3013,7 +3026,11 @@ function captureCommitPhaseErrorOnRoot(
   }
 }
 
-export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) {
+export function captureCommitPhaseError(
+  sourceFiber: Fiber,
+  error: mixed,
+  nearestMountedAncestor: Fiber | null,
+) {
   if (sourceFiber.tag === HostRoot) {
     // Error was thrown at the root. There is no parent, so the root
     // itself should capture it.
@@ -3022,6 +3039,7 @@ export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) {
   }
 
   let fiber = sourceFiber.return;
+  let foundErrorBoundary = false;
   while (fiber !== null) {
     if (fiber.tag === HostRoot) {
       captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error);
@@ -3034,6 +3052,8 @@ export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) {
         (typeof instance.componentDidCatch === 'function' &&
           !isAlreadyFailedLegacyErrorBoundary(instance))
       ) {
+        foundErrorBoundary = true;
+
         const errorInfo = createCapturedValue(error, sourceFiber);
         const update = createClassErrorUpdate(
           fiber,
@@ -3047,12 +3067,49 @@ export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) {
           markRootUpdated(root, SyncLane, eventTime);
           ensureRootIsScheduled(root, eventTime);
           schedulePendingInteractions(root, SyncLane);
+        } else {
+          // This component has already been unmounted.
+          // We can't schedule any follow up work for the root because the fiber is already unmounted,
+          // but we can still call the log-only boundary so the error isn't swallowed.
+          // TODO We will need to rethink this for function component boundaries
+          if (
+            typeof instance.componentDidCatch === 'function' &&
+            !isAlreadyFailedLegacyErrorBoundary(instance)
+          ) {
+            try {
+              instance.componentDidCatch(error, errorInfo);
+            } catch {}
+          }
         }
         return;
       }
     }
     fiber = fiber.return;
   }
+
+  if (!foundErrorBoundary) {
+    if (nearestMountedAncestor !== null) {
+      // An error was thrown inside of an unmounted subtree that contained no error boundaries.
+      // Bubble up to the nearest still-mounted error boundary for loggin purposes.
+      fiber = nearestMountedAncestor;
+      while (fiber !== null) {
+        if (fiber.tag === ClassComponent) {
+          const instance = fiber.stateNode;
+          if (
+            typeof instance.componentDidCatch === 'function' &&
+            !isAlreadyFailedLegacyErrorBoundary(instance)
+          ) {
+            const errorInfo = createCapturedValue(error, sourceFiber);
+            try {
+              instance.componentDidCatch(error, errorInfo);
+            } catch {}
+            return;
+          }
+        }
+        fiber = fiber.return;
+      }
+    }
+  }
 }
 
 export function pingSuspendedRoot(

packages/react-reconciler/src/ReactFiberWorkLoop.old.js

@@ -2806,6 +2806,19 @@ export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) {
           markRootUpdated(root, SyncLane, eventTime);
           ensureRootIsScheduled(root, eventTime);
           schedulePendingInteractions(root, SyncLane);
+        } else {
+          // This component has already been unmounted.
+          // We can't schedule any follow up work for the root because the fiber is already unmounted,
+          // but we can still call the log-only boundary so the error isn't swallowed.
+          // TODO We will need to rethink this for function component boundaries
+          if (
+            typeof instance.componentDidCatch === 'function' &&
+            !isAlreadyFailedLegacyErrorBoundary(instance)
+          ) {
+            try {
+              instance.componentDidCatch(error, errorInfo);
+            } catch {}
+          }
         }
         return;
       }