[Fizz] Reset error component stack and fix error messages (#27456)

The way we collect component stacks right now are pretty fragile. We expect that we'll call captureBoundaryErrorDetailsDev whenever an error happens. That resets lastBoundaryErrorComponentStackDev to null but if we don't, it just lingers and we don't set it to anything new then which leaks the previous component stack into the next time we have an error. So we need to reset it in a bunch of places. This is still broken with erroredReplay because it has the inverse problem that abortRemainingReplayNodes can call captureBoundaryErrorDetailsDev more than one time. So the second boundary won't get a stack. We probably should try to figure out an alternative way to carry along the stack. Perhaps WeakMap keyed by the error object. This also fixes an issue where we weren't invoking the onShellReady event if we error a replay. That event is a bit weird for resuming because we're probably really supposed to just invoke it immediately if we have already flushed the shell in the prerender which is always atm. Right now, it gets invoked later than necessary because you could have a resumed hole ready before a sibling in the shell is ready and that's blocked. DiffTrain build for [0fba3ec](0fba3ec)
facebook · Oct 4, 2023 · b239394 · b239394
1 parent 4efc125
commit b239394
Show file tree

Hide file tree

Showing 9 changed files with 316 additions and 183 deletions.
diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION
@@ -1 +1 @@
-ca237d6f0ab986e799f192224d3066f76d66b73b
+0fba3ecf73900a1b54ed6d3b0617462ac92d2fef
diff --git a/compiled/facebook-www/ReactART-dev.modern.js b/compiled/facebook-www/ReactART-dev.modern.js
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
   return self;
 }
 
-var ReactVersion = "18.3.0-www-modern-73c8edac";
+var ReactVersion = "18.3.0-www-modern-55737203";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;

diff --git a/compiled/facebook-www/ReactART-prod.modern.js b/compiled/facebook-www/ReactART-prod.modern.js
@@ -9774,7 +9774,7 @@ var slice = Array.prototype.slice,
       return null;
     },
     bundleType: 0,
-    version: "18.3.0-www-modern-e0184800",
+    version: "18.3.0-www-modern-f919825d",
     rendererPackageName: "react-art"
   };
 var internals$jscomp$inline_1284 = {
@@ -9805,7 +9805,7 @@ var internals$jscomp$inline_1284 = {
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-www-modern-e0184800"
+  reconcilerVersion: "18.3.0-www-modern-f919825d"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
   var hook$jscomp$inline_1285 = __REACT_DEVTOOLS_GLOBAL_HOOK__;

diff --git a/compiled/facebook-www/ReactDOMServer-dev.classic.js b/compiled/facebook-www/ReactDOMServer-dev.classic.js
@@ -19,7 +19,7 @@ if (__DEV__) {
 var React = require("react");
 var ReactDOM = require("react-dom");
 
-var ReactVersion = "18.3.0-www-classic-bdb0331d";
+var ReactVersion = "18.3.0-www-classic-6d747e3d";
 
 // This refers to a WWW module.
 var warningWWW = require("warning");
@@ -10649,8 +10649,7 @@ function replaySuspenseBoundary(
   } // TODO: This should be queued at a separate lower priority queue so that we only work
   // on preparing fallbacks if we don't have any more main content to task on.
 
-  request.pingedTasks.push(suspendedFallbackTask); // TODO: Should this be in the finally?
-
+  request.pingedTasks.push(suspendedFallbackTask);
   popComponentStackInDEV(task);
 }
 
@@ -11422,19 +11421,22 @@ function replayElement(
 
     if (keyOrIndex !== node[1]) {
       continue;
-    } // Let's double check that the component name matches as a precaution.
-
-    if (name !== null && name !== node[0]) {
-      throw new Error(
-        'Expected to see a component of type "' +
-          name +
-          '" in this slot. ' +
-          "The tree doesn't match so React will fallback to client rendering."
-      );
     }
 
     if (node.length === 4) {
       // Matched a replayable path.
+      // Let's double check that the component name matches as a precaution.
+      if (name !== null && name !== node[0]) {
+        throw new Error(
+          "Expected the resume to render <" +
+            node[0] +
+            "> in this slot but instead it rendered <" +
+            name +
+            ">. " +
+            "The tree doesn't match so React will fallback to client rendering."
+        );
+      }
+
       var childNodes = node[2];
       var childSlots = node[3];
       task.replay = {
@@ -11485,8 +11487,13 @@ function replayElement(
     } else {
       // Let's double check that the component type matches.
       if (type !== REACT_SUSPENSE_TYPE) {
+        var expectedType = "Suspense";
         throw new Error(
-          "Expected to see a Suspense boundary in this slot. " +
+          "Expected the resume to render <" +
+            expectedType +
+            "> in this slot but instead it rendered <" +
+            (getComponentNameFromType(type) || "Unknown") +
+            ">. " +
             "The tree doesn't match so React will fallback to client rendering."
         );
       } // Matched a replayable path.
@@ -11851,6 +11858,7 @@ function replayFragment(request, task, children, childIndex) {
       // in the original prerender. What's unable to complete is the child
       // replay nodes which might be Suspense boundaries which are able to
       // absorb the error and we can still continue with siblings.
+      // This is an error, stash the component stack if it is null.
 
       erroredReplay(request, task.blockedBoundary, x, childNodes, childSlots);
     } finally {
@@ -12165,6 +12173,7 @@ function erroredTask(request, boundary, error) {
   }
 
   if (boundary === null) {
+    lastBoundaryErrorComponentStackDev = null;
     fatalError(request, error);
   } else {
     boundary.pendingTasks--;
@@ -12185,6 +12194,8 @@ function erroredTask(request, boundary, error) {
         // We reuse the same queue for errors.
         request.clientRenderedBoundaries.push(boundary);
       }
+    } else {
+      lastBoundaryErrorComponentStackDev = null;
     }
   }
 
@@ -12323,8 +12334,6 @@ function abortTask(task, request, error) {
   }
 
   if (boundary === null) {
-    request.allPendingTasks--;
-
     if (request.status !== CLOSING && request.status !== CLOSED) {
       var replay = task.replay;
 
@@ -12333,6 +12342,7 @@ function abortTask(task, request, error) {
         // the request;
         logRecoverableError(request, error);
         fatalError(request, error);
+        return;
       } else {
         // If the shell aborts during a replay, that's not a fatal error. Instead
         // we should be able to recover by client rendering all the root boundaries in
@@ -12350,6 +12360,14 @@ function abortTask(task, request, error) {
             errorDigest
           );
         }
+
+        request.pendingRootTasks--;
+
+        if (request.pendingRootTasks === 0) {
+          request.onShellError = noop;
+          var onShellReady = request.onShellReady;
+          onShellReady();
+        }
       }
     }
   } else {
@@ -12390,12 +12408,13 @@ function abortTask(task, request, error) {
       return abortTask(fallbackTask, request, error);
     });
     boundary.fallbackAbortableTasks.clear();
-    request.allPendingTasks--;
+  }
 
-    if (request.allPendingTasks === 0) {
-      var onAllReady = request.onAllReady;
-      onAllReady();
-    }
+  request.allPendingTasks--;
+
+  if (request.allPendingTasks === 0) {
+    var onAllReady = request.onAllReady;
+    onAllReady();
   }
 }
 
@@ -12678,6 +12697,14 @@ function retryReplayTask(request, task) {
       task.replay.nodes,
       task.replay.slots
     );
+    request.pendingRootTasks--;
+
+    if (request.pendingRootTasks === 0) {
+      request.onShellError = noop;
+      var onShellReady = request.onShellReady;
+      onShellReady();
+    }
+
     request.allPendingTasks--;
 
     if (request.allPendingTasks === 0) {

diff --git a/compiled/facebook-www/ReactDOMServer-dev.modern.js b/compiled/facebook-www/ReactDOMServer-dev.modern.js
@@ -19,7 +19,7 @@ if (__DEV__) {
 var React = require("react");
 var ReactDOM = require("react-dom");
 
-var ReactVersion = "18.3.0-www-modern-73c8edac";
+var ReactVersion = "18.3.0-www-modern-55737203";
 
 // This refers to a WWW module.
 var warningWWW = require("warning");
@@ -10408,8 +10408,7 @@ function replaySuspenseBoundary(
   } // TODO: This should be queued at a separate lower priority queue so that we only work
   // on preparing fallbacks if we don't have any more main content to task on.
 
-  request.pingedTasks.push(suspendedFallbackTask); // TODO: Should this be in the finally?
-
+  request.pingedTasks.push(suspendedFallbackTask);
   popComponentStackInDEV(task);
 }
 
@@ -11170,19 +11169,22 @@ function replayElement(
 
     if (keyOrIndex !== node[1]) {
       continue;
-    } // Let's double check that the component name matches as a precaution.
-
-    if (name !== null && name !== node[0]) {
-      throw new Error(
-        'Expected to see a component of type "' +
-          name +
-          '" in this slot. ' +
-          "The tree doesn't match so React will fallback to client rendering."
-      );
     }
 
     if (node.length === 4) {
       // Matched a replayable path.
+      // Let's double check that the component name matches as a precaution.
+      if (name !== null && name !== node[0]) {
+        throw new Error(
+          "Expected the resume to render <" +
+            node[0] +
+            "> in this slot but instead it rendered <" +
+            name +
+            ">. " +
+            "The tree doesn't match so React will fallback to client rendering."
+        );
+      }
+
       var childNodes = node[2];
       var childSlots = node[3];
       task.replay = {
@@ -11233,8 +11235,13 @@ function replayElement(
     } else {
       // Let's double check that the component type matches.
       if (type !== REACT_SUSPENSE_TYPE) {
+        var expectedType = "Suspense";
         throw new Error(
-          "Expected to see a Suspense boundary in this slot. " +
+          "Expected the resume to render <" +
+            expectedType +
+            "> in this slot but instead it rendered <" +
+            (getComponentNameFromType(type) || "Unknown") +
+            ">. " +
             "The tree doesn't match so React will fallback to client rendering."
         );
       } // Matched a replayable path.
@@ -11599,6 +11606,7 @@ function replayFragment(request, task, children, childIndex) {
       // in the original prerender. What's unable to complete is the child
       // replay nodes which might be Suspense boundaries which are able to
       // absorb the error and we can still continue with siblings.
+      // This is an error, stash the component stack if it is null.
 
       erroredReplay(request, task.blockedBoundary, x, childNodes, childSlots);
     } finally {
@@ -11913,6 +11921,7 @@ function erroredTask(request, boundary, error) {
   }
 
   if (boundary === null) {
+    lastBoundaryErrorComponentStackDev = null;
     fatalError(request, error);
   } else {
     boundary.pendingTasks--;
@@ -11933,6 +11942,8 @@ function erroredTask(request, boundary, error) {
         // We reuse the same queue for errors.
         request.clientRenderedBoundaries.push(boundary);
       }
+    } else {
+      lastBoundaryErrorComponentStackDev = null;
     }
   }
 
@@ -12071,8 +12082,6 @@ function abortTask(task, request, error) {
   }
 
   if (boundary === null) {
-    request.allPendingTasks--;
-
     if (request.status !== CLOSING && request.status !== CLOSED) {
       var replay = task.replay;
 
@@ -12081,6 +12090,7 @@ function abortTask(task, request, error) {
         // the request;
         logRecoverableError(request, error);
         fatalError(request, error);
+        return;
       } else {
         // If the shell aborts during a replay, that's not a fatal error. Instead
         // we should be able to recover by client rendering all the root boundaries in
@@ -12098,6 +12108,14 @@ function abortTask(task, request, error) {
             errorDigest
           );
         }
+
+        request.pendingRootTasks--;
+
+        if (request.pendingRootTasks === 0) {
+          request.onShellError = noop;
+          var onShellReady = request.onShellReady;
+          onShellReady();
+        }
       }
     }
   } else {
@@ -12138,12 +12156,13 @@ function abortTask(task, request, error) {
       return abortTask(fallbackTask, request, error);
     });
     boundary.fallbackAbortableTasks.clear();
-    request.allPendingTasks--;
+  }
 
-    if (request.allPendingTasks === 0) {
-      var onAllReady = request.onAllReady;
-      onAllReady();
-    }
+  request.allPendingTasks--;
+
+  if (request.allPendingTasks === 0) {
+    var onAllReady = request.onAllReady;
+    onAllReady();
   }
 }
 
@@ -12426,6 +12445,14 @@ function retryReplayTask(request, task) {
       task.replay.nodes,
       task.replay.slots
     );
+    request.pendingRootTasks--;
+
+    if (request.pendingRootTasks === 0) {
+      request.onShellError = noop;
+      var onShellReady = request.onShellReady;
+      onShellReady();
+    }
+
     request.allPendingTasks--;
 
     if (request.allPendingTasks === 0) {
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		ca237d6f0ab986e799f192224d3066f76d66b73b
		0fba3ecf73900a1b54ed6d3b0617462ac92d2fef