[Flight][Static] When prerendering serialize infinite promise when ab…

…orting with no reason When prerendering if you abort the prerender without a reason instead of erroring each remaining task complete it with a promise that never resolve Unfortunately when you abort with an AbortSignal without a value the aborted reason is defaulted to an AbortError DOMexception. We test for this and basically just say that if you abort with an AbortError DOMException that is equivalent to aborting with nothing. Practically this should be fine because usually you abort with a specific reason.
facebook · Aug 15, 2024 · f953493 · f953493
1 parent 0a55ade
commit f953493
Show file tree

Hide file tree

Showing 9 changed files with 247 additions and 25 deletions.
diff --git a/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js
@@ -26,6 +26,8 @@ import {
   startFlowing,
   stopFlowing,
   abort,
+  suspend,
+  isDefaultAbortError,
 } from 'react-server/src/ReactFlightServer';
 
 import {
@@ -187,10 +189,20 @@ function prerenderToNodeStream(
     if (options && options.signal) {
       const signal = options.signal;
       if (signal.aborted) {
-        abort(request, (signal: any).reason);
+        const reason = (signal: any).reason;
+        if (isDefaultAbortError(reason)) {
+          suspend(request);
+        } else {
+          abort(request, reason);
+        }
       } else {
         const listener = () => {
-          abort(request, (signal: any).reason);
+          const reason = (signal: any).reason;
+          if (isDefaultAbortError(reason)) {
+            suspend(request);
+          } else {
+            abort(request, reason);
+          }
           signal.removeEventListener('abort', listener);
         };
         signal.addEventListener('abort', listener);

diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js
@@ -18,6 +18,8 @@ import {
   startFlowing,
   stopFlowing,
   abort,
+  suspend,
+  isDefaultAbortError,
 } from 'react-server/src/ReactFlightServer';
 
 import {
@@ -146,10 +148,20 @@ function prerender(
     if (options && options.signal) {
       const signal = options.signal;
       if (signal.aborted) {
-        abort(request, (signal: any).reason);
+        const reason = (signal: any).reason;
+        if (isDefaultAbortError(reason)) {
+          suspend(request);
+        } else {
+          abort(request, reason);
+        }
       } else {
         const listener = () => {
-          abort(request, (signal: any).reason);
+          const reason = (signal: any).reason;
+          if (isDefaultAbortError(reason)) {
+            suspend(request);
+          } else {
+            abort(request, reason);
+          }
           signal.removeEventListener('abort', listener);
         };
         signal.addEventListener('abort', listener);

diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js
@@ -18,6 +18,8 @@ import {
   startFlowing,
   stopFlowing,
   abort,
+  suspend,
+  isDefaultAbortError,
 } from 'react-server/src/ReactFlightServer';
 
 import {
@@ -146,10 +148,20 @@ function prerender(
     if (options && options.signal) {
       const signal = options.signal;
       if (signal.aborted) {
-        abort(request, (signal: any).reason);
+        const reason = (signal: any).reason;
+        if (isDefaultAbortError(reason)) {
+          suspend(request);
+        } else {
+          abort(request, reason);
+        }
       } else {
         const listener = () => {
-          abort(request, (signal: any).reason);
+          const reason = (signal: any).reason;
+          if (isDefaultAbortError(reason)) {
+            suspend(request);
+          } else {
+            abort(request, reason);
+          }
           signal.removeEventListener('abort', listener);
         };
         signal.addEventListener('abort', listener);

diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js
@@ -26,6 +26,8 @@ import {
   startFlowing,
   stopFlowing,
   abort,
+  suspend,
+  isDefaultAbortError,
 } from 'react-server/src/ReactFlightServer';
 
 import {
@@ -189,10 +191,20 @@ function prerenderToNodeStream(
     if (options && options.signal) {
       const signal = options.signal;
       if (signal.aborted) {
-        abort(request, (signal: any).reason);
+        const reason = (signal: any).reason;
+        if (isDefaultAbortError(reason)) {
+          suspend(request);
+        } else {
+          abort(request, reason);
+        }
       } else {
         const listener = () => {
-          abort(request, (signal: any).reason);
+          const reason = (signal: any).reason;
+          if (isDefaultAbortError(reason)) {
+            suspend(request);
+          } else {
+            abort(request, reason);
+          }
           signal.removeEventListener('abort', listener);
         };
         signal.addEventListener('abort', listener);

diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js
@@ -2722,4 +2722,83 @@ describe('ReactFlightDOM', () => {
     await readInto(container, fizzReadable);
     expect(getMeaningfulChildren(container)).toEqual(<div>hello world</div>);
   });
+
+  // @gate experimental
+  it('serializes unfinished tasks with infinite promises when aborting a prerender without a reason', async () => {
+    let resolveGreeting;
+    const greetingPromise = new Promise(resolve => {
+      resolveGreeting = resolve;
+    });
+
+    function App() {
+      return (
+        <div>
+          <Suspense fallback="loading...">
+            <Greeting />
+          </Suspense>
+        </div>
+      );
+    }
+
+    async function Greeting() {
+      await greetingPromise;
+      return 'hello world';
+    }
+
+    const controller = new AbortController();
+    const {pendingResult} = await serverAct(async () => {
+      // destructure trick to avoid the act scope from awaiting the returned value
+      return {
+        pendingResult: ReactServerDOMStaticServer.prerenderToNodeStream(
+          <App />,
+          webpackMap,
+          {
+            signal: controller.signal,
+          },
+        ),
+      };
+    });
+
+    controller.abort();
+    resolveGreeting();
+    const {prelude} = await pendingResult;
+
+    const preludeWeb = Readable.toWeb(prelude);
+    const response = ReactServerDOMClient.createFromReadableStream(preludeWeb);
+
+    const {writable: fizzWritable, readable: fizzReadable} = getTestStream();
+
+    function ClientApp() {
+      return use(response);
+    }
+
+    const shellErrors = [];
+    let abortFizz;
+    await serverAct(async () => {
+      const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream(
+        React.createElement(ClientApp),
+        {
+          onShellError(error) {
+            shellErrors.push(error.message);
+          },
+        },
+      );
+      pipe(fizzWritable);
+      abortFizz = abort;
+    });
+
+    await serverAct(() => {
+      try {
+        React.unstable_postpone('abort reason');
+      } catch (reason) {
+        abortFizz(reason);
+      }
+    });
+
+    expect(shellErrors).toEqual([]);
+
+    const container = document.createElement('div');
+    await readInto(container, fizzReadable);
+    expect(getMeaningfulChildren(container)).toEqual(<div>loading...</div>);
+  });
 });
diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js
@@ -18,6 +18,8 @@ import {
   startFlowing,
   stopFlowing,
   abort,
+  suspend,
+  isDefaultAbortError,
 } from 'react-server/src/ReactFlightServer';
 
 import {
@@ -146,10 +148,20 @@ function prerender(
     if (options && options.signal) {
       const signal = options.signal;
       if (signal.aborted) {
-        abort(request, (signal: any).reason);
+        const reason = (signal: any).reason;
+        if (isDefaultAbortError(reason)) {
+          suspend(request);
+        } else {
+          abort(request, reason);
+        }
       } else {
         const listener = () => {
-          abort(request, (signal: any).reason);
+          const reason = (signal: any).reason;
+          if (isDefaultAbortError(reason)) {
+            suspend(request);
+          } else {
+            abort(request, reason);
+          }
           signal.removeEventListener('abort', listener);
         };
         signal.addEventListener('abort', listener);

diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js
@@ -18,6 +18,8 @@ import {
   startFlowing,
   stopFlowing,
   abort,
+  suspend,
+  isDefaultAbortError,
 } from 'react-server/src/ReactFlightServer';
 
 import {
@@ -146,10 +148,20 @@ function prerender(
     if (options && options.signal) {
       const signal = options.signal;
       if (signal.aborted) {
-        abort(request, (signal: any).reason);
+        const reason = (signal: any).reason;
+        if (isDefaultAbortError(reason)) {
+          suspend(request);
+        } else {
+          abort(request, reason);
+        }
       } else {
         const listener = () => {
-          abort(request, (signal: any).reason);
+          const reason = (signal: any).reason;
+          if (isDefaultAbortError(reason)) {
+            suspend(request);
+          } else {
+            abort(request, reason);
+          }
           signal.removeEventListener('abort', listener);
         };
         signal.addEventListener('abort', listener);

diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js
@@ -26,6 +26,8 @@ import {
   startFlowing,
   stopFlowing,
   abort,
+  suspend,
+  isDefaultAbortError,
 } from 'react-server/src/ReactFlightServer';
 
 import {
@@ -189,10 +191,20 @@ function prerenderToNodeStream(
     if (options && options.signal) {
       const signal = options.signal;
       if (signal.aborted) {
-        abort(request, (signal: any).reason);
+        const reason = (signal: any).reason;
+        if (isDefaultAbortError(reason)) {
+          suspend(request);
+        } else {
+          abort(request, reason);
+        }
       } else {
         const listener = () => {
-          abort(request, (signal: any).reason);
+          const reason = (signal: any).reason;
+          if (isDefaultAbortError(reason)) {
+            suspend(request);
+          } else {
+            abort(request, reason);
+          }
           signal.removeEventListener('abort', listener);
         };
         signal.addEventListener('abort', listener);