Skip to content

fix: pass through plain ReadableStream to fix cancel and perf#1816

Open
eL1fe wants to merge 2 commits intoelysiajs:mainfrom
eL1fe:fix/readable-stream-cancel-and-perf
Open

fix: pass through plain ReadableStream to fix cancel and perf#1816
eL1fe wants to merge 2 commits intoelysiajs:mainfrom
eL1fe:fix/readable-stream-cancel-and-perf

Conversation

@eL1fe
Copy link

@eL1fe eL1fe commented Mar 24, 2026

Summary

Problem

// #1741: 100x slower than wrapping in new Response()
app.get('/file', () => Bun.file('large.bin').stream())

// #1768: cancel() never called on client disconnect
app.get('/stream', () => new ReadableStream({
  cancel() { /* cleanup resources - never called */ }
}))

The re-wrapping added per-chunk async overhead (iterator.next() + format + enqueue) and broke the native stream cancel propagation.

Changes

  • src/adapter/utils.ts: detect plain ReadableStreams (non-SSE, non-generator) and pipe them through a minimal TransformStream that only normalizes non-standard chunk types (DataView, Blob → Uint8Array) while passing standard chunks through directly

Test plan

  • 10MB stream completes in ~6ms (was 100x+ slower before)
  • ReadableStream cancel callback fires on reader cancellation
  • Generator streams still work correctly
  • Mixed chunk types (DataView, Blob) still normalized properly
  • All existing stream tests pass (26 pass, 0 fail)
  • All core + response tests pass (273 pass, 0 fail)

Fixes #1741, fixes #1768

Summary by CodeRabbit

  • Improvements
    • Stream handling now better detects native readable streams and efficiently passes them through without unnecessary wrapping. Chunks of various types (binary, string, blobs, and array buffers) are normalized on-the-fly to ensure reliable delivery and reduce formatting overhead, improving performance and compatibility with native data sources.

When a handler returns a ReadableStream, Elysia was wrapping it in a
new ReadableStream with per-chunk async iteration, causing ~100x latency
for large streams (elysiajs#1741) and preventing the cancel callback from firing
on client disconnect (elysiajs#1768).

Plain ReadableStreams (non-SSE, non-generator) are now piped through a
lightweight TransformStream that only normalizes non-standard chunk types
(DataView, Blob) while passing Uint8Array/string/ArrayBuffer chunks
through with minimal overhead.

Fixes elysiajs#1741, fixes elysiajs#1768
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cc0e4e7f-5e9a-4c16-a20f-52acbf1a608e

📥 Commits

Reviewing files that changed from the base of the PR and between fcab510 and 59ce750.

📒 Files selected for processing (1)
  • src/adapter/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/adapter/utils.ts

Walkthrough

Enhanced createStreamHandler to detect native ReadableStream inputs that are not SSE/generators and return a Response that reuses the stream via a TransformStream with type-aware chunk handling, avoiding the iterator-based wrapping path.

Changes

Cohort / File(s) Summary
Stream Pass-Through Optimization
src/adapter/utils.ts
Added early-return for native ReadableStream bodies: returns a Response that pipes the stream through a TransformStream, converting chunk types (Uint8Array, ArrayBuffer, views, string, Blob, and fallback stringification) and skipping the iterator-based wrapper and SSE/formatting path.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client
  participant Handler as createStreamHandler
  participant Stream as ReadableStream
  participant Transform as TransformStream
  participant Response as Response

  Client->>Handler: sends request / expects Response
  Handler->>Stream: inspects returned body
  alt native ReadableStream (not SSE/generator)
    Handler->>Transform: pipeThrough(type-aware transformer)
    Transform->>Response: attach transformed stream
    Response-->>Client: streamed response (pass-through)
  else non-native / generator / SSE
    Handler->>Handler: wrap via async-iterator (for-await)
    Handler->>Response: build iterator-backed stream
    Response-->>Client: streamed response (iterator path)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰
Streams once tangled, now set free,
I pipe and hop with nimbleness,
Chunks become bytes in gentle flight,
No more loops to fuss and stress,
Hooray — the server hums with less! 🥕

🚥 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 'fix: pass through plain ReadableStream to fix cancel and perf' accurately describes the main change: detecting plain ReadableStreams and passing them through a TransformStream to fix both performance and cancellation issues.
Linked Issues check ✅ Passed Changes address both linked issues: detect and pass-through plain ReadableStreams to fix 100x+ latency (#1741) and enable cancel callback propagation on client disconnect (#1768).
Out of Scope Changes check ✅ Passed All changes in src/adapter/utils.ts are scoped to stream detection and TransformStream passthrough logic directly tied to fixing #1741 and #1768; no unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/adapter/utils.ts (1)

343-348: Consider adding try/catch around JSON.stringify for consistency.

The existing pull handler (lines 435-442) catches JSON.stringify errors and falls back to chunk.toString(). The new transform path doesn't, which could throw on edge cases like circular references or BigInt values.

While plain ReadableStreams typically emit binary data rather than objects, adding the fallback would maintain consistent behavior.

♻️ Suggested fix
                             else
-                                controller.enqueue(
-                                    typeof chunk === 'object'
-                                        ? JSON.stringify(chunk)
-                                        : String(chunk)
-                                )
+                                try {
+                                    controller.enqueue(
+                                        typeof chunk === 'object'
+                                            ? JSON.stringify(chunk)
+                                            : String(chunk)
+                                    )
+                                } catch {
+                                    controller.enqueue(String(chunk))
+                                }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/adapter/utils.ts` around lines 343 - 348, The transform path currently
calls controller.enqueue(typeof chunk === 'object' ? JSON.stringify(chunk) :
String(chunk)) and can throw on JSON.stringify errors (e.g., circular refs);
wrap the JSON.stringify call in a try/catch and on error fall back to
chunk.toString() (or String(chunk)) before calling controller.enqueue so
behavior matches the existing pull handler's fallback; update the transform
branch around controller.enqueue and references to chunk to use this safe
stringify pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/adapter/utils.ts`:
- Around line 343-348: The transform path currently calls
controller.enqueue(typeof chunk === 'object' ? JSON.stringify(chunk) :
String(chunk)) and can throw on JSON.stringify errors (e.g., circular refs);
wrap the JSON.stringify call in a try/catch and on error fall back to
chunk.toString() (or String(chunk)) before calling controller.enqueue so
behavior matches the existing pull handler's fallback; update the transform
branch around controller.enqueue and references to chunk to use this safe
stringify pattern.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 99ae9042-f710-49cf-8c93-768b8774615d

📥 Commits

Reviewing files that changed from the base of the PR and between 56310be and fcab510.

📒 Files selected for processing (1)
  • src/adapter/utils.ts

Matches the existing fallback pattern in the pull handler for edge
cases like circular references or BigInt values.
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.

ReadableStream cancel callback never invoked on client disconnect Returning a ReadableStream results in 100% CPU and >100x latency

1 participant