From 4af5e693503d10639b67f7e5118272b8d93c8b5f Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Sep 2025 10:42:04 -0400 Subject: [PATCH] Avoid decoding CDN errors that never reached the origin server --- .changeset/happy-hornets-judge.md | 7 +++++++ integration/error-boundary-v2-test.ts | 4 ++-- integration/error-data-request-test.ts | 6 +++--- .../__tests__/server-runtime/server-test.ts | 16 +++++++--------- .../react-router/lib/dom/ssr/single-fetch.tsx | 12 ++++++++---- packages/react-router/lib/rsc/browser.tsx | 12 ++++++++---- packages/react-router/lib/rsc/server.rsc.ts | 4 ++-- packages/react-router/lib/rsc/server.ssr.tsx | 2 +- .../react-router/lib/server-runtime/server.ts | 6 ------ 9 files changed, 38 insertions(+), 31 deletions(-) create mode 100644 .changeset/happy-hornets-judge.md diff --git a/.changeset/happy-hornets-judge.md b/.changeset/happy-hornets-judge.md new file mode 100644 index 0000000000..bb336a6b07 --- /dev/null +++ b/.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 diff --git a/integration/error-boundary-v2-test.ts b/integration/error-boundary-v2-test.ts index 32602e10a6..87d0fcbdfc 100644 --- a/integration/error-boundary-v2-test.ts +++ b/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!", ); }); }); diff --git a/integration/error-data-request-test.ts b/integration/error-data-request-test.ts index 772ef8af57..187ac67273 100644 --- a/integration/error-data-request-test.ts +++ b/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( diff --git a/packages/react-router/__tests__/server-runtime/server-test.ts b/packages/react-router/__tests__/server-runtime/server-test.ts index 69ab365b3a..725c49792e 100644 --- a/packages/react-router/__tests__/server-runtime/server-test.ts +++ b/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); }); diff --git a/packages/react-router/lib/dom/ssr/single-fetch.tsx b/packages/react-router/lib/dom/ssr/single-fetch.tsx index 4043e35c46..bffc02708d 100644 --- a/packages/react-router/lib/dom/ssr/single-fetch.tsx +++ b/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) diff --git a/packages/react-router/lib/rsc/browser.tsx b/packages/react-router/lib/rsc/browser.tsx index 119db6deef..8b9ce5086d 100644 --- a/packages/react-router/lib/rsc/browser.tsx +++ b/packages/react-router/lib/rsc/browser.tsx @@ -529,10 +529,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"); diff --git a/packages/react-router/lib/rsc/server.rsc.ts b/packages/react-router/lib/rsc/server.rsc.ts index d8880506ac..c911c7641a 100644 --- a/packages/react-router/lib/rsc/server.rsc.ts +++ b/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; } diff --git a/packages/react-router/lib/rsc/server.ssr.tsx b/packages/react-router/lib/rsc/server.ssr.tsx index 7eeb8a91f6..7662d29947 100644 --- a/packages/react-router/lib/rsc/server.ssr.tsx +++ b/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 || "", { diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index d3b78ae510..71f4d11b04 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/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", - }, }, ); }