Skip to content

Conversation

@abhishekg999
Copy link

@abhishekg999 abhishekg999 commented Dec 5, 2025

When using a stream with a trace and onAfterHandle handler, the stream still pointed to the stream object before tee is called in reportHandler. This PR updates it so that response and reponseValue don't point to the main generator.

The minimal POC for this is this:

import { Elysia } from './src'
import { req } from './test/utils'

const PluginA = () =>
	new Elysia({ name: 'PluginA' })
		.onAfterHandle(() => {})
		.trace(() => {})
		.as('global')

const app = new Elysia().use(PluginA()).get('/', async function* () {
	yield '1\n'
	yield '2\n'
	yield '3\n'
})

const response = await app.handle(req('/'))
console.log(await response.text())
process.exit(0)

this will print just "2". since the other values from the generator are getting captured by the trace.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed response handling for Server-Sent Events (SSE) when using plugin global hooks with trace enabled.
  • Tests

    • Added test coverage for SSE functionality with plugin global hooks and trace.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 5, 2025

Walkthrough

This PR modifies stream response handling in the compose layer to update context response values when afterHandle hooks are present during traced request processing, and adds a test verifying SSE behavior with global hooks and tracing.

Changes

Cohort / File(s) Summary
Stream Response Logic
src/compose.ts
Adds conditional assignment to update c.response and c.responseValue to current stream value when hooks.afterHandle hooks exist during traced stream iteration, occurring before listener setup and afterHandle hook execution.
Stream Response Tests
test/response/stream.test.ts
Imports reordered alphabetically; new test case "handle sse with plugin global hooks and trace" validates SSE stream output when a plugin with global lifecycle hooks and tracing is applied.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review the conditional logic in compose.ts to verify the placement and necessity of the response state update within the stream iteration branch
  • Confirm the new SSE test correctly validates the interaction between global hooks, tracing, and stream responses

Possibly related PRs

Suggested reviewers

  • SaltyAom

Poem

🐰 A stream flows through the hooks so bright,
Response values set just right,
When afterHandle stands in line,
The context state updates just fine,
Now SSE dances with trace and glee! 🌊

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main fix: updating stream references to use the teed value instead of the original generator, which directly matches the core change in compose.ts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • 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 07b449a and d929d08.

📒 Files selected for processing (2)
  • src/compose.ts (1 hunks)
  • test/response/stream.test.ts (2 hunks)
🔇 Additional comments (3)
src/compose.ts (1)

1753-1767: Keep c.response in sync with teed stream when afterHandle hooks exist

Updating c.response/c.responseValue to the teed r inside the traced-stream branch is correct here: without this, r was changed to stream[0] but later r = c.response would revert it back to the pre‑tee generator, so afterHandle + mapResponse operated on the wrong stream, causing chunks to be “eaten” by tracing. This conditional keeps all response references aligned with the teed stream path while leaving non‑afterHandle cases unchanged.

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

1-1: Import order change is fine

Reordering the bun:test named imports has no behavioral impact and matches the rest of the file’s usage.


564-597: New SSE test accurately covers trace + global hooks regression

This test is well‑targeted: it wires a global plugin with onAfterHandle and trace (plus other hooks), streams three SSE events, and validates both the content type and that all three chunks emerge via streamResponse. This directly exercises the fixed tee/trace path and should prevent regressions.


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.

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.

1 participant