fix: include set-cookie headers from bubbled thrown responses

.changeset/bubbled-response-cookies.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/server-runtime": patch
+---
+
+Automatically include set-cookie headers from bubbled thrown responses

integration/headers-test.ts

@@ -160,6 +160,43 @@ test.describe("headers export", () => {
 
           export default function Component() { return <div/> }
         `,
+
+        "app/routes/cookie.jsx": js`
+          import { json } from "@remix-run/server-runtime";
+          import { Outlet } from "@remix-run/react";
+
+          export function loader({ request }) {
+            if (new URL(request.url).searchParams.has("parent-throw")) {
+              throw json(null, { headers: { "Set-Cookie": "parent-thrown-cookie=true" } });
+            }
+            return null
+          };
+
+          export default function Parent() {
+            return <Outlet />;
+          }
+
+          export function ErrorBoundary() {
+            return <h1>Caught!</h1>;
+          }
+        `,
+
+        "app/routes/cookie.child.jsx": js`
+          import { json } from "@remix-run/node";
+
+          export function loader({ request }) {
+            if (new URL(request.url).searchParams.has("throw")) {
+              throw json(null, { headers: { "Set-Cookie": "thrown-cookie=true" } });
+            }
+            return json(null, {
+              headers: { "Set-Cookie": "normal-cookie=true" },
+            });
+          };
+
+          export default function Child() {
+            return <p>Child</p>;
+          }
+        `,
       },
     });
   });
@@ -350,6 +387,36 @@ test.describe("headers export", () => {
       ])
     );
   });
+
+  test("automatically includes cookie headers from normal responses", async () => {
+    let response = await appFixture.requestDocument("/cookie/child");
+    expect(JSON.stringify(Array.from(response.headers.entries()))).toBe(
+      JSON.stringify([
+        ["content-type", "text/html"],
+        ["set-cookie", "normal-cookie=true"],
+      ])
+    );
+  });
+
+  test("automatically includes cookie headers from thrown responses", async () => {
+    let response = await appFixture.requestDocument("/cookie/child?throw");
+    expect(JSON.stringify(Array.from(response.headers.entries()))).toBe(
+      JSON.stringify([
+        ["content-type", "text/html"],
+        ["set-cookie", "thrown-cookie=true"],
+      ])
+    );
+  });
+
+  test("does not duplicate thrown cookie headers from boundary route", async () => {
+    let response = await appFixture.requestDocument("/cookie?parent-throw");
+    expect(JSON.stringify(Array.from(response.headers.entries()))).toBe(
+      JSON.stringify([
+        ["content-type", "text/html"],
+        ["set-cookie", "parent-thrown-cookie=true"],
+      ])
+    );
+  });
 });
 
 test.describe("v1 behavior (future.v2_headers=false)", () => {
@@ -411,6 +478,43 @@ test.describe("v1 behavior (future.v2_headers=false)", () => {
 
           export default function Component() { return <div/> }
         `,
+
+        "app/routes/cookie.jsx": js`
+          import { json } from "@remix-run/server-runtime";
+          import { Outlet } from "@remix-run/react";
+
+          export function loader({ request }) {
+            if (new URL(request.url).searchParams.has("parent-throw")) {
+              throw json(null, { headers: { "Set-Cookie": "parent-thrown-cookie=true" } });
+            }
+            return null
+          };
+
+          export default function Parent() {
+            return <Outlet />;
+          }
+
+          export function ErrorBoundary() {
+            return <h1>Caught!</h1>;
+          }
+        `,
+
+        "app/routes/cookie.child.jsx": js`
+          import { json } from "@remix-run/node";
+
+          export function loader({ request }) {
+            if (new URL(request.url).searchParams.has("throw")) {
+              throw json(null, { headers: { "Set-Cookie": "thrown-cookie=true" } });
+            }
+            return json(null, {
+              headers: { "Set-Cookie": "normal-cookie=true" },
+            });
+          };
+
+          export default function Child() {
+            return <p>Child</p>;
+          }
+        `,
       },
     });
   });
@@ -431,4 +535,34 @@ test.describe("v1 behavior (future.v2_headers=false)", () => {
       JSON.stringify([["content-type", "text/html"]])
     );
   });
+
+  test("automatically includes cookie headers from normal responses", async () => {
+    let response = await appFixture.requestDocument("/cookie/child");
+    expect(JSON.stringify(Array.from(response.headers.entries()))).toBe(
+      JSON.stringify([
+        ["content-type", "text/html"],
+        ["set-cookie", "normal-cookie=true"],
+      ])
+    );
+  });
+
+  test("automatically includes cookie headers from thrown responses", async () => {
+    let response = await appFixture.requestDocument("/cookie/child?throw");
+    expect(JSON.stringify(Array.from(response.headers.entries()))).toBe(
+      JSON.stringify([
+        ["content-type", "text/html"],
+        ["set-cookie", "thrown-cookie=true"],
+      ])
+    );
+  });
+
+  test("does not duplicate thrown cookie headers from boundary route", async () => {
+    let response = await appFixture.requestDocument("/cookie?parent-throw");
+    expect(JSON.stringify(Array.from(response.headers.entries()))).toBe(
+      JSON.stringify([
+        ["content-type", "text/html"],
+        ["set-cookie", "parent-thrown-cookie=true"],
+      ])
+    );
+  });
 });

integration/hmr-log-test.ts

@@ -28,6 +28,7 @@ let fixture = (options: {
           v2_errorBoundary: true,
           v2_normalizeFormMethod: true,
           v2_meta: true,
+          v2_headers: true,
         },
       };
     `,

integration/hmr-test.ts

@@ -28,6 +28,7 @@ let fixture = (options: {
           v2_errorBoundary: true,
           v2_normalizeFormMethod: true,
           v2_meta: true,
+          v2_headers: true,
         },
       };
     `,

packages/remix-server-runtime/headers.ts

@@ -38,10 +38,25 @@ export function getDocumentHeadersRR(
     let loaderHeaders = context.loaderHeaders[id] || new Headers();
     let actionHeaders = context.actionHeaders[id] || new Headers();
 
+    // Only expose errorHeaders to the leaf headers() function to
+    // avoid duplication via parentHeaders
+    let includeErrorHeaders =
+      errorHeaders != undefined && idx === matches.length - 1;
+    // Only prepend cookies from errorHeaders at the leaf renderable route
+    // when it's not the same as loaderHeaders/actionHeaders to avoid
+    // duplicate cookies
+    let includeErrorCookies =
+      includeErrorHeaders &&
+      errorHeaders !== loaderHeaders &&
+      errorHeaders !== actionHeaders;
+
     // When the future flag is enabled, use the parent headers for any route
     // that doesn't have a `headers` export
     if (routeModule.headers == null && build.future.v2_headers) {
-      let headers = parentHeaders;
+      let headers = new Headers(parentHeaders);
+      if (includeErrorCookies) {
+        prependCookies(errorHeaders!, headers);
+      }
       prependCookies(actionHeaders, headers);
       prependCookies(loaderHeaders, headers);
       return headers;
@@ -54,17 +69,17 @@ export function getDocumentHeadersRR(
               loaderHeaders,
               parentHeaders,
               actionHeaders,
-              // Only expose errorHeaders to the leaf headers() function to
-              // avoid duplication via parentHeaders
-              errorHeaders:
-                idx === matches.length - 1 ? errorHeaders : undefined,
+              errorHeaders: includeErrorHeaders ? errorHeaders : undefined,
             })
           : routeModule.headers
         : undefined
     );
 
-    // Automatically preserve Set-Cookie headers that were set either by the
-    // loader or by a parent route.
+    // Automatically preserve Set-Cookie headers from bubbled responses,
+    // loaders, errors, and parent routes
+    if (includeErrorCookies) {
+      prependCookies(errorHeaders!, headers);
+    }
     prependCookies(actionHeaders, headers);
     prependCookies(loaderHeaders, headers);
     prependCookies(parentHeaders, headers);