Retry once after error even for sync renders

Except in legacy mode.

This is to support the `useOpaqueReference` hook, which uses an error
to trigger a retry at lower priority.

Author: acdlite

packages/react-reconciler/src/ReactFiberRoot.js

@@ -74,6 +74,10 @@ type BaseFiberRootProperties = {|
   // render again
   lastPingedTime: ExpirationTime,
   lastExpiredTime: ExpirationTime,
+
+  // Set to true when the root completes with an error. We'll try rendering
+  // again. This field ensures we don't retry more than once.
+  didError: boolean,
 |};
 
 // The following attributes are only used by interaction tracing builds.
@@ -123,6 +127,7 @@ function FiberRootNode(containerInfo, tag, hydrate) {
   this.nextKnownPendingLevel = NoWork;
   this.lastPingedTime = NoWork;
   this.lastExpiredTime = NoWork;
+  this.didError = false;
 
   if (enableSchedulerTracing) {
     this.interactionThreadID = unstable_getThreadID();
@@ -249,6 +254,8 @@ export function markRootFinishedAtTime(
     // Clear the expired time
     root.lastExpiredTime = NoWork;
   }
+
+  root.didError = false;
 }
 
 export function markRootExpiredAtTime(

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -89,6 +89,7 @@ import {
   SimpleMemoComponent,
   Chunk,
 } from 'shared/ReactWorkTags';
+import {LegacyRoot} from 'shared/ReactRootTags';
 import {
   NoEffect,
   PerformedWork,
@@ -647,7 +648,10 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
   // Check if the render expired.
   const expirationTime = getNextRootExpirationTimeToWorkOn(root);
   if (didTimeout) {
-    if (workInProgressRoot === root && renderExpirationTime !== NoWork) {
+    if (
+      root === workInProgressRoot &&
+      expirationTime === renderExpirationTime
+    ) {
       // We're already in the middle of working on this tree. Expire the tree at
       // the already-rendering time so we don't throw out the partial work. If
       // there's another expired level after that, we'll hit the `else` branch
@@ -776,6 +780,7 @@ function finishConcurrentRender(
         root,
         expirationTime > Idle ? Idle : expirationTime,
       );
+      root.didError = true;
       // We assume that this second render pass will be synchronous
       // and therefore not hit this path again.
       break;
@@ -1078,7 +1083,25 @@ function finishSyncRender(root, exitStatus, expirationTime) {
       flushSuspensePriorityWarningInDEV();
     }
   }
-  commitRoot(root);
+
+  // Try rendering one more time, synchronously, to see if the error goes away.
+  // If there are lower priority updates, let's include those, too, in case they
+  // fix the inconsistency. Render at Idle to include all updates. If it was
+  // Idle or Never or some not-yet-invented time, render at that time.
+  if (
+    workInProgressRootExitStatus === RootErrored &&
+    // Legacy mode shouldn't retry, only blocking and concurrent mode
+    root.tag !== LegacyRoot &&
+    // Don't retry more than once.
+    !root.didError
+  ) {
+    root.didError = true;
+    markRootExpiredAtTime(root, expirationTime > Idle ? Idle : expirationTime);
+  } else {
+    // In all other cases, since this was a synchronous render,
+    // commit immediately.
+    commitRoot(root);
+  }
 }
 
 export function flushRoot(root: FiberRoot, expirationTime: ExpirationTime) {

packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js

@@ -588,7 +588,12 @@ describe('ReactIncrementalErrorHandling', () => {
     expect(ops).toEqual([
       'ErrorBoundary render success',
       'BrokenRender',
-      // React doesn't retry because we're already rendering synchronously.
+
+      // React retries one more time
+      'ErrorBoundary render success',
+      'BrokenRender',
+
+      // Errored again on retry. Now handle it.
       'ErrorBoundary componentDidCatch',
       'ErrorBoundary render error',
     ]);
@@ -632,7 +637,12 @@ describe('ReactIncrementalErrorHandling', () => {
     expect(ops).toEqual([
       'ErrorBoundary render success',
       'BrokenRender',
-      // React doesn't retry because we're already rendering synchronously.
+
+      // React retries one more time
+      'ErrorBoundary render success',
+      'BrokenRender',
+
+      // Errored again on retry. Now handle it.
       'ErrorBoundary componentDidCatch',
       'ErrorBoundary render error',
     ]);
@@ -751,7 +761,12 @@ describe('ReactIncrementalErrorHandling', () => {
     expect(ops).toEqual([
       'RethrowErrorBoundary render',
       'BrokenRender',
-      // React doesn't retry because we're already rendering synchronously.
+
+      // React retries one more time
+      'RethrowErrorBoundary render',
+      'BrokenRender',
+
+      // Errored again on retry. Now handle it.
       'RethrowErrorBoundary componentDidCatch',
     ]);
     expect(ReactNoop.getChildren()).toEqual([]);
@@ -790,7 +805,12 @@ describe('ReactIncrementalErrorHandling', () => {
     expect(ops).toEqual([
       'RethrowErrorBoundary render',
       'BrokenRender',
-      // React doesn't retry because we're already rendering synchronously.
+
+      // React retries one more time
+      'RethrowErrorBoundary render',
+      'BrokenRender',
+
+      // Errored again on retry. Now handle it.
       'RethrowErrorBoundary componentDidCatch',
     ]);
     expect(ReactNoop.getChildren()).toEqual([]);

packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js

@@ -189,8 +189,14 @@ describe('ReactIncrementalErrorLogging', () => {
       [
         'render: 0',
         __DEV__ && 'render: 0', // replay
+
         'render: 1',
         __DEV__ && 'render: 1', // replay
+
+        // Retry one more time before handling error
+        'render: 1',
+        __DEV__ && 'render: 1', // replay
+
         'componentWillUnmount: 0',
       ].filter(Boolean),
     );