Stream Pro rolling-deploy bundle downloads with caps#3435
Conversation
WalkthroughStreams rolling-deploy bundle tarballs into a Tempfile with a 50 MB compressed-body cap (COMPRESSED_BODY_CAP), refactors HTTP helper construction to share request building, and adds specs plus a CHANGELOG entry documenting the change. ChangesStreaming Bundle Downloads with Compressed-Body Cap
Sequence DiagramsequenceDiagram
participant fetch as fetch(bundle_hash)
participant download as download_bundle_tarball
participant stream as http_stream
participant tempfile as Tempfile
participant extract as extract_payload
participant error as ReactOnRailsPro::Error
fetch->>download: yield block
download->>stream: stream HTTP response
stream->>tempfile: write chunks
tempfile->>tempfile: track compressed bytes
alt exceeds COMPRESSED_BODY_CAP
tempfile->>error: raise error
error->>download: cleanup & return nil
else within cap
tempfile->>extract: flush & rewind
extract->>download: return extracted paths
download->>fetch: return result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fef4bdcd61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 41: Update the CHANGELOG.md entry that starts with "HTTP rolling-deploy
adapter now streams bundle downloads with a compressed-size cap" to append the
required PR/author suffix in the format "[PR ####](link) by [username](link)";
locate the line containing that exact entry text and add the PR number and
author links at the end of the sentence so the entry matches the repository
changelog formatting guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1726acaa-2e02-4242-95d5-c2b37f5b009d
📒 Files selected for processing (3)
CHANGELOG.mdreact_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rbreact_on_rails_pro/spec/react_on_rails_pro/rolling_deploy_adapters/http_spec.rb
Greptile SummaryThis PR replaces the buffered
Confidence Score: 5/5Safe to merge. The streaming + cap logic is correct, cleanup is guaranteed by Tempfile.create's ensure, and both new code paths are covered by specs. The buffered-to-streaming migration is well-contained. Tarball.extract already handles both String and IO sources, so the Tempfile argument to extract_payload is a safe drop-in. The COMPRESSED_BODY_CAP is checked before writing each chunk, keeping the tempfile always below the cap. Non-local returns and mid-stream exceptions both trigger Tempfile cleanup correctly. No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "Prevent rolling deploy download heap exh..." | Re-trigger Greptile |
fef4bdc to
da7b7af
Compare
Mattered
Optional
Discuss
Skipped
Scan window: full PR history, because no previous |
Summary
Fixes #3416.
Pull Request checklist
Update documentationTest plan
cd react_on_rails_pro && bundle exec rspec spec/react_on_rails_pro/rolling_deploy_adapters/http_spec.rbcd react_on_rails_pro && bundle exec rubocop --ignore-parent-exclusioncorepack pnpm exec prettier --check CHANGELOG.mdgit diff --checkReview notes
/bundles/:hashbodies was valid. The adapter now drains those responses through the same compressed-byte cap while discarding chunks, preventing Net::HTTP from materializing an oversized error body after the request block exits.Note
Low Risk
Localized refactor of Pro rolling-deploy HTTP streaming with added tests; no auth or public API surface changes beyond safer response handling.
Overview
Pro HTTP rolling-deploy bundle download handling now routes all streamed bodies through a shared
each_capped_body_chunkhelper so the 50 MB compressed cap applies whether the response is saved or discarded.On non-success
GET /bundles/:hashresponses, the adapter drains the body through that cap (viadrain_response_body) instead of stopping without consuming the stream—so oversized error payloads still trip the same abort/cleanup path as successful downloads. Success paths still write chunks to the Tempfile throughstream_response_body.CHANGELOG adds the PR attribution for the streaming/cap fix. Specs cover 404 + oversized body draining and cap enforcement.
Reviewed by Cursor Bugbot for commit 01ddf15. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
Tests
Documentation