Fix StreamResponse status fallback for non-streaming errors by justin808 · Pull Request #2416 · shakacode/react_on_rails

justin808 · 2026-02-15T07:53:54Z

Summary

Fix StreamRequest#process_response_chunks to treat responses as errors when status delegation raises NoMethodError
Avoid masking the original HTTPX error path when a StreamResponse cannot answer #status
Add a regression spec that exercises the missing-status delegation path

Closes #2408

Test plan

Proposed fix (UNTESTED in this environment)
Attempted: bundle exec rspec react_on_rails_pro/spec/react_on_rails_pro/stream_request_spec.rb
Blocked by local Ruby version (2.6.10) vs project requirement (>= 3.0.0)

Summary by CodeRabbit

Bug Fixes
- Improved error handling for streaming responses to properly detect and handle error conditions in edge cases.
Tests
- Added test coverage for streaming response error scenarios.

coderabbitai · 2026-02-15T07:54:10Z

Walkthrough

Added a helper method response_has_error_status? to safely check error status in HTTPX responses, handling NoMethodError when status delegation is unavailable. Updated process_response_chunks to use this helper instead of an inline condition.

Changes

Cohort / File(s)	Summary
Error Status Detection `react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb`	Extracted `response_has_error_status?` helper method to check for error responses (HTTPX::ErrorResponse or status >= 400) while safely rescuing NoMethodError. Updated `process_response_chunks` to use this helper.
Streaming Response Test `react_on_rails_pro/spec/react_on_rails_pro/stream_request_spec.rb`	Added test case for `process_response_chunks` with a mocked response that raises NoMethodError when status is accessed, verifying error body capture and no chunk emission.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A streaming response slipped through the night,
With status lost in delegation's plight.
But now a helper catches the error with care,
Rescuing bodies floating in air. 🐰✨

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 directly matches the main code change: adding a status fallback helper to handle StreamResponse errors when status delegation fails.
Linked Issues check	✅ Passed	The code changes fully implement the proposed fix from issue `#2408`: a response_has_error_status? helper rescues NoMethodError and treats responses as errors when status is unavailable, with regression test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the issue: extracting a status check helper and adding a regression test for the missing-status delegation path.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into `master`

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

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch codex/fix-2408-streamresponse-status

No actionable comments were generated in the recent review. 🎉

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.}

greptile-apps · 2026-02-15T07:56:16Z

Greptile Summary

Fixes error handling in StreamRequest#process_response_chunks by extracting status check logic into a defensive response_has_error_status? method that rescues NoMethodError when HTTPX::StreamResponse cannot delegate #status for non-streaming errors.

Prevents masking of original HTTPX errors when status delegation fails
Adds comprehensive regression test validating the NoMethodError path
Ensures error body is properly collected and no chunks are yielded to the caller when status check fails

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The fix is a focused defensive code change that adds proper error handling for a known edge case (NoMethodError on status delegation). The implementation is minimal, well-tested with a regression spec, and follows the existing error handling pattern in the codebase.
No files require special attention

Important Files Changed

Filename	Overview
react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb	Adds defensive `NoMethodError` rescue in `response_has_error_status?` to handle HTTPX::StreamResponse status delegation failures
react_on_rails_pro/spec/react_on_rails_pro/stream_request_spec.rb	Adds regression test for NoMethodError on status delegation, validates error body collection and no chunk yielding

Flowchart

flowchart TD
    A[process_response_chunks called] --> B{response_has_error_status?}
    B --> C{is HTTPX::ErrorResponse?}
    C -->|Yes| D[Return true - collect error body]
    C -->|No| E{response.status >= 400?}
    E -->|Yes| D
    E -->|No| F[Return false - yield chunk]
    E -->|NoMethodError| G[Rescue NoMethodError]
    G --> D
    D --> H[Append chunk to error_body]
    F --> I[Yield processed chunk to caller]

_{Last reviewed commit: 2284910}

justin808 · 2026-02-15T08:08:51Z

@AbanoubGhadban @ihabadham @alexeyr-ci2 This was done independently by codex.

Add entries for user-visible changes since v16.4.0.rc.6: - #2539: env-var-driven ports in Procfile templates - #2417: rspack generator config path fix - #2419: precompile hook load-based execution fix - #2577: create-react-on-rails-app validation improvements - #2416: StreamResponse status fallback fix (Pro) - #2566: empty-string license plan mismatch fix (Pro) - Updated #2561 entry to include #2568 contributor credit Co-Authored-By: Claude Opus 4.6 <[email protected]>

## 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)  --- > [!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>  Co-authored-by: Claude Opus 4.6 <[email protected]>

Handle stream error responses when HTTPX status delegation fails

2284910

coderabbitai Bot approved these changes Feb 15, 2026

View reviewed changes

justin808 requested a review from AbanoubGhadban February 15, 2026 08:07

Retrigger Claude Code Review

a1d2ffe

justin808 added codex PRs created from codex-named branches release:16.4.0-must-have Must-have for 16.4.0: critical bug/perf/usability labels Feb 25, 2026

justin808 added the P1 Target this sprint label Mar 5, 2026

justin808 self-assigned this Mar 5, 2026

justin808 mentioned this pull request Mar 8, 2026

Release 16.4.0 Triage: PRs, Issues, and Prioritization #2560

Closed

justin808 merged commit 34e8c45 into master Mar 8, 2026
23 checks passed

justin808 deleted the codex/fix-2408-streamresponse-status branch March 8, 2026 07:54

justin808 added the bug label Mar 8, 2026

justin808 mentioned this pull request Mar 9, 2026

Add missing changelog entries for unreleased PRs #2578

Merged

2 tasks

This was referenced Mar 17, 2026

Improve create/install validation flows and migration docs #2650

Merged

Docs: Add RSC landing, ExecJS limitations, debugging, and benchmarks pages #2785

Merged

Raise docs version floor to 16.4.0 in install/demo guidance #2610

Merged

This was referenced Mar 26, 2026

Add RSC Flight payload optimization guide (Article 7) #2827

Merged

Eliminate double JSON.stringify in RSC payload embedding #2835

Merged

feat: Add automatic license renewal for gem and Node Renderer #2254

Closed

This was referenced Mar 28, 2026

Add evaluator comparison docs path #2651

Merged

Revert "Eliminate double JSON.stringify in RSC payload embedding (#2835)" #2878

Merged

chore: security audit cleanup — CodeQL, workflow permissions, dependency overrides #2884

Merged

This was referenced Apr 18, 2026

docs: address RSC pitfalls review follow-ups (#3155) #3156

Merged

refactor: unify renderer cache staging with mode: :copy / :symlink #3167

Merged

Fix Bencher reporting permanently broken on pushes to main #3148

Merged

This was referenced May 4, 2026

feat: add REACT_ON_RAILS_BASE_PORT for concurrent worktree port management #3142

Open

feat: rolling_deploy_adapter for seeding previous bundle hashes #3173

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix StreamResponse status fallback for non-streaming errors#2416

Fix StreamResponse status fallback for non-streaming errors#2416
justin808 merged 2 commits intomasterfrom
codex/fix-2408-streamresponse-status

justin808 commented Feb 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Feb 15, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Feb 15, 2026

Uh oh!

justin808 commented Feb 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

justin808 commented Feb 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

greptile-apps Bot commented Feb 15, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

justin808 commented Feb 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

justin808 commented Feb 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Feb 15, 2026 •

edited

Loading