Skip to content

fix(start-server-core): copy immutable Response.redirect() headers when merging pending set-cookie values#7765

Open
thribhuvan003 wants to merge 1 commit into
TanStack:mainfrom
thribhuvan003:fix/redirect-immutable-headers
Open

fix(start-server-core): copy immutable Response.redirect() headers when merging pending set-cookie values#7765
thribhuvan003 wants to merge 1 commit into
TanStack:mainfrom
thribhuvan003:fix/redirect-immutable-headers

Conversation

@thribhuvan003

@thribhuvan003 thribhuvan003 commented Jul 9, 2026

Copy link
Copy Markdown

returning Response.redirect() from a handler after calling setCookie() crashes the whole request — mergeEventResponseHeaders calls .delete('set-cookie') on the redirect's headers, which are immutable per spec, so the merge throws TypeError: Headers are immutable and the user gets a 500 instead of their redirect + cookie.

fixed by copying the response (same status/statusText/body, fresh mutable headers) when the in-place delete throws, and merging into the copy. mutable responses keep the exact in-place behavior they had before.

added two tests: the redirect case fails without the fix and passes with it, and a mutable non-ok response still merges the same way. the three test files already failing in this package (createCsrfMiddleware, createStartHandler, frame-protocol) fail identically on a clean checkout of main, so they're unrelated.

fixes #7755

Summary by CodeRabbit

  • Bug Fixes

    • Fixed redirects and other non-OK responses so cookies set during a request are preserved instead of causing a server error.
    • Improved handling of responses with immutable headers, preventing failed handler requests and keeping redirect destinations intact.
  • Tests

    • Added coverage for cookie merging with redirect responses and other responses that already include headers.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 17506a6a-e61e-467e-92d3-d2fd95cc7cbc

📥 Commits

Reviewing files that changed from the base of the PR and between a3e24c3 and a50c212.

📒 Files selected for processing (3)
  • .changeset/redirect-immutable-headers.md
  • packages/start-server-core/src/request-response.ts
  • packages/start-server-core/tests/request-response.test.ts

📝 Walkthrough

Walkthrough

Fixes a bug where merging pending set-cookie headers into an immutable Response (e.g., from Response.redirect()) threw a TypeError, causing a 500 error. mergeEventResponseHeaders now returns a merged Response, copying it when headers can't be mutated; attachResponseHeaders returns this result. Tests and a changeset are added.

Changes

Immutable header merge fix

Layer / File(s) Summary
mergeEventResponseHeaders and attachResponseHeaders fix
packages/start-server-core/src/request-response.ts
mergeEventResponseHeaders returns a merged Response, copying it into a mutable instance when deleting set-cookie fails (immutable headers), then appends cookies from both sources; attachResponseHeaders returns this merged Response instead of discarding it.
Tests and changelog for cookie merge fix
packages/start-server-core/tests/request-response.test.ts, .changeset/redirect-immutable-headers.md
Adds tests verifying cookies merge correctly into a Response.redirect() result and into a mutable non-OK Response with existing headers; adds a changeset documenting the fix and referencing issue #7755.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Handler
  participant attachResponseHeaders
  participant mergeEventResponseHeaders
  participant Response

  Handler->>attachResponseHeaders: return Response.redirect(url)
  attachResponseHeaders->>mergeEventResponseHeaders: merge(response, event)
  mergeEventResponseHeaders->>Response: delete set-cookie header
  Response--x mergeEventResponseHeaders: throws TypeError (immutable)
  mergeEventResponseHeaders->>Response: copy into new mutable Response
  mergeEventResponseHeaders->>Response: append merged set-cookie values
  mergeEventResponseHeaders-->>attachResponseHeaders: merged Response
  attachResponseHeaders-->>Handler: merged Response (302 with cookies)
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main fix for immutable Response.redirect() header merging.
Linked Issues check ✅ Passed The code and tests implement the requested redirect-cookie merge fallback for immutable headers and preserve mutable-response behavior.
Out of Scope Changes check ✅ Passed The only extra change is a changeset entry for the same fix; no unrelated functionality was added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[start] Unhandled 500 (TypeError: immutable) when a server route returns Response.redirect() while a set-cookie is pending on the event

1 participant