Include regular stack trace in serialized errors from Fizz (facebook#…

…28684) We previously only included the component stack. Cleaned up the fields in Fizz server that wasn't using consistent hidden classes in dev vs prod. Added a prefix to errors serialized from server rendering. It can be a bit confusing to see where this error came from otherwise since it didn't come from elsewhere on the client. It's really kind of confusing with other recoverable errors that happen on the client too.
EdisonVan · Apr 15, 2024 · 545dd3f · 545dd3f
1 parent 37f53ee
commit 545dd3f
Show file tree

Hide file tree

Showing 13 changed files with 229 additions and 104 deletions.
diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
@@ -1226,22 +1226,29 @@ export function isSuspenseInstanceFallback(
 
 export function getSuspenseInstanceFallbackErrorDetails(
   instance: SuspenseInstance,
-): {digest: ?string, message?: string, stack?: string} {
+): {
+  digest: ?string,
+  message?: string,
+  stack?: string,
+  componentStack?: string,
+} {
   const dataset =
     instance.nextSibling && ((instance.nextSibling: any): HTMLElement).dataset;
-  let digest, message, stack;
+  let digest, message, stack, componentStack;
   if (dataset) {
     digest = dataset.dgst;
     if (__DEV__) {
       message = dataset.msg;
       stack = dataset.stck;
+      componentStack = dataset.cstck;
     }
   }
   if (__DEV__) {
     return {
       message,
       digest,
       stack,
+      componentStack,
     };
   } else {
     // Object gets DCE'd if constructed in tail position and matches callsite destructuring

diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js b/packages/react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js
@@ -94,6 +94,7 @@ function handleNode(node_: Node) {
       dataset['dgst'],
       dataset['msg'],
       dataset['stck'],
+      dataset['cstck'],
     );
     node.remove();
   } else if (dataset['rri'] != null) {

diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
@@ -3801,6 +3801,8 @@ const clientRenderedSuspenseBoundaryError1B =
   stringToPrecomputedChunk(' data-msg="');
 const clientRenderedSuspenseBoundaryError1C =
   stringToPrecomputedChunk(' data-stck="');
+const clientRenderedSuspenseBoundaryError1D =
+  stringToPrecomputedChunk(' data-cstck="');
 const clientRenderedSuspenseBoundaryError2 =
   stringToPrecomputedChunk('></template>');
 
@@ -3843,7 +3845,8 @@ export function writeStartClientRenderedSuspenseBoundary(
   destination: Destination,
   renderState: RenderState,
   errorDigest: ?string,
-  errorMesssage: ?string,
+  errorMessage: ?string,
+  errorStack: ?string,
   errorComponentStack: ?string,
 ): boolean {
   let result;
@@ -3861,19 +3864,27 @@ export function writeStartClientRenderedSuspenseBoundary(
     );
   }
   if (__DEV__) {
-    if (errorMesssage) {
+    if (errorMessage) {
       writeChunk(destination, clientRenderedSuspenseBoundaryError1B);
       writeChunk(
         destination,
-        stringToChunk(escapeTextForBrowser(errorMesssage)),
+        stringToChunk(escapeTextForBrowser(errorMessage)),
       );
       writeChunk(
         destination,
         clientRenderedSuspenseBoundaryErrorAttrInterstitial,
       );
     }
-    if (errorComponentStack) {
+    if (errorStack) {
       writeChunk(destination, clientRenderedSuspenseBoundaryError1C);
+      writeChunk(destination, stringToChunk(escapeTextForBrowser(errorStack)));
+      writeChunk(
+        destination,
+        clientRenderedSuspenseBoundaryErrorAttrInterstitial,
+      );
+    }
+    if (errorComponentStack) {
+      writeChunk(destination, clientRenderedSuspenseBoundaryError1D);
       writeChunk(
         destination,
         stringToChunk(escapeTextForBrowser(errorComponentStack)),
@@ -4236,6 +4247,7 @@ const clientRenderData1 = stringToPrecomputedChunk(
 const clientRenderData2 = stringToPrecomputedChunk('" data-dgst="');
 const clientRenderData3 = stringToPrecomputedChunk('" data-msg="');
 const clientRenderData4 = stringToPrecomputedChunk('" data-stck="');
+const clientRenderData5 = stringToPrecomputedChunk('" data-cstck="');
 const clientRenderDataEnd = dataElementQuotedEnd;
 
 export function writeClientRenderBoundaryInstruction(
@@ -4244,8 +4256,9 @@ export function writeClientRenderBoundaryInstruction(
   renderState: RenderState,
   id: number,
   errorDigest: ?string,
-  errorMessage?: string,
-  errorComponentStack?: string,
+  errorMessage: ?string,
+  errorStack: ?string,
+  errorComponentStack: ?string,
 ): boolean {
   const scriptFormat =
     !enableFizzExternalRuntime ||
@@ -4276,7 +4289,7 @@ export function writeClientRenderBoundaryInstruction(
     writeChunk(destination, clientRenderScript1A);
   }
 
-  if (errorDigest || errorMessage || errorComponentStack) {
+  if (errorDigest || errorMessage || errorStack || errorComponentStack) {
     if (scriptFormat) {
       // ,"JSONString"
       writeChunk(destination, clientRenderErrorScriptArgInterstitial);
@@ -4293,7 +4306,7 @@ export function writeClientRenderBoundaryInstruction(
       );
     }
   }
-  if (errorMessage || errorComponentStack) {
+  if (errorMessage || errorStack || errorComponentStack) {
     if (scriptFormat) {
       // ,"JSONString"
       writeChunk(destination, clientRenderErrorScriptArgInterstitial);
@@ -4310,6 +4323,23 @@ export function writeClientRenderBoundaryInstruction(
       );
     }
   }
+  if (errorStack || errorComponentStack) {
+    // ,"JSONString"
+    if (scriptFormat) {
+      writeChunk(destination, clientRenderErrorScriptArgInterstitial);
+      writeChunk(
+        destination,
+        stringToChunk(escapeJSStringsForInstructionScripts(errorStack || '')),
+      );
+    } else {
+      // " data-stck="HTMLString
+      writeChunk(destination, clientRenderData4);
+      writeChunk(
+        destination,
+        stringToChunk(escapeTextForBrowser(errorStack || '')),
+      );
+    }
+  }
   if (errorComponentStack) {
     // ,"JSONString"
     if (scriptFormat) {
@@ -4321,8 +4351,8 @@ export function writeClientRenderBoundaryInstruction(
         ),
       );
     } else {
-      // " data-stck="HTMLString
-      writeChunk(destination, clientRenderData4);
+      // " data-cstck="HTMLString
+      writeChunk(destination, clientRenderData5);
       writeChunk(
         destination,
         stringToChunk(escapeTextForBrowser(errorComponentStack)),

diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
@@ -219,6 +219,7 @@ export function writeStartClientRenderedSuspenseBoundary(
   // flushing these error arguments are not currently supported in this legacy streaming format.
   errorDigest: ?string,
   errorMessage: ?string,
+  errorStack: ?string,
   errorComponentStack: ?string,
 ): boolean {
   if (renderState.generateStaticMarkup) {
@@ -231,6 +232,7 @@ export function writeStartClientRenderedSuspenseBoundary(
     renderState,
     errorDigest,
     errorMessage,
+    errorStack,
     errorComponentStack,
   );
 }

diff --git a/...m-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js b/...m-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js
diff --git a/...es/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetShared.js b/...es/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetShared.js
@@ -19,6 +19,7 @@ export function clientRenderBoundary(
   suspenseBoundaryID,
   errorDigest,
   errorMsg,
+  errorStack,
   errorComponentStack,
 ) {
   // Find the fallback's first element.
@@ -36,7 +37,8 @@ export function clientRenderBoundary(
   const dataset = suspenseIdNode.dataset;
   if (errorDigest) dataset['dgst'] = errorDigest;
   if (errorMsg) dataset['msg'] = errorMsg;
-  if (errorComponentStack) dataset['stck'] = errorComponentStack;
+  if (errorStack) dataset['stck'] = errorStack;
+  if (errorComponentStack) dataset['cstck'] = errorComponentStack;
   // Tell React to retry it if the parent already hydrated.
   if (suspenseNode['_reactRetry']) {
     suspenseNode['_reactRetry']();

diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
@@ -796,7 +796,8 @@ describe('ReactDOMFizzServer', () => {
       errors,
       [
         [
-          theError.message,
+          'Switched to client rendering because the server rendering errored:\n\n' +
+            theError.message,
           expectedDigest,
           componentStack(['Lazy', 'Suspense', 'div', 'App']),
         ],
@@ -919,7 +920,8 @@ describe('ReactDOMFizzServer', () => {
       errors,
       [
         [
-          theError.message,
+          'Switched to client rendering because the server rendering errored:\n\n' +
+            theError.message,
           expectedDigest,
           componentStack(['Lazy', 'Suspense', 'div', 'App']),
         ],
@@ -1002,7 +1004,8 @@ describe('ReactDOMFizzServer', () => {
       errors,
       [
         [
-          theError.message,
+          'Switched to client rendering because the server rendering errored:\n\n' +
+            theError.message,
           expectedDigest,
           componentStack([
             'Erroring',
@@ -1088,7 +1091,8 @@ describe('ReactDOMFizzServer', () => {
       errors,
       [
         [
-          theError.message,
+          'Switched to client rendering because the server rendering errored:\n\n' +
+            theError.message,
           expectedDigest,
           componentStack(['Lazy', 'Suspense', 'div', 'App']),
         ],
@@ -1414,13 +1418,15 @@ describe('ReactDOMFizzServer', () => {
       errors,
       [
         [
-          'The server did not finish this Suspense boundary: The render was aborted by the server without a reason.',
+          'Switched to client rendering because the server rendering aborted due to:\n\n' +
+            'The render was aborted by the server without a reason.',
           expectedDigest,
           // We get the stack of the task when it was aborted which is why we see `h1`
           componentStack(['h1', 'Suspense', 'div', 'App']),
         ],
         [
-          'The server did not finish this Suspense boundary: The render was aborted by the server without a reason.',
+          'Switched to client rendering because the server rendering aborted due to:\n\n' +
+            'The render was aborted by the server without a reason.',
           expectedDigest,
           componentStack(['Suspense', 'main', 'div', 'App']),
         ],
@@ -2155,7 +2161,8 @@ describe('ReactDOMFizzServer', () => {
       errors,
       [
         [
-          theError.message,
+          'Switched to client rendering because the server rendering errored:\n\n' +
+            theError.message,
           expectedDigest,
           componentStack([
             'AsyncText',
@@ -3441,12 +3448,14 @@ describe('ReactDOMFizzServer', () => {
       errors,
       [
         [
-          'The server did not finish this Suspense boundary: foobar',
+          'Switched to client rendering because the server rendering aborted due to:\n\n' +
+            'foobar',
           'a digest',
           componentStack(['Suspense', 'p', 'div', 'App']),
         ],
         [
-          'The server did not finish this Suspense boundary: foobar',
+          'Switched to client rendering because the server rendering aborted due to:\n\n' +
+            'foobar',
           'a digest',
           componentStack(['Suspense', 'span', 'div', 'App']),
         ],
@@ -3522,12 +3531,14 @@ describe('ReactDOMFizzServer', () => {
       errors,
       [
         [
-          'The server did not finish this Suspense boundary: uh oh',
+          'Switched to client rendering because the server rendering aborted due to:\n\n' +
+            'uh oh',
           'a digest',
           componentStack(['Suspense', 'p', 'div', 'App']),
         ],
         [
-          'The server did not finish this Suspense boundary: uh oh',
+          'Switched to client rendering because the server rendering aborted due to:\n\n' +
+            'uh oh',
           'a digest',
           componentStack(['Suspense', 'span', 'div', 'App']),
         ],
@@ -4001,7 +4012,8 @@ describe('ReactDOMFizzServer', () => {
         errors,
         [
           [
-            theError.message,
+            'Switched to client rendering because the server rendering errored:\n\n' +
+              theError.message,
             expectedDigest,
             componentStack(['Erroring', 'Suspense', 'div', 'App']),
           ],
@@ -6782,7 +6794,14 @@ describe('ReactDOMFizzServer', () => {
 
     expect(recoverableErrors).toEqual(
       __DEV__
-        ? ['server error', 'replay error', 'server error']
+        ? [
+            'Switched to client rendering because the server rendering errored:\n\n' +
+              'server error',
+            'Switched to client rendering because the server rendering errored:\n\n' +
+              'replay error',
+            'Switched to client rendering because the server rendering errored:\n\n' +
+              'server error',
+          ]
         : [
             'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
             'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
@@ -6941,8 +6960,10 @@ describe('ReactDOMFizzServer', () => {
     expect(recoverableErrors).toEqual(
       __DEV__
         ? [
-            'The server did not finish this Suspense boundary: aborted',
-            'The server did not finish this Suspense boundary: aborted',
+            'Switched to client rendering because the server rendering aborted due to:\n\n' +
+              'aborted',
+            'Switched to client rendering because the server rendering aborted due to:\n\n' +
+              'aborted',
           ]
         : [
             'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
@@ -7113,8 +7134,10 @@ describe('ReactDOMFizzServer', () => {
       // It surfaced in two different suspense boundaries.
       __DEV__
         ? [
-            'The server did not finish this Suspense boundary: replay error',
-            'The server did not finish this Suspense boundary: replay error',
+            'Switched to client rendering because the server rendering errored:\n\n' +
+              'replay error',
+            'Switched to client rendering because the server rendering errored:\n\n' +
+              'replay error',
           ]
         : [
             'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
@@ -7240,7 +7263,10 @@ describe('ReactDOMFizzServer', () => {
 
     expect(recoverableErrors).toEqual(
       __DEV__
-        ? ['server error']
+        ? [
+            'Switched to client rendering because the server rendering errored:\n\n' +
+              'server error',
+          ]
         : [
             'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
           ],