Add onErrorShell Callback (#23247)

This indicates that an error has happened before the shell completed and there's no point in emitting the result of this stream. This is not quite the same as other fatal errors that can happen even after streaming as started. It's also not quite the same as onError before onCompleteShell because onError can be called for an error inside a Suspense boundary before the shell completes. Implement shell error handling in Node SSR fixtures Instead of hanging indefinitely. Update Browser Fixture Expose onErrorShell to the Node build This API is not Promisified so it's just a separate callback instead. Promisify the Browser Fizz API It's now a Promise of a readable stream. The Promise resolves when the shell completes. If the shell errors, the Promise is rejected.
2026-01-15 12:15:22 +00:00 · 2022-02-08 22:38:14 -05:00
parent 0dedfcc681
commit 5690932765
8 changed files with 137 additions and 96 deletions
--- a/fixtures/fizz-ssr-browser/index.html
+++ b/fixtures/fizz-ssr-browser/index.html
@@ -20,22 +20,29 @@
    <script src="../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js"></script>
    <script src="https://unpkg.com/babel-standalone@6/babel.js"></script>
    <script type="text/babel">
-      let controller = new AbortController();
-      let stream = ReactDOMServer.renderToReadableStream(
-        <html>
-          <body>Success</body>
-        </html>,
-        {
-          signal: controller.signal,
+      async function render() {
+        let controller = new AbortController();
+        let response;
+        try {
+          let stream = await ReactDOMServer.renderToReadableStream(
+            <html>
+              <body>Success</body>
+            </html>,
+            {
+              signal: controller.signal,
+            }
+          );
+          response = new Response(stream, {
+            headers: {'Content-Type': 'text/html'},
+          });
+        } catch (x) {
+          response = new Response('<!doctype><p>Error</p>', {
+            status: 500,
+            headers: {'Content-Type': 'text/html'},
+          });
        }
-      );
-      let response = new Response(stream, {
-        headers: {'Content-Type': 'text/html'},
-      });
-      display(response);

-      async function display(responseToDisplay) {
-        let blob = await responseToDisplay.blob();
+        let blob = await response.blob();
        let url = URL.createObjectURL(blob);
        let iframe = document.createElement('iframe');
        iframe.src = url;
@@ -43,6 +50,7 @@
        container.innerHTML = '';
        container.appendChild(iframe);
      }
+      render();
    </script>
  </body>
 </html>
--- a/fixtures/ssr/server/render.js
+++ b/fixtures/ssr/server/render.js
@@ -28,6 +28,11 @@ export default function render(url, res) {
      res.setHeader('Content-type', 'text/html');
      pipe(res);
    },
+    onErrorShell(x) {
+      // Something errored before we could complete the shell so we emit an alternative shell.
+      res.statusCode = 500;
+      res.send('<!doctype><p>Error</p>');
+    },
    onError(x) {
      didError = true;
      console.error(x);
--- a/fixtures/ssr2/server/render.js
+++ b/fixtures/ssr2/server/render.js
@@ -49,6 +49,11 @@ module.exports = function render(url, res) {
        res.setHeader('Content-type', 'text/html');
        pipe(res);
      },
+      onErrorShell(x) {
+        // Something errored before we could complete the shell so we emit an alternative shell.
+        res.statusCode = 500;
+        res.send('<!doctype><p>Error</p>');
+      },
      onError(x) {
        didError = true;
        console.error(x);
--- a/packages/react-dom/src/tests/ReactDOMFizzServerBrowser-test.js
+++ b/packages/react-dom/src/tests/ReactDOMFizzServerBrowser-test.js
@@ -51,7 +51,7 @@ describe('ReactDOMFizzServer', () => {

  // @gate experimental
  it('should call renderToReadableStream', async () => {
-    const stream = ReactDOMFizzServer.renderToReadableStream(
+    const stream = await ReactDOMFizzServer.renderToReadableStream(
      <div>hello world</div>,
    );
    const result = await readResult(stream);
@@ -60,7 +60,7 @@ describe('ReactDOMFizzServer', () => {

  // @gate experimental
  it('should emit DOCTYPE at the root of the document', async () => {
-    const stream = ReactDOMFizzServer.renderToReadableStream(
+    const stream = await ReactDOMFizzServer.renderToReadableStream(
      <html>
        <body>hello world</body>
      </html>,
@@ -73,7 +73,7 @@ describe('ReactDOMFizzServer', () => {

  // @gate experimental
  it('should emit bootstrap script src at the end', async () => {
-    const stream = ReactDOMFizzServer.renderToReadableStream(
+    const stream = await ReactDOMFizzServer.renderToReadableStream(
      <div>hello world</div>,
      {
        bootstrapScriptContent: 'INIT();',
@@ -99,7 +99,7 @@ describe('ReactDOMFizzServer', () => {
      return 'Done';
    }
    let isComplete = false;
-    const stream = ReactDOMFizzServer.renderToReadableStream(
+    const stream = await ReactDOMFizzServer.renderToReadableStream(
      <div>
        <Suspense fallback="Loading">
          <Wait />
@@ -128,63 +128,55 @@ describe('ReactDOMFizzServer', () => {
  });

  // @gate experimental
-  it('should error the stream when an error is thrown at the root', async () => {
+  it('should reject the promise when an error is thrown at the root', async () => {
    const reportedErrors = [];
-    const stream = ReactDOMFizzServer.renderToReadableStream(
-      <div>
-        <Throw />
-      </div>,
-      {
-        onError(x) {
-          reportedErrors.push(x);
-        },
-      },
-    );
-
    let caughtError = null;
-    let result = '';
    try {
-      result = await readResult(stream);
-    } catch (x) {
-      caughtError = x;
+      await ReactDOMFizzServer.renderToReadableStream(
+        <div>
+          <Throw />
+        </div>,
+        {
+          onError(x) {
+            reportedErrors.push(x);
+          },
+        },
+      );
+    } catch (error) {
+      caughtError = error;
    }
    expect(caughtError).toBe(theError);
-    expect(result).toBe('');
    expect(reportedErrors).toEqual([theError]);
  });

  // @gate experimental
-  it('should error the stream when an error is thrown inside a fallback', async () => {
+  it('should reject the promise when an error is thrown inside a fallback', async () => {
    const reportedErrors = [];
-    const stream = ReactDOMFizzServer.renderToReadableStream(
-      <div>
-        <Suspense fallback={<Throw />}>
-          <InfiniteSuspend />
-        </Suspense>
-      </div>,
-      {
-        onError(x) {
-          reportedErrors.push(x);
-        },
-      },
-    );
-
    let caughtError = null;
-    let result = '';
    try {
-      result = await readResult(stream);
-    } catch (x) {
-      caughtError = x;
+      await ReactDOMFizzServer.renderToReadableStream(
+        <div>
+          <Suspense fallback={<Throw />}>
+            <InfiniteSuspend />
+          </Suspense>
+        </div>,
+        {
+          onError(x) {
+            reportedErrors.push(x);
+          },
+        },
+      );
+    } catch (error) {
+      caughtError = error;
    }
    expect(caughtError).toBe(theError);
-    expect(result).toBe('');
    expect(reportedErrors).toEqual([theError]);
  });

  // @gate experimental
  it('should not error the stream when an error is thrown inside suspense boundary', async () => {
    const reportedErrors = [];
-    const stream = ReactDOMFizzServer.renderToReadableStream(
+    const stream = await ReactDOMFizzServer.renderToReadableStream(
      <div>
        <Suspense fallback={<div>Loading</div>}>
          <Throw />
@@ -205,7 +197,7 @@ describe('ReactDOMFizzServer', () => {
  // @gate experimental
  it('should be able to complete by aborting even if the promise never resolves', async () => {
    const controller = new AbortController();
-    const stream = ReactDOMFizzServer.renderToReadableStream(
+    const stream = await ReactDOMFizzServer.renderToReadableStream(
      <div>
        <Suspense fallback={<div>Loading</div>}>
          <InfiniteSuspend />
--- a/packages/react-dom/src/tests/ReactDOMFizzServerNode-test.js
+++ b/packages/react-dom/src/tests/ReactDOMFizzServerNode-test.js
@@ -168,6 +168,7 @@ describe('ReactDOMFizzServer', () => {
  // @gate experimental
  it('should error the stream when an error is thrown at the root', async () => {
    const reportedErrors = [];
+    const reportedShellErrors = [];
    const {writable, output, completed} = getTestWritable();
    const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
      <div>
@@ -178,6 +179,9 @@ describe('ReactDOMFizzServer', () => {
        onError(x) {
          reportedErrors.push(x);
        },
+        onErrorShell(x) {
+          reportedShellErrors.push(x);
+        },
      },
    );

@@ -190,11 +194,13 @@ describe('ReactDOMFizzServer', () => {
    expect(output.result).toBe('');
    // This type of error is reported to the error callback too.
    expect(reportedErrors).toEqual([theError]);
+    expect(reportedShellErrors).toEqual([theError]);
  });

  // @gate experimental
  it('should error the stream when an error is thrown inside a fallback', async () => {
    const reportedErrors = [];
+    const reportedShellErrors = [];
    const {writable, output, completed} = getTestWritable();
    const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
      <div>
@@ -207,6 +213,9 @@ describe('ReactDOMFizzServer', () => {
        onError(x) {
          reportedErrors.push(x);
        },
+        onErrorShell(x) {
+          reportedShellErrors.push(x);
+        },
      },
    );
    pipe(writable);
@@ -216,11 +225,13 @@ describe('ReactDOMFizzServer', () => {
    expect(output.error).toBe(theError);
    expect(output.result).toBe('');
    expect(reportedErrors).toEqual([theError]);
+    expect(reportedShellErrors).toEqual([theError]);
  });

  // @gate experimental
  it('should not error the stream when an error is thrown inside suspense boundary', async () => {
    const reportedErrors = [];
+    const reportedShellErrors = [];
    const {writable, output, completed} = getTestWritable();
    const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
      <div>
@@ -233,6 +244,9 @@ describe('ReactDOMFizzServer', () => {
        onError(x) {
          reportedErrors.push(x);
        },
+        onErrorShell(x) {
+          reportedShellErrors.push(x);
+        },
      },
    );
    pipe(writable);
@@ -243,6 +257,7 @@ describe('ReactDOMFizzServer', () => {
    expect(output.result).toContain('Loading');
    // While no error is reported to the stream, the error is reported to the callback.
    expect(reportedErrors).toEqual([theError]);
+    expect(reportedShellErrors).toEqual([]);
  });

  // @gate experimental
--- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js
+++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js
@@ -32,7 +32,6 @@ type Options = {|
  bootstrapModules?: Array<string>,
  progressiveChunkSize?: number,
  signal?: AbortSignal,
-  onCompleteShell?: () => void,
  onCompleteAll?: () => void,
  onError?: (error: mixed) => void,
 |};
@@ -40,46 +39,52 @@ type Options = {|
 function renderToReadableStream(
  children: ReactNodeList,
  options?: Options,
-): ReadableStream {
-  const request = createRequest(
-    children,
-    createResponseState(
-      options ? options.identifierPrefix : undefined,
-      options ? options.nonce : undefined,
-      options ? options.bootstrapScriptContent : undefined,
-      options ? options.bootstrapScripts : undefined,
-      options ? options.bootstrapModules : undefined,
-    ),
-    createRootFormatContext(options ? options.namespaceURI : undefined),
-    options ? options.progressiveChunkSize : undefined,
-    options ? options.onError : undefined,
-    options ? options.onCompleteAll : undefined,
-    options ? options.onCompleteShell : undefined,
-  );
-  if (options && options.signal) {
-    const signal = options.signal;
-    const listener = () => {
-      abort(request);
-      signal.removeEventListener('abort', listener);
-    };
-    signal.addEventListener('abort', listener);
-  }
-  const stream = new ReadableStream({
-    start(controller) {
-      startWork(request);
-    },
-    pull(controller) {
-      // Pull is called immediately even if the stream is not passed to anything.
-      // That's buffering too early. We want to start buffering once the stream
-      // is actually used by something so we can give it the best result possible
-      // at that point.
-      if (stream.locked) {
-        startFlowing(request, controller);
-      }
-    },
-    cancel(reason) {},
+): Promise<ReadableStream> {
+  return new Promise((resolve, reject) => {
+    function onCompleteShell() {
+      const stream = new ReadableStream({
+        pull(controller) {
+          // Pull is called immediately even if the stream is not passed to anything.
+          // That's buffering too early. We want to start buffering once the stream
+          // is actually used by something so we can give it the best result possible
+          // at that point.
+          if (stream.locked) {
+            startFlowing(request, controller);
+          }
+        },
+        cancel(reason) {},
+      });
+      resolve(stream);
+    }
+    function onErrorShell(error: mixed) {
+      reject(error);
+    }
+    const request = createRequest(
+      children,
+      createResponseState(
+        options ? options.identifierPrefix : undefined,
+        options ? options.nonce : undefined,
+        options ? options.bootstrapScriptContent : undefined,
+        options ? options.bootstrapScripts : undefined,
+        options ? options.bootstrapModules : undefined,
+      ),
+      createRootFormatContext(options ? options.namespaceURI : undefined),
+      options ? options.progressiveChunkSize : undefined,
+      options ? options.onError : undefined,
+      options ? options.onCompleteAll : undefined,
+      onCompleteShell,
+      onErrorShell,
+    );
+    if (options && options.signal) {
+      const signal = options.signal;
+      const listener = () => {
+        abort(request);
+        signal.removeEventListener('abort', listener);
+      };
+      signal.addEventListener('abort', listener);
+    }
+    startWork(request);
  });
-  return stream;
 }

 export {renderToReadableStream, ReactVersion as version};
--- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js
+++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js
@@ -37,6 +37,7 @@ type Options = {|
  bootstrapModules?: Array<string>,
  progressiveChunkSize?: number,
  onCompleteShell?: () => void,
+  onErrorShell?: () => void,
  onCompleteAll?: () => void,
  onError?: (error: mixed) => void,
 |};
@@ -63,6 +64,7 @@ function createRequestImpl(children: ReactNodeList, options: void | Options) {
    options ? options.onError : undefined,
    options ? options.onCompleteAll : undefined,
    options ? options.onCompleteShell : undefined,
+    options ? options.onErrorShell : undefined,
  );
 }

--- a/packages/react-server/src/ReactFizzServer.js
+++ b/packages/react-server/src/ReactFizzServer.js
@@ -199,6 +199,9 @@ export opaque type Request = {
  // Typically you don't need this callback because it's best practice to always have a
  // root fallback ready so there's no need to wait.
  onCompleteShell: () => void,
+  // onErrorShell is called when the shell didn't complete. That means you probably want to
+  // emit a different response to the stream instead.
+  onErrorShell: (error: mixed) => void,
 };

 // This is a default heuristic for how to split up the HTML content into progressive
@@ -232,6 +235,7 @@ export function createRequest(
  onError: void | ((error: mixed) => void),
  onCompleteAll: void | (() => void),
  onCompleteShell: void | (() => void),
+  onErrorShell: void | ((error: mixed) => void),
 ): Request {
  const pingedTasks = [];
  const abortSet: Set<Task> = new Set();
@@ -256,6 +260,7 @@ export function createRequest(
    onError: onError === undefined ? defaultErrorHandler : onError,
    onCompleteAll: onCompleteAll === undefined ? noop : onCompleteAll,
    onCompleteShell: onCompleteShell === undefined ? noop : onCompleteShell,
+    onErrorShell: onErrorShell === undefined ? noop : onErrorShell,
  };
  // This segment represents the root fallback.
  const rootSegment = createPendingSegment(request, 0, null, rootFormatContext);
@@ -412,6 +417,8 @@ function fatalError(request: Request, error: mixed): void {
  // This is called outside error handling code such as if the root errors outside
  // a suspense boundary or if the root suspense boundary's fallback errors.
  // It's also called if React itself or its host configs errors.
+  const onErrorShell = request.onErrorShell;
+  onErrorShell(error);
  if (request.destination !== null) {
    request.status = CLOSED;
    closeWithError(request.destination, error);
@@ -1433,6 +1440,8 @@ function finishedTask(
    }
    request.pendingRootTasks--;
    if (request.pendingRootTasks === 0) {
+      // We have completed the shell so the shell can't error anymore.
+      request.onErrorShell = noop;
      const onCompleteShell = request.onCompleteShell;
      onCompleteShell();
    }