Properly handle empty body response codes in single fetch requests (#12760)

Author: brophdawg11

.changeset/fifty-eagles-love.md

@@ -0,0 +1,5 @@
+---
+"react-router": patch
+---
+
+Properly handle status codes that cannot have a body in single fetch responses (204, etc.)

integration/single-fetch-test.ts

@@ -1700,6 +1700,69 @@ test.describe("single-fetch", () => {
     ]);
   });
 
+  test("does not try to encode a turbo-stream body into 204 responses", async ({
+    page,
+  }) => {
+    let fixture = await createFixture({
+      files: {
+        ...files,
+        "app/routes/_index.tsx": js`
+          import { data, Form, useActionData, useNavigation } from "react-router";
+
+          export async function action({ request }) {
+            await new Promise(r => setTimeout(r, 500));
+            return data(null, { status: 204 });
+          };
+
+          export default function Index() {
+            const navigation = useNavigation();
+            const actionData = useActionData();
+            return (
+              <Form method="post">
+                {navigation.state === "idle" ? <p data-idle>idle</p> : <p data-active>active</p>}
+                <button data-submit type="submit">{actionData ?? 'no content!'}</button>
+              </Form>
+            );
+          }
+        `,
+      },
+    });
+    let appFixture = await createAppFixture(fixture);
+
+    let app = new PlaywrightFixture(appFixture, page);
+
+    let requests: [string, number, string][] = [];
+    page.on("request", async (req) => {
+      if (req.url().includes(".data")) {
+        let url = new URL(req.url());
+        requests.push([
+          req.method(),
+          (await req.response())!.status(),
+          url.pathname + url.search,
+        ]);
+      }
+    });
+
+    // Document requests
+    let documentRes = await fixture.requestDocument("/?index", {
+      method: "post",
+    });
+    expect(documentRes.status).toBe(204);
+    expect(await documentRes.text()).toBe("");
+
+    // Data requests
+    await app.goto("/");
+    (await page.$("[data-submit]"))?.click();
+    await page.waitForSelector("[data-active]");
+    await page.waitForSelector("[data-idle]");
+
+    expect(await page.innerText("[data-submit]")).toEqual("no content!");
+    expect(requests).toEqual([
+      ["POST", 204, "/_root.data?index"],
+      ["GET", 200, "/_root.data"],
+    ]);
+  });
+
   test("does not try to encode a turbo-stream body into 304 responses", async () => {
     let fixture = await createFixture({
       files: {

packages/react-router/lib/dom/ssr/single-fetch.tsx

@@ -414,7 +414,10 @@ export function singleFetchUrl(reqUrl: URL | string) {
   return url;
 }
 
-async function fetchAndDecode(url: URL, init: RequestInit) {
+async function fetchAndDecode(
+  url: URL,
+  init: RequestInit
+): Promise<{ status: number; data: unknown }> {
   let res = await fetch(url, init);
 
   // If this 404'd without hitting the running server (most likely in a
@@ -423,6 +426,22 @@ async function fetchAndDecode(url: URL, init: RequestInit) {
     throw new ErrorResponseImpl(404, "Not Found", true);
   }
 
+  // some status codes are not permitted to have bodies, so we want to just
+  // treat those as "no data" instead of throwing an exception.
+  // 304 is not included here because the browser should fill those responses
+  // with the cached body content.
+  const NO_BODY_STATUS_CODES = new Set([100, 101, 204, 205]);
+  if (NO_BODY_STATUS_CODES.has(res.status)) {
+    if (!init.method || init.method === "GET") {
+      // SingleFetchResults can just have no routeId keys which will result
+      // in no data for all routes
+      return { status: res.status, data: {} };
+    } else {
+      // SingleFetchResult is for a singular route and can specify no data
+      return { status: res.status, data: { data: undefined } };
+    }
+  }
+
   invariant(res.body, "No response body to decode");
 
   try {

packages/react-router/lib/server-runtime/server.ts

@@ -42,6 +42,17 @@ export type CreateRequestHandlerFunction = (
   mode?: string
 ) => RequestHandler;
 
+// Do not include a response body if the status code is one of these,
+// otherwise `undici` will throw an error when constructing the Response:
+//   https://github.com/nodejs/undici/blob/bd98a6303e45d5e0d44192a93731b1defdb415f3/lib/web/fetch/response.js#L522-L528
+//
+// Specs:
+//   https://datatracker.ietf.org/doc/html/rfc9110#name-informational-1xx
+//   https://datatracker.ietf.org/doc/html/rfc9110#name-204-no-content
+//   https://datatracker.ietf.org/doc/html/rfc9110#name-205-reset-content
+//   https://datatracker.ietf.org/doc/html/rfc9110#name-304-not-modified
+const NO_BODY_STATUS_CODES = new Set([100, 101, 204, 205, 304]);
+
 function derive(build: ServerBuild, mode?: string) {
   let routes = createRoutes(build.routes);
   let dataRoutes = createStaticHandlerDataRoutes(build.routes, build.future);
@@ -297,9 +308,9 @@ async function handleSingleFetchRequest(
   let resultHeaders = new Headers(headers);
   resultHeaders.set("X-Remix-Response", "yes");
 
-  // 304 responses should not have a body
-  if (status === 304) {
-    return new Response(null, { status: 304, headers: resultHeaders });
+  // Skip response body for unsupported status codes
+  if (NO_BODY_STATUS_CODES.has(status)) {
+    return new Response(null, { status, headers: resultHeaders });
   }
 
   // We use a less-descriptive `text/x-script` here instead of something like
@@ -347,9 +358,9 @@ async function handleDocumentRequest(
 
   let headers = getDocumentHeaders(build, context);
 
-  // 304 responses should not have a body or a content-type
-  if (context.statusCode === 304) {
-    return new Response(null, { status: 304, headers });
+  // Skip response body for unsupported status codes
+  if (NO_BODY_STATUS_CODES.has(context.statusCode)) {
+    return new Response(null, { status: context.statusCode, headers });
   }
 
   // Sanitize errors outside of development environments