Document compression middleware compatibility with streaming SSR#2544
Document compression middleware compatibility with streaming SSR#2544AbanoubGhadban merged 1 commit intomasterfrom
Conversation
Document the Rack::Deflater/Brotli deadlock when `:if` conditions call `body.each` on streaming responses, and provide the correct `to_ary` check pattern. Closes #2519. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughDocumentation updates addressing RSC payload streaming deadlock issues with Rack compression middleware. Adds guidance on compression middleware compatibility, explains the deadlock scenario caused by body.each consumption on streaming responses, and provides correct patterns with troubleshooting references across four documentation files. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
Greptile SummaryThis PR is a documentation-only change that adds a "Compression Middleware Compatibility" section to both the open-source and Pro streaming SSR guides, a troubleshooting entry in the Pro troubleshooting doc, and a cross-reference note in the general server-rendering tips page. It addresses a real, non-obvious deadlock (#2519) where compression middleware Key additions:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant RackMiddleware as Rack Compression Middleware
participant SizedQueue as ActionController::Live::Buffer (SizedQueue)
participant Compressor
Note over RackMiddleware: BAD: :if condition calls body.each
Browser->>RackMiddleware: HTTP Request (streaming page)
RackMiddleware->>SizedQueue: body.each { |i| sum += i.length } — drains queue
SizedQueue-->>RackMiddleware: All chunks consumed (queue empty)
RackMiddleware->>Compressor: Attempt to compress remaining body
Compressor->>SizedQueue: each (blocks waiting for chunks)
Note over Compressor,SizedQueue: DEADLOCK — queue empty, no more chunks arrive
Note over RackMiddleware: GOOD: :if condition checks to_ary first
Browser->>RackMiddleware: HTTP Request (streaming page)
RackMiddleware->>SizedQueue: body.respond_to?(:to_ary) → false
RackMiddleware-->>Compressor: return true (always compress streaming body)
loop Each streamed chunk
SizedQueue-->>Compressor: chunk
Compressor-->>Browser: compressed chunk (sync flush)
end
Last reviewed commit: d78ab4b |
|
|
||
| **Cause**: A compression middleware (`Rack::Deflater`, `Rack::Brotli`) is configured with an `:if` condition that calls `body.each` to check the response size. This destructively consumes streaming chunks from the `SizedQueue`, causing a deadlock. | ||
|
|
||
| **Fix**: See the "Compression Middleware Compatibility" section in the [Streaming Server Rendering guide](./streaming-server-rendering.md). |
There was a problem hiding this comment.
Missing section anchor in cross-reference link
The link here drops the user at the top of the streaming guide rather than directly at the relevant section. The corresponding link in docs/deployment/server-rendering-tips.md correctly uses the #compression-middleware-compatibility anchor. Adding it here keeps both cross-references consistent and saves the reader from having to scroll.
| **Fix**: See the "Compression Middleware Compatibility" section in the [Streaming Server Rendering guide](./streaming-server-rendering.md). | |
| **Fix**: See the "Compression Middleware Compatibility" section in the [Streaming Server Rendering guide](./streaming-server-rendering.md#compression-middleware-compatibility). |
Review: Documentation — Compression Middleware Compatibility with Streaming SSRThis is a well-written, technically accurate documentation addition that addresses a real and subtle deadlock issue. The explanation of the root cause (destructive Positive aspects
IssuesMinor (documentation clarity)
Non-issues / confirmed correct
Overall this is a valuable addition. Addressing the |
| # BAD — causes deadlocks with streaming responses | ||
| config.middleware.use Rack::Deflater, if: lambda { |*, body| | ||
| sum = 0 | ||
| body.each { |i| sum += i.length } # destructive — drains the queue |
There was a problem hiding this comment.
The bad example uses i.length (character count) while the good example uses i.bytesize (byte count). Since the only anti-pattern here is the destructive body.each call, keeping the measurement consistent (bytesize in both) would isolate the single mistake and make the before/after diff clearer for readers.
| body.each { |i| sum += i.length } # destructive — drains the queue | |
| body.each { |i| sum += i.bytesize } # destructive — drains the queue |
| sum = 0 | ||
| body.each { |i| sum += i.length } # destructive — drains the queue | ||
| sum > 512 | ||
| } |
There was a problem hiding this comment.
Same i.length vs i.bytesize inconsistency as in the open-source streaming guide — using bytesize here too would keep the BAD/GOOD examples focused on the single anti-pattern (destructive iteration).
| } | |
| body.each { |i| sum += i.bytesize } # destructive — drains the queue |
## Summary - Add "Compression Middleware Compatibility" section to both streaming SSR guides (open-source and Pro) explaining the `Rack::Deflater`/`Rack::Brotli` deadlock when `:if` conditions call `body.each` on streaming responses - Add troubleshooting entry to Pro troubleshooting docs for the "streaming hangs indefinitely" symptom - Add a note to the server rendering tips troubleshooting section with a cross-reference The root cause is that streaming bodies use `ActionController::Live::Buffer` backed by a `SizedQueue`, where `each` is destructive (pops items). When a middleware `:if` condition calls `body.each` to check response size, it drains the queue, leaving nothing for the compressor. The fix is to check `body.respond_to?(:to_ary)` first — streaming bodies don't support `to_ary`, so the condition should return `true` (always compress) for them and only check size for buffered responses. Closes #2519 ## Test plan - [x] Documentation-only change, no code changes - [x] Verified markdown renders correctly - [x] Cross-references between docs use correct relative paths 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Documentation * Added comprehensive guidance on compression middleware compatibility with streaming server-side rendering, including common deadlock scenarios and correct implementation patterns. * Enhanced troubleshooting documentation with new section addressing streaming server-side rendering request hangs and resolution steps. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…upport * origin/master: (38 commits) fix: use env-var-driven ports in Procfile templates to support multiple worktrees (#2539) Fix prettier formatting in auto-bundling doc Docs: Clarify .client/.server suffixes vs use client RSC directive (#2406) Warn against using .server/.client variants with RSC features Docs: move internal-only docs out of published docs trees (#2414) Fix crash when HTTPX::ErrorResponse is returned in get_form_body_for_file (#2532) Skip flaky external URLs in lychee checks (#2547) Update mise docs: prefer shell-level shims over conductor-exec (#2537) Document compression middleware compatibility with streaming SSR (#2544) Fix duplicate node-renderer message reporting in render failures (#2531) Fix private_output_path not configured on fresh Shakapacker installs (#2289) Bump the npm-security group across 1 directory with 3 updates (#2387) docs: use https links (#2518) Consolidate changelog to keep only rc6 prerelease (#2533) Fix CSS module class name divergence with rspack SSR (#2489) Bump version to 16.4.0.rc.6 Add BugBot license scanning config (#2515) Fix buildVM promise cleanup ordering (#2483) (#2484) Fix streaming SSR hangs and silent error absorption in RSC payload injection (#2407) Ensure lefthook uses mise tools in non-interactive shells (#2512) ... # Conflicts: # CHANGELOG.md
## Summary - Add changelog entries for 6 user-visible PRs merged since v16.4.0.rc.6 that were missing from `[Unreleased]` - Update existing #2561 entry to include #2568 contributor credit ### New entries added | Section | PR | Description | |---|---|---| | Added | #2539 | Environment-variable-driven ports in Procfile templates | | Fixed | #2417 | Rspack generator config path fix | | Fixed | #2419 | Precompile hook load-based execution fix | | Fixed | #2577 | `create-react-on-rails-app` validation improvements | | Pro Fixed | #2416 | StreamResponse status fallback fix | | Pro Fixed | #2566 | Empty-string license plan mismatch fix | ### Skipped PRs (not user-visible) Docs (#2406, #2414, #2479, #2494, #2518, #2537, #2544), CI/internal (#2533, #2547, #2555, #2557, #2558, #2564), dependabot (#2387, #2541), dev dependencies (#2559, #2569, #2573). ## Test plan - [ ] Verify changelog formatting matches existing entries - [ ] Verify all user-visible PRs since v16.4.0.rc.6 are covered 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only changelog updates with no runtime or build behavior changes. > > **Overview** > Updates `CHANGELOG.md`’s **[Unreleased]** section to include previously missing user-facing entries: Procfile templates now support env-driven ports, several generator/`bin/dev` precompile-hook and rspack-path fixes are documented, and `create-react-on-rails-app` validation improvements are noted. > > Also adds two Pro fix entries (StreamResponse status fallback and license plan empty-string validation) and updates the existing `bin/dev` precompile-hook entry to credit an additional PR/contributor. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e75d2b5. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Rack::Deflater/Rack::Brotlideadlock when:ifconditions callbody.eachon streaming responsesThe root cause is that streaming bodies use
ActionController::Live::Bufferbacked by aSizedQueue, whereeachis destructive (pops items). When a middleware:ifcondition callsbody.eachto check response size, it drains the queue, leaving nothing for the compressor. The fix is to checkbody.respond_to?(:to_ary)first — streaming bodies don't supportto_ary, so the condition should returntrue(always compress) for them and only check size for buffered responses.Closes #2519
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation