diff --git a/.changeset/cyan-dingos-report.md b/.changeset/cyan-dingos-report.md new file mode 100644 index 00000000000..0a9b7616faf --- /dev/null +++ b/.changeset/cyan-dingos-report.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Fix issue with setting additional headers on raw native fetch responses with immutable headers diff --git a/packages/remix-server-runtime/__tests__/server-test.ts b/packages/remix-server-runtime/__tests__/server-test.ts index 1195e4fdaf5..f84719f12c8 100644 --- a/packages/remix-server-runtime/__tests__/server-test.ts +++ b/packages/remix-server-runtime/__tests__/server-test.ts @@ -153,6 +153,48 @@ describe("shared server runtime", () => { expect(resourceLoader.mock.calls.length).toBe(1); }); + test("calls resource route loader throwing response with immutable headers", async () => { + let build = mockServerBuild({ + root: { + default: {}, + }, + "routes/resource": { + parentId: "root", + path: "resource", + loader() { + let headers = new Headers({ "x-test": "yes" }); + let headersProxy = new Proxy(headers, { + get(target, prop, receiver) { + if (prop === "set") { + throw new TypeError("immutable"); + } + return Reflect.get(target, prop, receiver); + }, + }); + // Mock a "response" that will pass the `isResponse` check + throw { + status: 400, + statusText: "Bad Request", + headers: headersProxy, + body: "text", + text: () => Promise.resolve("text"), + }; + }, + }, + }); + let handler = createRequestHandler(build, ServerMode.Development); + + let request = new Request(`${baseUrl}/resource`, { + method: "get", + }); + + let result = await handler(request); + expect(result.status).toBe(400); + expect(result.headers.get("x-test")).toBe("yes"); + expect(result.headers.get("X-Remix-Catch")).toBe("yes"); + expect(await result.text()).toBe("text"); + }); + test("calls sub resource route loader", async () => { let rootLoader = jest.fn(() => { return "root"; @@ -612,6 +654,90 @@ describe("shared server runtime", () => { expect(indexLoader.mock.calls.length).toBe(1); }); + test("data request calls loader returning response with immutable headers", async () => { + let build = mockServerBuild({ + root: { + default: {}, + }, + "routes/_index": { + parentId: "root", + index: true, + loader() { + let headers = new Headers({ "x-test": "yes" }); + let headersProxy = new Proxy(headers, { + get(target, prop, receiver) { + if (prop === "set") { + throw new TypeError("immutable"); + } + return Reflect.get(target, prop, receiver); + }, + }); + // Mock a "response" that will pass the `isResponse` check + return { + status: 200, + statusText: "OK", + headers: headersProxy, + body: "text", + text: () => Promise.resolve("text"), + }; + }, + }, + }); + let handler = createRequestHandler(build, ServerMode.Development); + + let request = new Request(`${baseUrl}/?_data=routes/_index`, { + method: "get", + }); + + let result = await handler(request); + expect(result.status).toBe(200); + expect(result.headers.get("x-test")).toBe("yes"); + expect(result.headers.get("X-Remix-Response")).toBe("yes"); + expect(await result.text()).toBe("text"); + }); + + test("data request calls loader throwing response with immutable headers", async () => { + let build = mockServerBuild({ + root: { + default: {}, + }, + "routes/_index": { + parentId: "root", + index: true, + loader() { + let headers = new Headers({ "x-test": "yes" }); + let headersProxy = new Proxy(headers, { + get(target, prop, receiver) { + if (prop === "set") { + throw new TypeError("immutable"); + } + return Reflect.get(target, prop, receiver); + }, + }); + // Mock a "response" that will pass the `isResponse` check + throw { + status: 400, + statusText: "Bad Request", + headers: headersProxy, + body: "text", + text: () => Promise.resolve("text"), + }; + }, + }, + }); + let handler = createRequestHandler(build, ServerMode.Development); + + let request = new Request(`${baseUrl}/?_data=routes/_index`, { + method: "get", + }); + + let result = await handler(request); + expect(result.status).toBe(400); + expect(result.headers.get("x-test")).toBe("yes"); + expect(result.headers.get("X-Remix-Catch")).toBe("yes"); + expect(await result.text()).toBe("text"); + }); + test("data request calls loader and responds with generic message and error header", async () => { let rootLoader = jest.fn(() => { throw new Error("test"); diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 620604acd49..fd81da453b9 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -360,12 +360,12 @@ async function handleDataRequest( // Mark all successful responses with a header so we can identify in-flight // network errors that are missing this header - response.headers.set("X-Remix-Response", "yes"); + response = safelySetHeader(response, "X-Remix-Response", "yes"); return response; } catch (error: unknown) { if (isResponse(error)) { - error.headers.set("X-Remix-Catch", "yes"); - return error; + let response = safelySetHeader(error, "X-Remix-Catch", "yes"); + return response; } if (isRouteErrorResponse(error)) { @@ -705,8 +705,8 @@ async function handleResourceRequest( 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; + let response = safelySetHeader(error, "X-Remix-Catch", "yes"); + return response; } if (isResponseStub(error)) { @@ -799,3 +799,23 @@ function createRemixRedirectResponse( headers, }); } + +// Anytime we are setting a header on a `Response` created in the loader/action, +// we have to so it in this manner since in an `undici` world, if the `Response` +// came directly from a `fetch` call, the headers are immutable will throw if +// we try to set a new header. This is a sort of shallow clone of the `Response` +// so we can safely set our own header. +function safelySetHeader( + response: Response, + name: string, + value: string +): Response { + let headers = new Headers(response.headers); + headers.set(name, value); + return new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers, + duplex: response.body ? "half" : undefined, + } as ResponseInit & { duplex?: "half" }); +}