Only log hydration error if client render succeeds

Similar to previous step.

When an error occurs during hydration, we only want to log it if falling
back to client rendering _succeeds_. If client rendering fails,
the error will get reported to the nearest error boundary, so there's
no need for a duplicate log.

To implement this, I added a list of errors to the hydration context.
If the Suspense boundary successfully completes, they are added to
the main recoverable errors queue (the one I added in the
previous step.)

Author: acdlite

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

@@ -1983,14 +1983,15 @@ describe('ReactDOMFizzServer', () => {
       isClient = true;
       ReactDOM.hydrateRoot(container, <App />, {
         onRecoverableError(error) {
-          // TODO: We logged a hydration error, but the same error ends up
-          // being thrown during the fallback to client rendering, too. Maybe
-          // we should only log if the client render succeeds.
-          Scheduler.unstable_yieldValue(error.message);
+          Scheduler.unstable_yieldValue(
+            'Log recoverable error: ' + error.message,
+          );
         },
       });
 
-      expect(Scheduler).toFlushAndYield(['Oops!']);
+      // Because we failed to recover from the error, onRecoverableError
+      // shouldn't be called.
+      expect(Scheduler).toFlushAndYield([]);
       expect(getVisibleChildren(container)).toEqual('Oops!');
     },
   );

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

@@ -214,11 +214,7 @@ describe('ReactDOMServerPartialHydration', () => {
       },
     });
     if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) {
-      // Hydration error is logged
-      expect(Scheduler).toFlushAndYield([
-        'An error occurred during hydration. The server HTML was replaced ' +
-          'with client content',
-      ]);
+      Scheduler.unstable_flushAll();
     } else {
       expect(() => {
         Scheduler.unstable_flushAll();
@@ -309,13 +305,6 @@ describe('ReactDOMServerPartialHydration', () => {
       'Component',
       'Component',
       'Component',
-
-      // Hydration mismatch errors are logged.
-      // TODO: This could get noisy. Is there some way to dedupe?
-      'An error occurred during hydration. The server HTML was replaced with client content',
-      'An error occurred during hydration. The server HTML was replaced with client content',
-      'An error occurred during hydration. The server HTML was replaced with client content',
-      'An error occurred during hydration. The server HTML was replaced with client content',
     ]);
     jest.runAllTimers();
 

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

@@ -131,6 +131,7 @@ import {
   resetHydrationState,
   getIsHydrating,
   hasUnhydratedTailNodes,
+  queueRecoverableHydrationErrors,
 } from './ReactFiberHydrationContext.new';
 import {
   enableSuspenseCallback,
@@ -1099,6 +1100,12 @@ function completeWork(
             return null;
           }
         }
+
+        // Successfully completed this tree. If this was a forced client render,
+        // there may have been recoverable errors during first hydration
+        // attempt. If so, add them to a queue so we can log them in the
+        // commit phase.
+        queueRecoverableHydrationErrors();
       }
 
       if ((workInProgress.flags & DidCapture) !== NoFlags) {

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

@@ -131,6 +131,7 @@ import {
   resetHydrationState,
   getIsHydrating,
   hasUnhydratedTailNodes,
+  queueRecoverableHydrationErrors,
 } from './ReactFiberHydrationContext.old';
 import {
   enableSuspenseCallback,
@@ -1099,6 +1100,12 @@ function completeWork(
             return null;
           }
         }
+
+        // Successfully completed this tree. If this was a forced client render,
+        // there may have been recoverable errors during first hydration
+        // attempt. If so, add them to a queue so we can log them in the
+        // commit phase.
+        queueRecoverableHydrationErrors();
       }
 
       if ((workInProgress.flags & DidCapture) !== NoFlags) {

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

@@ -77,13 +77,17 @@ import {
   getSuspendedTreeContext,
   restoreSuspendedTreeContext,
 } from './ReactFiberTreeContext.new';
+import {queueRecoverableErrors} from './ReactFiberWorkLoop.new';
 
 // The deepest Fiber on the stack involved in a hydration context.
 // This may have been an insertion or a hydration.
 let hydrationParentFiber: null | Fiber = null;
 let nextHydratableInstance: null | HydratableInstance = null;
 let isHydrating: boolean = false;
 
+// Hydration errors that were thrown inside this boundary
+let hydrationErrors: Array<mixed> | null = null;
+
 function warnIfHydrating() {
   if (__DEV__) {
     if (isHydrating) {
@@ -105,6 +109,7 @@ function enterHydrationState(fiber: Fiber): boolean {
   );
   hydrationParentFiber = fiber;
   isHydrating = true;
+  hydrationErrors = null;
   return true;
 }
 
@@ -121,6 +126,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
   );
   hydrationParentFiber = fiber;
   isHydrating = true;
+  hydrationErrors = null;
   if (treeContext !== null) {
     restoreSuspendedTreeContext(fiber, treeContext);
   }
@@ -601,10 +607,28 @@ function resetHydrationState(): void {
   isHydrating = false;
 }
 
+export function queueRecoverableHydrationErrors(): void {
+  if (hydrationErrors !== null) {
+    // Successfully completed a forced client render. The errors that occurred
+    // during the hydration attempt are now recovered. We will log them in
+    // commit phase, once the entire tree has finished.
+    queueRecoverableErrors(hydrationErrors);
+    hydrationErrors = null;
+  }
+}
+
 function getIsHydrating(): boolean {
   return isHydrating;
 }
 
+export function queueHydrationError(error: mixed): void {
+  if (hydrationErrors === null) {
+    hydrationErrors = [error];
+  } else {
+    hydrationErrors.push(error);
+  }
+}
+
 export {
   warnIfHydrating,
   enterHydrationState,

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

@@ -77,13 +77,17 @@ import {
   getSuspendedTreeContext,
   restoreSuspendedTreeContext,
 } from './ReactFiberTreeContext.old';
+import {queueRecoverableErrors} from './ReactFiberWorkLoop.old';
 
 // The deepest Fiber on the stack involved in a hydration context.
 // This may have been an insertion or a hydration.
 let hydrationParentFiber: null | Fiber = null;
 let nextHydratableInstance: null | HydratableInstance = null;
 let isHydrating: boolean = false;
 
+// Hydration errors that were thrown inside this boundary
+let hydrationErrors: Array<mixed> | null = null;
+
 function warnIfHydrating() {
   if (__DEV__) {
     if (isHydrating) {
@@ -105,6 +109,7 @@ function enterHydrationState(fiber: Fiber): boolean {
   );
   hydrationParentFiber = fiber;
   isHydrating = true;
+  hydrationErrors = null;
   return true;
 }
 
@@ -121,6 +126,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
   );
   hydrationParentFiber = fiber;
   isHydrating = true;
+  hydrationErrors = null;
   if (treeContext !== null) {
     restoreSuspendedTreeContext(fiber, treeContext);
   }
@@ -601,10 +607,28 @@ function resetHydrationState(): void {
   isHydrating = false;
 }
 
+export function queueRecoverableHydrationErrors(): void {
+  if (hydrationErrors !== null) {
+    // Successfully completed a forced client render. The errors that occurred
+    // during the hydration attempt are now recovered. We will log them in
+    // commit phase, once the entire tree has finished.
+    queueRecoverableErrors(hydrationErrors);
+    hydrationErrors = null;
+  }
+}
+
 function getIsHydrating(): boolean {
   return isHydrating;
 }
 
+export function queueHydrationError(error: mixed): void {
+  if (hydrationErrors === null) {
+    hydrationErrors = [error];
+  } else {
+    hydrationErrors.push(error);
+  }
+}
+
 export {
   warnIfHydrating,
   enterHydrationState,

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

@@ -37,7 +37,6 @@ import {
 import {
   supportsPersistence,
   getOffscreenContainerProps,
-  logRecoverableError,
 } from './ReactFiberHostConfig';
 import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new';
 import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
@@ -80,7 +79,10 @@ import {
   mergeLanes,
   pickArbitraryLane,
 } from './ReactFiberLane.new';
-import {getIsHydrating} from './ReactFiberHydrationContext.new';
+import {
+  getIsHydrating,
+  queueHydrationError,
+} from './ReactFiberHydrationContext.new';
 
 const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
 
@@ -511,11 +513,7 @@ function throwException(
 
         // Even though the user may not be affected by this error, we should
         // still log it so it can be fixed.
-        // TODO: For now, we only log errors that occur during hydration, but we
-        // probably want to log any error that is recovered from without
-        // triggering an error boundary — or maybe even those, too. Need to
-        // figure out the right API.
-        logRecoverableError(root.errorLoggingConfig, value);
+        queueHydrationError(value);
         return;
       }
     } else {

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

@@ -37,7 +37,6 @@ import {
 import {
   supportsPersistence,
   getOffscreenContainerProps,
-  logRecoverableError,
 } from './ReactFiberHostConfig';
 import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.old';
 import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
@@ -80,7 +79,10 @@ import {
   mergeLanes,
   pickArbitraryLane,
 } from './ReactFiberLane.old';
-import {getIsHydrating} from './ReactFiberHydrationContext.old';
+import {
+  getIsHydrating,
+  queueHydrationError,
+} from './ReactFiberHydrationContext.old';
 
 const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
 
@@ -511,11 +513,7 @@ function throwException(
 
         // Even though the user may not be affected by this error, we should
         // still log it so it can be fixed.
-        // TODO: For now, we only log errors that occur during hydration, but we
-        // probably want to log any error that is recovered from without
-        // triggering an error boundary — or maybe even those, too. Need to
-        // figure out the right API.
-        logRecoverableError(root.errorLoggingConfig, value);
+        queueHydrationError(value);
         return;
       }
     } else {

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

@@ -909,14 +909,7 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
       // The errors from the failed first attempt have been recovered. Add
       // them to the collection of recoverable errors. We'll log them in the
       // commit phase.
-      if (workInProgressRootConcurrentErrors === null) {
-        workInProgressRootRecoverableErrors = errorsFromFirstAttempt;
-      } else {
-        workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
-          null,
-          errorsFromFirstAttempt,
-        );
-      }
+      queueRecoverableErrors(errorsFromFirstAttempt);
     }
   } else {
     // The UI failed to recover.
@@ -927,6 +920,17 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
   return exitStatus;
 }
 
+export function queueRecoverableErrors(errors: Array<mixed>) {
+  if (workInProgressRootConcurrentErrors === null) {
+    workInProgressRootRecoverableErrors = errors;
+  } else {
+    workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
+      null,
+      errors,
+    );
+  }
+}
+
 function finishConcurrentRender(root, exitStatus, lanes) {
   switch (exitStatus) {
     case RootIncomplete:

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

@@ -909,14 +909,7 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
       // The errors from the failed first attempt have been recovered. Add
       // them to the collection of recoverable errors. We'll log them in the
       // commit phase.
-      if (workInProgressRootConcurrentErrors === null) {
-        workInProgressRootRecoverableErrors = errorsFromFirstAttempt;
-      } else {
-        workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
-          null,
-          errorsFromFirstAttempt,
-        );
-      }
+      queueRecoverableErrors(errorsFromFirstAttempt);
     }
   } else {
     // The UI failed to recover.
@@ -927,6 +920,17 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
   return exitStatus;
 }
 
+export function queueRecoverableErrors(errors: Array<mixed>) {
+  if (workInProgressRootConcurrentErrors === null) {
+    workInProgressRootRecoverableErrors = errors;
+  } else {
+    workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
+      null,
+      errors,
+    );
+  }
+}
+
 function finishConcurrentRender(root, exitStatus, lanes) {
   switch (exitStatus) {
     case RootIncomplete: