Repository: facebook/react
Pull request: Implement onError signature for renderToMarkup

Stacked on #30132.

This way we can get parent and owner stacks from the error.

This forces us to confront multiple errors and whether or not a Flight error that ends up being unobservable needs to really reject the render.

This implements stashing of Flight errors with a digest and only errors if they end up erroring the Fizz render too. At this point they'll have parent stacks so we can surface those.


Changed files:
filename: packages/react-html/src/ReactHTMLClient.js
patch: @@ -8,6 +8,7 @@
  */
 
 import type {ReactNodeList} from 'shared/ReactTypes';
+import type {ErrorInfo} from 'react-server/src/ReactFizzServer';
 
 import ReactVersion from 'shared/ReactVersion';
 
@@ -27,6 +28,7 @@ import {
 type MarkupOptions = {
   identifierPrefix?: string,
   signal?: AbortSignal,
+  onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
 };
 
 export function renderToMarkup(
@@ -49,11 +51,16 @@ export function renderToMarkup(
         reject(error);
       },
     };
-    function onError(error: mixed) {
+    function handleError(error: mixed, errorInfo: ErrorInfo) {
       // Any error rejects the promise, regardless of where it happened.
       // Unlike other React SSR we don't want to put Suspense boundaries into
       // client rendering mode because there's no client rendering here.
       reject(error);
+
+      const onError = options && options.onError;
+      if (onError) {
+        onError(error, errorInfo);
+      }
     }
     const resumableState = createResumableState(
       options ? options.identifierPrefix : undefined,
@@ -72,7 +79,7 @@ export function renderToMarkup(
       ),
       createRootFormatContext(),
       Infinity,
-      onError,
+      handleError,
       undefined,
       undefined,
       undefined,

filename: packages/react-html/src/ReactHTMLServer.js
patch: @@ -9,9 +9,13 @@
 
 import type {ReactNodeList} from 'shared/ReactTypes';
 import type {LazyComponent} from 'react/src/ReactLazy';
+import type {ErrorInfo} from 'react-server/src/ReactFizzServer';
 
 import ReactVersion from 'shared/ReactVersion';
 
+import ReactSharedInternalsServer from 'react-server/src/ReactSharedInternalsServer';
+import ReactSharedInternalsClient from 'shared/ReactSharedInternals';
+
 import {
   createRequest as createFlightRequest,
   startWork as startFlightWork,
@@ -62,6 +66,7 @@ type ReactMarkupNodeList =
 type MarkupOptions = {
   identifierPrefix?: string,
   signal?: AbortSignal,
+  onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
 };
 
 function noServerCallOrFormAction() {
@@ -109,17 +114,60 @@ export function renderToMarkup(
         reject(error);
       },
     };
-    function onError(error: mixed) {
+
+    let stashErrorIdx = 1;
+    const stashedErrors: Map<string, mixed> = new Map();
+
+    function handleFlightError(error: mixed): string {
+      // For Flight errors we don't immediately reject, because they might not matter
+      // to the output of the HTML. We stash the error with a digest in case we need
+      // to get to the original error from the Fizz render.
+      const id = '' + stashErrorIdx++;
+      stashedErrors.set(id, error);
+      return id;
+    }
+
+    function handleError(error: mixed, errorInfo: ErrorInfo) {
+      if (typeof error === 'object' && error !== null) {
+        const id = error.digest;
+        // Note that the original error might be `undefined` so we need a has check.
+        if (typeof id === 'string' && stashedErrors.has(id)) {
+          // Get the original error thrown inside Flight.
+          error = stashedErrors.get(id);
+        }
+      }
+
       // Any error rejects the promise, regardless of where it happened.
       // Unlike other React SSR we don't want to put Suspense boundaries into
       // client rendering mode because there's no client rendering here.
       reject(error);
+
+      const onError = options && options.onError;
+      if (onError) {
+        if (__DEV__) {
+          const prevGetCurrentStackImpl =
+            ReactSharedInternalsServer.getCurrentStack;
+          // We're inside a "client" callback from Fizz but we only have access to the
+          // "server" runtime so to get access to a stack trace within this callback we
+          // need to override it to get it from the client runtime.
+          ReactSharedInternalsServer.getCurrentStack =
+            ReactSharedInternalsClient.getCurrentStack;
+          try {
+            onError(error, errorInfo);
+          } finally {
+            ReactSharedInternalsServer.getCurrentStack =
+              prevGetCurrentStackImpl;
+          }
+        } else {
+          onError(error, errorInfo);
+        }
+      }
     }
     const flightRequest = createFlightRequest(
       // $FlowFixMe: This should be a subtype but not everything is typed covariant.
       children,
       null,
-      onError,
+      handleFlightError,
       options ? options.identifierPrefix : undefined,
       undefined,
       'Markup',
@@ -153,7 +201,7 @@ export function renderToMarkup(
       ),
       createRootFormatContext(),
       Infinity,
-      onError,
+      handleError,
       undefined,
       undefined,
       undefined,

filename: packages/react-html/src/__tests__/ReactHTMLClient-test.js
patch: @@ -12,6 +12,15 @@
 let React;
 let ReactHTML;
 
+function normalizeCodeLocInfo(str) {
+  return (
+    str &&
+    String(str).replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) {
+      return '\n    in ' + name + ' (at **)';
+    })
+  );
+}
+
 if (!__EXPERIMENTAL__) {
   it('should not be built in stable', () => {
     try {
@@ -170,5 +179,58 @@ if (!__EXPERIMENTAL__) {
       const html = await ReactHTML.renderToMarkup(<Component />);
       expect(html).toBe('<div>01</div>');
     });
+
+    it('can get the component owner stacks for onError in dev', async () => {
+      const thrownError = new Error('hi');
+      const caughtErrors = [];
+
+      function Foo() {
+        return <Bar />;
+      }
+      function Bar() {
+        return (
+          <div>
+            <Baz />
+          </div>
+        );
+      }
+      function Baz({unused}) {
+        throw thrownError;
+      }
+
+      await expect(async () => {
+        await ReactHTML.renderToMarkup(
+          <div>
+            <Foo />
+          </div>,
+          {
+            onError(error, errorInfo) {
+              caughtErrors.push({
+                error: error,
+                parentStack: errorInfo.componentStack,
+                ownerStack: React.captureOwnerStack
+                  ? React.captureOwnerStack()
+                  : null,
+              });
+            },
+          },
+        );
+      }).rejects.toThrow(thrownError);
+
+      expect(caughtErrors.length).toBe(1);
+      expect(caughtErrors[0].error).toBe(thrownError);
+      expect(normalizeCodeLocInfo(caughtErrors[0].parentStack)).toBe(
+        '\n    in Baz (at **)' +
+          '\n    in div (at **)' +
+          '\n    in Bar (at **)' +
+          '\n    in Foo (at **)' +
+          '\n    in div (at **)',
+      );
+      expect(normalizeCodeLocInfo(caughtErrors[0].ownerStack)).toBe(
+        __DEV__ && gate(flags => flags.enableOwnerStacks)
+          ? '\n    in Bar (at **)' + '\n    in Foo (at **)'
+          : null,
+      );
+    });
   });
 }

filename: packages/react-html/src/__tests__/ReactHTMLServer-test.js
patch: @@ -15,6 +15,15 @@ global.TextEncoder = require('util').TextEncoder;
 let React;
 let ReactHTML;
 
+function normalizeCodeLocInfo(str) {
+  return (
+    str &&
+    String(str).replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) {
+      return '\n    in ' + name + ' (at **)';
+    })
+  );
+}
+
 if (!__EXPERIMENTAL__) {
   it('should not be built in stable', () => {
     try {
@@ -200,5 +209,54 @@ if (!__EXPERIMENTAL__) {
       );
       expect(html).toBe('<div>00</div>');
     });
+
+    it('can get the component owner stacks for onError in dev', async () => {
+      const thrownError = new Error('hi');
+      const caughtErrors = [];
+
+      function Foo() {
+        return React.createElement(Bar);
+      }
+      function Bar() {
+        return React.createElement('div', null, React.createElement(Baz));
+      }
+      function Baz({unused}) {
+        throw thrownError;
+      }
+
+      await expect(async () => {
+        await ReactHTML.renderToMarkup(
+          React.createElement('div', null, React.createElement(Foo)),
+          {
+            onError(error, errorInfo) {
+              caughtErrors.push({
+                error: error,
+                parentStack: errorInfo.componentStack,
+                ownerStack: React.captureOwnerStack
+                  ? React.captureOwnerStack()
+                  : null,
+              });
+            },
+          },
+        );
+      }).rejects.toThrow(thrownError);
+
+      expect(caughtErrors.length).toBe(1);
+      expect(caughtErrors[0].error).toBe(thrownError);
+      expect(normalizeCodeLocInfo(caughtErrors[0].parentStack)).toBe(
+        // TODO: Because Fizz doesn't yet implement debugInfo for parent stacks
+        // it doesn't have the Server Components in the parent stacks.
+        '\n    in Lazy (at **)' +
+          '\n    in div (at **)' +
+          '\n    in div (at **)',
+      );
+      expect(normalizeCodeLocInfo(caughtErrors[0].ownerStack)).toBe(
+        __DEV__ && gate(flags => flags.enableOwnerStacks)
+          ? // TODO: Because Fizz doesn't yet implement debugInfo for parent stacks
+            // it doesn't have the Server Components in the parent stacks.
+            '\n    in Lazy (at **)'
+          : null,
+      );
+    });
   });
 }