Avoid decoding CDN errors that never reached the origin server (#14385)

Author: brophdawg11

.changeset/happy-hornets-judge.md

@@ -0,0 +1,7 @@
+---
+"react-router": patch
+---
+
+Do not try to use `turbo-stream` to decode CDN errors that never reached the server
+
+- We used to do this but lost this check with the adoption of single fetch

integration/error-boundary-v2-test.ts

@@ -177,8 +177,8 @@ test.describe("ErrorBoundary", () => {
       await waitForAndAssert(
         page,
         app,
-        "#parent-error",
-        "Unable to decode turbo-stream response",
+        "#parent-error-response",
+        "500 CDN Error!",
       );
     });
   });

integration/error-data-request-test.ts

@@ -110,7 +110,7 @@ test.describe("ErrorBoundary", () => {
     let { status, headers, data } =
       await fixture.requestSingleFetchData("/_root.data");
     expect(status).toBe(200);
-    expect(headers.has("X-Remix-Error")).toBe(false);
+    expect(headers.has("X-Remix-Response")).toBe(true);
     expect(data).toEqual({});
   });
 
@@ -122,7 +122,7 @@ test.describe("ErrorBoundary", () => {
       },
     );
     expect(status).toBe(405);
-    expect(headers.has("X-Remix-Error")).toBe(false);
+    expect(headers.has("X-Remix-Response")).toBe(true);
     expect(data).toEqual({
       error: new ErrorResponseImpl(
         405,
@@ -153,7 +153,7 @@ test.describe("ErrorBoundary", () => {
       "/i/match/nothing.data",
     );
     expect(status).toBe(404);
-    expect(headers.has("X-Remix-Error")).toBe(false);
+    expect(headers.has("X-Remix-Response")).toBe(true);
     expect(data).toEqual({
       root: {
         error: new ErrorResponseImpl(

packages/react-router/__tests__/server-runtime/server-test.ts

@@ -555,7 +555,7 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(400);
-      expect(result.headers.get("X-Remix-Error")).toBe("yes");
+      expect(headers.has("X-Remix-Response")).toBe(true);
       expect((await result.json()).message).toBeTruthy();
     });
 
@@ -585,7 +585,7 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(403);
-      expect(result.headers.get("X-Remix-Error")).toBe("yes");
+      expect(headers.has("X-Remix-Response")).toBe(true);
       expect((await result.json()).message).toBeTruthy();
     });
 
@@ -608,7 +608,7 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(404);
-      expect(result.headers.get("X-Remix-Error")).toBe("yes");
+      expect(headers.has("X-Remix-Response")).toBe(true);
       expect((await result.json()).message).toBeTruthy();
     });
 
@@ -670,7 +670,7 @@ describe("shared server runtime", () => {
       let result = await handler(request);
       expect(result.status).toBe(500);
       expect((await result.json()).message).toBe("Unexpected Server Error");
-      expect(result.headers.get("X-Remix-Error")).toBe("yes");
+      expect(headers.has("X-Remix-Response")).toBe(true);
       expect(rootLoader.mock.calls.length).toBe(1);
       expect(testAction.mock.calls.length).toBe(0);
     });
@@ -704,7 +704,7 @@ describe("shared server runtime", () => {
       let result = await handler(request);
       expect(result.status).toBe(500);
       expect((await result.json()).message).toBe(message);
-      expect(result.headers.get("X-Remix-Error")).toBe("yes");
+      expect(headers.has("X-Remix-Response")).toBe(true);
       expect(rootLoader.mock.calls.length).toBe(1);
       expect(testAction.mock.calls.length).toBe(0);
       expect(spy.console.mock.calls.length).toBe(1);
@@ -737,7 +737,6 @@ describe("shared server runtime", () => {
       let result = await handler(request);
       expect(result.status).toBe(400);
       expect(await result.text()).toBe("test");
-      expect(result.headers.get("X-Remix-Catch")).toBe("yes");
       expect(rootLoader.mock.calls.length).toBe(1);
       expect(testAction.mock.calls.length).toBe(0);
     });
@@ -800,7 +799,7 @@ describe("shared server runtime", () => {
       let result = await handler(request);
       expect(result.status).toBe(500);
       expect((await result.json()).message).toBe("Unexpected Server Error");
-      expect(result.headers.get("X-Remix-Error")).toBe("yes");
+      expect(headers.has("X-Remix-Response")).toBe(true);
       expect(rootLoader.mock.calls.length).toBe(0);
       expect(testAction.mock.calls.length).toBe(1);
     });
@@ -834,7 +833,7 @@ describe("shared server runtime", () => {
       let result = await handler(request);
       expect(result.status).toBe(500);
       expect((await result.json()).message).toBe(message);
-      expect(result.headers.get("X-Remix-Error")).toBe("yes");
+      expect(headers.has("X-Remix-Response")).toBe(true);
       expect(rootLoader.mock.calls.length).toBe(0);
       expect(testAction.mock.calls.length).toBe(1);
       expect(spy.console.mock.calls.length).toBe(1);
@@ -867,7 +866,6 @@ describe("shared server runtime", () => {
       let result = await handler(request);
       expect(result.status).toBe(400);
       expect(await result.text()).toBe("test");
-      expect(result.headers.get("X-Remix-Catch")).toBe("yes");
       expect(rootLoader.mock.calls.length).toBe(0);
       expect(testAction.mock.calls.length).toBe(1);
     });

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

@@ -610,10 +610,14 @@ async function fetchAndDecodeViaTurboStream(
 
   let res = await fetch(url, await createRequestInit(request));
 
-  // If this 404'd without hitting the running server (most likely in a
-  // pre-rendered app using a CDN), then bubble a standard 404 ErrorResponse
-  if (res.status === 404 && !res.headers.has("X-Remix-Response")) {
-    throw new ErrorResponseImpl(404, "Not Found", true);
+  // If this error'd without hitting the running server, then bubble a normal
+  // `ErrorResponse` and don't try to decode the body with `turbo-stream`.
+  //
+  // This could be triggered by a few scenarios:
+  // - `.data` request 404 on a pre-rendered app using a CDN
+  // - 429 error returned from a CDN on a SSR app
+  if (res.status >= 400 && !res.headers.has("X-Remix-Response")) {
+    throw new ErrorResponseImpl(res.status, res.statusText, await res.text());
   }
 
   // Handle non-RR redirects (i.e., from express middleware)

packages/react-router/lib/rsc/browser.tsx

@@ -530,10 +530,14 @@ function getFetchAndDecodeViaRSC(
       new Request(url, await createRequestInit(request)),
     );
 
-    // If this 404'd without hitting the running server (most likely in a
-    // pre-rendered app using a CDN), then bubble a standard 404 ErrorResponse
-    if (res.status === 404 && !res.headers.has("X-Remix-Response")) {
-      throw new ErrorResponseImpl(404, "Not Found", true);
+    // If this error'd without hitting the running server, then bubble a normal
+    // `ErrorResponse` and don't try to decode the body with `turbo-stream`.
+    //
+    // This could be triggered by a few scenarios:
+    // - `.data` request 404 on a pre-rendered app using a CDN
+    // - 429 error returned from a CDN on a SSR app
+    if (res.status >= 400 && !res.headers.has("X-Remix-Response")) {
+      throw new ErrorResponseImpl(res.status, res.statusText, await res.text());
     }
 
     invariant(res.body, "No response body to decode");

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

@@ -455,8 +455,8 @@ export async function matchRSCServerRequest({
     generateResponse,
     temporaryReferences,
   );
-  // The front end uses this to know whether a 404 status came from app code
-  // or 404'd and never reached the origin server
+  // The front end uses this to know whether a 4xx/5xx status came from app code
+  // or never reached the origin server
   response.headers.set("X-Remix-Response", "yes");
   return response;
 }

packages/react-router/lib/rsc/server.ssr.tsx

@@ -196,7 +196,7 @@ export async function routeRSCServerRequest({
       headers.delete("Content-Encoding");
       headers.delete("Content-Length");
       headers.delete("Content-Type");
-      headers.delete("x-remix-response");
+      headers.delete("X-Remix-Response");
       headers.set("Location", payload.location);
 
       return new Response(serverResponseB?.body || "", {

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

@@ -661,9 +661,6 @@ async function handleResourceRequest(
 
   function handleQueryRouteError(error: unknown) {
     if (isResponse(error)) {
-      // Note: Not functionally required but ensures that our response headers
-      // match identically to what Remix returns
-      error.headers.set("X-Remix-Catch", "yes");
       return error;
     }
 
@@ -701,9 +698,6 @@ function errorResponseToJson(
     {
       status: errorResponse.status,
       statusText: errorResponse.statusText,
-      headers: {
-        "X-Remix-Error": "yes",
-      },
     },
   );
 }