Skip to content

Fix StreamResponse status fallback for non-streaming errors#2416

Open
justin808 wants to merge 2 commits intomasterfrom
codex/fix-2408-streamresponse-status
Open

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

Conversation

@justin808
Copy link
Member

@justin808 justin808 commented Feb 15, 2026

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
Copy link
Contributor

coderabbitai bot commented Feb 15, 2026

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
Copy link

greptile-apps bot commented Feb 15, 2026

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]
Loading

Last reviewed commit: 2284910

@justin808
Copy link
Member Author

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

@justin808 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex PRs created from codex-named branches release:16.4.0-must-have Must-have for 16.4.0: critical bug/perf/usability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix NoMethodError on HTTPX::StreamResponse.status for non-streaming error responses

1 participant