fix abort behavior for renderToReadableStream

.changeset/short-maps-reflect.md

@@ -0,0 +1,10 @@
+---
+"@remix-run/dev": patch
+---
+
+Stop passing `request.signal` as the `renderToReadableStream` `signal` to abort server rendering for cloudflare/deno runtimes because by the time that `request` is aborted, aborting the rendering is useless because there's no way for React to flush down the unresolved boundaries
+
+- This has been incorrect for some time, but only recently exposed due to a bug in how we were aborting requests when running via `remix vite:dev` because we were incorrectly aborting requests after successful renders - which was causing us to abort a completed React render, and try to close an already closed `ReadableStream`.
+- This has likely not shown up in any production scenarios because cloudflare/deno production runtimes are (correctly) not aborting the `request.signal` on successful renders
+- The built-in `entry.server` files no longer pass a `signal` to `renderToReadableStream` because adding a timeout-based abort signal to the default behavior would constitute a breaking change
+- Users can configure this abort behavior via their own `entry.server` via `remix reveal entry.server`, and the template entry.server files have been updated with an example approach for newly created Remix apps

packages/remix-dev/config/defaults/entry.server.cloudflare.tsx

@@ -13,7 +13,9 @@ export default async function handleRequest(
   const body = await renderToReadableStream(
     <RemixServer context={remixContext} url={request.url} />,
     {
-      signal: request.signal,
+      // If you wish to abort the rendering process, you can pass a signal here.
+      // Please refer to the templates for example son how to configure this.
+      // signal: controller.signal,
       onError(error: unknown) {
         // Log streaming rendering errors from inside the shell
         console.error(error);

packages/remix-dev/config/defaults/entry.server.deno.tsx

@@ -13,7 +13,9 @@ export default async function handleRequest(
   const body = await renderToReadableStream(
     <RemixServer context={remixContext} url={request.url} />,
     {
-      signal: request.signal,
+      // If you wish to abort the rendering process, you can pass a signal here.
+      // Please refer to the templates for example son how to configure this.
+      // signal: controller.signal,
       onError(error: unknown) {
         // Log streaming rendering errors from inside the shell
         console.error(error);

templates/classic-remix-compiler/cloudflare-pages/app/entry.server.tsx

@@ -9,6 +9,8 @@ import { RemixServer } from "@remix-run/react";
 import { isbot } from "isbot";
 import { renderToReadableStream } from "react-dom/server";
 
+const ABORT_DELAY = 5000;
+
 export default async function handleRequest(
   request: Request,
   responseStatusCode: number,
@@ -19,18 +21,29 @@ export default async function handleRequest(
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   loadContext: AppLoadContext
 ) {
+  const controller = new AbortController();
+  const timeoutId = setTimeout(() => controller.abort(), ABORT_DELAY);
+
   const body = await renderToReadableStream(
-    <RemixServer context={remixContext} url={request.url} />,
+    <RemixServer
+      context={remixContext}
+      url={request.url}
+      abortDelay={ABORT_DELAY}
+    />,
     {
-      signal: request.signal,
+      signal: controller.signal,
       onError(error: unknown) {
-        // Log streaming rendering errors from inside the shell
-        console.error(error);
+        if (!controller.signal.aborted) {
+          // Log streaming rendering errors from inside the shell
+          console.error(error);
+        }
         responseStatusCode = 500;
       },
     }
   );
 
+  body.allReady.then(() => clearTimeout(timeoutId));
+
   if (isbot(request.headers.get("user-agent") || "")) {
     await body.allReady;
   }

templates/classic-remix-compiler/cloudflare-workers/app/entry.server.tsx

@@ -9,6 +9,8 @@ import { RemixServer } from "@remix-run/react";
 import { isbot } from "isbot";
 import { renderToReadableStream } from "react-dom/server";
 
+const ABORT_DELAY = 5000;
+
 export default async function handleRequest(
   request: Request,
   responseStatusCode: number,
@@ -19,18 +21,29 @@ export default async function handleRequest(
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   loadContext: AppLoadContext
 ) {
+  const controller = new AbortController();
+  const timeoutId = setTimeout(() => controller.abort(), ABORT_DELAY);
+
   const body = await renderToReadableStream(
-    <RemixServer context={remixContext} url={request.url} />,
+    <RemixServer
+      context={remixContext}
+      url={request.url}
+      abortDelay={ABORT_DELAY}
+    />,
     {
-      signal: request.signal,
+      signal: controller.signal,
       onError(error: unknown) {
-        // Log streaming rendering errors from inside the shell
-        console.error(error);
+        if (!controller.signal.aborted) {
+          // Log streaming rendering errors from inside the shell
+          console.error(error);
+        }
         responseStatusCode = 500;
       },
     }
   );
 
+  body.allReady.then(() => clearTimeout(timeoutId));
+
   if (isbot(request.headers.get("user-agent") || "")) {
     await body.allReady;
   }

templates/classic-remix-compiler/deno/app/entry.server.tsx

@@ -9,28 +9,40 @@ import { RemixServer } from "@remix-run/react";
 import { isbot } from "isbot";
 import { renderToReadableStream } from "react-dom/server";
 
+const ABORT_DELAY = 5000;
+
 export default async function handleRequest(
   request: Request,
   responseStatusCode: number,
   responseHeaders: Headers,
   remixContext: EntryContext,
   // This is ignored so we can keep it in the template for visibility.  Feel
   // free to delete this parameter in your app if you're not using it!
-
   loadContext: AppLoadContext
 ) {
+  const controller = new AbortController();
+  const timeoutId = setTimeout(() => controller.abort(), ABORT_DELAY);
+
   const body = await renderToReadableStream(
-    <RemixServer context={remixContext} url={request.url} />,
+    <RemixServer
+      context={remixContext}
+      url={request.url}
+      abortDelay={ABORT_DELAY}
+    />,
     {
-      signal: request.signal,
+      signal: controller.signal,
       onError(error: unknown) {
-        // Log streaming rendering errors from inside the shell
-        console.error(error);
+        if (!controller.signal.aborted) {
+          // Log streaming rendering errors from inside the shell
+          console.error(error);
+        }
         responseStatusCode = 500;
       },
     }
   );
 
+  body.allReady.then(() => clearTimeout(timeoutId));
+
   if (isbot(request.headers.get("user-agent") || "")) {
     await body.allReady;
   }

templates/cloudflare-workers/app/entry.server.tsx

@@ -9,6 +9,8 @@ import { RemixServer } from "@remix-run/react";
 import { isbot } from "isbot";
 import { renderToReadableStream } from "react-dom/server";
 
+const ABORT_DELAY = 5000;
+
 export default async function handleRequest(
   request: Request,
   responseStatusCode: number,
@@ -19,18 +21,29 @@ export default async function handleRequest(
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   loadContext: AppLoadContext
 ) {
+  const controller = new AbortController();
+  const timeoutId = setTimeout(() => controller.abort(), ABORT_DELAY);
+
   const body = await renderToReadableStream(
-    <RemixServer context={remixContext} url={request.url} />,
+    <RemixServer
+      context={remixContext}
+      url={request.url}
+      abortDelay={ABORT_DELAY}
+    />,
     {
-      signal: request.signal,
+      signal: controller.signal,
       onError(error: unknown) {
-        // Log streaming rendering errors from inside the shell
-        console.error(error);
+        if (!controller.signal.aborted) {
+          // Log streaming rendering errors from inside the shell
+          console.error(error);
+        }
         responseStatusCode = 500;
       },
     }
   );
 
+  body.allReady.then(() => clearTimeout(timeoutId));
+
   if (isbot(request.headers.get("user-agent") || "")) {
     await body.allReady;
   }

templates/cloudflare/app/entry.server.tsx

@@ -9,6 +9,8 @@ import { RemixServer } from "@remix-run/react";
 import { isbot } from "isbot";
 import { renderToReadableStream } from "react-dom/server";
 
+const ABORT_DELAY = 5000;
+
 export default async function handleRequest(
   request: Request,
   responseStatusCode: number,
@@ -19,18 +21,29 @@ export default async function handleRequest(
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   loadContext: AppLoadContext
 ) {
+  const controller = new AbortController();
+  const timeoutId = setTimeout(() => controller.abort(), ABORT_DELAY);
+
   const body = await renderToReadableStream(
-    <RemixServer context={remixContext} url={request.url} />,
+    <RemixServer
+      context={remixContext}
+      url={request.url}
+      abortDelay={ABORT_DELAY}
+    />,
     {
-      signal: request.signal,
+      signal: controller.signal,
       onError(error: unknown) {
-        // Log streaming rendering errors from inside the shell
-        console.error(error);
+        if (!controller.signal.aborted) {
+          // Log streaming rendering errors from inside the shell
+          console.error(error);
+        }
         responseStatusCode = 500;
       },
     }
   );
 
+  body.allReady.then(() => clearTimeout(timeoutId));
+
   if (isbot(request.headers.get("user-agent") || "")) {
     await body.allReady;
   }