Skip to content

fix: missing cookies in sse#1435

Merged
SaltyAom merged 1 commit intoelysiajs:mainfrom
Rouret:fix/missing-handleSet-for-cookie-in-stream-response
Sep 27, 2025
Merged

fix: missing cookies in sse#1435
SaltyAom merged 1 commit intoelysiajs:mainfrom
Rouret:fix/missing-handleSet-for-cookie-in-stream-response

Conversation

@Rouret
Copy link
Copy Markdown
Contributor

@Rouret Rouret commented Sep 24, 2025

Fix #1434

Problem

When using generator functions (streaming responses) with cookies in ElysiaJS, the set-cookie headers were not being included in the HTTP response headers. This occurred because cookies set via context.cookie['name'].set() before yielding the first chunk were not processed and added to the response headers.

new Elysia().get('/', async function* (context) {
  context.cookie['my-cookie'].set({ value: 'test-value' })
  yield sse({ event: 'my-event', data: { message: 'Hello' } })
})
// The set-cookie header was missing from the response

Root Cause

The createStreamHandler function in src/adapter/utils.ts was not calling handleSet() to process cookies and other context properties before creating the Response object. The handleSet() function is responsible for serializing cookies from context.set.cookie into proper set-cookie headers.

Solution

Added a call to handleSet(set) in the createStreamHandler function before the generator starts processing, ensuring that cookies are properly serialized and included in the response headers.

Summary by CodeRabbit

  • Bug Fixes

    • Ensures headers/cookies are applied before the first streamed event, preventing missed mutations at stream start.
    • Preserves multiple Set-Cookie headers in streamed responses (e.g., SSE), improving session and auth reliability.
  • Tests

    • Added test verifying that streamed responses include multiple Set-Cookie headers.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 24, 2025

Walkthrough

Adjusts stream handling to apply set mutations (headers/cookies) before awaiting the generator’s first yield in createStreamHandler. Adds a test verifying multiple Set-Cookie headers are included in streamed SSE responses when cookies are set prior to the first chunk.

Changes

Cohort / File(s) Summary
Stream handler header/cookie timing
src/adapter/utils.ts
Move handleSet(set) invocation to occur before awaiting the generator’s initial result in createStreamHandler so header/cookie mutations are applied earlier. No signature changes.
SSE cookie header test
test/response/stream.test.ts
Add test asserting two Set-Cookie headers are present in an SSE streamed response when two cookies are set before the first yield. No changes to existing tests.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant H as createStreamHandler
  participant S as handleSet(set)
  participant G as Stream Generator
  participant R as Response

  Note over H: New flow: apply set before first yield
  C->>H: HTTP request (SSE)
  H->>S: Apply headers/cookies from set
  S-->>H: Set mutations applied
  H->>G: Start generator
  G-->>H: First yield (SSE chunk)
  H->>R: Send headers (incl. Set-Cookie) + first chunk
  loop Subsequent chunks
    G-->>H: Next yield
    H-->>R: Stream chunk
  end
  G-->>H: Complete
  H-->>C: Stream closed
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nudge the stream before it flows,
Set-cookie crumbs on breezy rows;
Two sweet treats in headers ride,
As events downstream gently glide.
Thump-thump! my paws approve the art—
Cookies first, then stories start. 🍪🐇

Pre-merge checks and finishing touches

✅ 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 succinctly describes the fix for missing cookies in SSE streaming responses and aligns with the primary change of invoking handleSet to include Set-Cookie headers before the first chunk is sent.
Linked Issues Check ✅ Passed The implementation addresses issue #1434 by calling handleSet before processing the first yield so that cookies are serialized into Set-Cookie headers, and the added test verifies the presence of multiple Set-Cookie headers in the streamed response.
Out of Scope Changes Check ✅ Passed All changes are directly focused on adding handleSet for cookies in the streaming handler and introducing a corresponding test; no unrelated files or logic were modified.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 eb4d126 and 84018f3.

📒 Files selected for processing (2)
  • src/adapter/utils.ts (1 hunks)
  • test/response/stream.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/adapter/utils.ts (1)
src/cookies.ts (1)
  • set (251-261)
test/response/stream.test.ts (2)
src/utils.ts (1)
  • sse (1109-1167)
test/utils.ts (1)
  • req (1-2)
🔇 Additional comments (2)
src/adapter/utils.ts (1)

162-162: LGTM! Fix resolves the core issue with cookie serialization timing.

The addition of handleSet(set) before awaiting the initial generator result ensures cookies are serialized into Set-Cookie headers before the Response object is created. This addresses the root cause where cookies set via context.cookie['name'].set() weren't being serialized for streaming responses.

test/response/stream.test.ts (1)

99-122: Comprehensive test validates the fix for multiple cookie headers in streaming responses.

The test correctly verifies that:

  1. Multiple cookies can be set before the first yield in a streaming handler
  2. Both cookies are serialized into separate Set-Cookie headers
  3. The SSE streaming functionality works alongside cookie handling

This test case aligns perfectly with the PR objectives and provides regression coverage for issue #1434.


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.

@SaltyAom SaltyAom merged commit 6ce672c into elysiajs:main Sep 27, 2025
1 check passed
@SaltyAom SaltyAom mentioned this pull request Sep 27, 2025
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.

set-cookie does not work for streamed responses

2 participants