Add onErrorShell Callback (facebook#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.
zhengjitf · Apr 15, 2022 · a9ed957 · a9ed957
1 parent 5bfe41e
commit a9ed957
Show file tree

Hide file tree

Showing 8 changed files with 137 additions and 96 deletions.
diff --git a/fixtures/fizz-ssr-browser/index.html b/fixtures/fizz-ssr-browser/index.html
@@ -20,29 +20,37 @@ <h1>Fizz Example</h1>
     <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;
         let container = document.getElementById('container');
         container.innerHTML = '';
         container.appendChild(iframe);
       }
+      render();
     </script>
   </body>
 </html>
diff --git 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);

diff --git 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);

diff --git 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 />

diff --git 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

diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js
@@ -32,54 +32,59 @@ type Options = {|
   bootstrapModules?: Array<string>,
   progressiveChunkSize?: number,
   signal?: AbortSignal,
-  onCompleteShell?: () => void,
   onCompleteAll?: () => void,
   onError?: (error: mixed) => void,
 |};
 
 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};