Skip to content

fix: clear event.res after prepare#1259

Merged
pi0 merged 2 commits into
mainfrom
fix/prepare-once
Dec 8, 2025
Merged

fix: clear event.res after prepare#1259
pi0 merged 2 commits into
mainfrom
fix/prepare-once

Conversation

@pi0

@pi0 pi0 commented Dec 8, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

resolves #1257 similar issue to #1226

When using onResponse middleware helper, it internally uses toResponse, which merges headers. In final response we merge headers again which can cause duplicate cookies.

Although we could call toResponse with a special flag to avoid merge in middleware, this PR fixes the root cause by always clearing event.response state after first prepare to avoid similar issues.

Summary by CodeRabbit

  • New Features

    • Added an exported onResponse middleware for handling response processing.

  • Bug Fixes

    • Fixed an issue that could cause response headers (notably Set-Cookie) to be duplicated during middleware processing.

  • Tests

    • Added tests to ensure response headers are preserved correctly and not duplicated when middleware appends headers.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 8, 2025 •
edited
Loading

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Clears the prepared response stored on the event object after computing a prepared response to avoid duplicated response side-effects, adds an export for onResponse, and adds a test ensuring Set-Cookie headers are not duplicated when middleware appends them. No public API signatures changed.

Changes

Cohort / File(s) Summary
Response cleanup
src/response.ts		 Adds a cleanup step that sets (event as any)[kEventRes] = undefined after computing prepared response data to prevent duplication of response data on the event.
Public export
src/index.ts		 Adds export of onResponse (used by middleware/tests).
Test added
test/middleware.test.ts		 Imports onResponse and adds a test validating onResponse() does not produce duplicate Set-Cookie headers when subsequent code appends a Set-Cookie.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Small, localized change with a single side-effect.
  • Review focus:
    • src/response.ts — confirm cleanup placement and that it doesn't alter control flow or error handling.
    • src/index.ts — verify the onResponse export is correct.
    • test/middleware.test.ts — ensure the new test correctly exercises the scenario and assertions are precise.

Poem

🐰 I hopped through the event and tidied the nest,
Cleared the old crumb so cookies won't duplicate—best!
A tiny export, a test that will see,
One cookie, one header, all tidy for me. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: clear prepared headers after prepare' directly describes the main change: clearing prepared response state after initial preparation to prevent header duplication.
Linked Issues check ✅ Passed The PR implements the proposed fix for issue #1257 by clearing the prepared response state after prepare to prevent duplicate Set-Cookie headers, addressing the root cause of the duplication.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the duplicate headers issue: clearing prepared response in src/response.ts and adding a test in test/middleware.test.ts to verify the fix works.
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ec436f and 46642d2.

📒 Files selected for processing (1)
  • test/middleware.test.ts (2 hunks)

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 and usage tips.

@pi0 pi0 changed the title fix: clear prepared headers after prepare fix: clear event.res headers after prepare Dec 8, 2025
@pi0 pi0 changed the title fix: clear event.res headers after prepare fix: clear event.res after prepare Dec 8, 2025
@pi0
pi0 merged commit 9991a5d into main Dec 8, 2025
4 of 6 checks passed
@pi0
pi0 deleted the fix/prepare-once branch December 8, 2025 12:39
@pi0 pi0 mentioned this pull request Dec 8, 2025
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Dec 23, 2025
Closed
schiller-manuel added a commit to TanStack/router that referenced this pull request Dec 30, 2025
@schiller-manuel
pin h3 version
89c0fd7 
h3js/h3#1259 broke formdata handling in server functions, so pin h3 to a working version temporarily
@coderabbitai coderabbitai Bot mentioned this pull request Mar 19, 2026
Merged
3 tasks
@pi0x pi0x mentioned this pull request Jul 18, 2026
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Jul 18, 2026
Merged
This was referenced Jul 22, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

The setCookie method executes twice to set the same set-cookie header.

1 participant

@pi0