Skip to content

Conversation

@him2994
Copy link

@him2994 him2994 commented Aug 16, 2025

Summary

This PR enhances the Response.raise_for_status() method to detect when redirect responses (3xx status codes) automatically should not raise exceptions. When a request is made with follow_redirects=False, HTTPX sets response.next_request for redirect responses, and raise_for_status() now intelligently detects this and treats redirects as successful responses instead of raising HTTPStatusError.
This change addresses the inconsistency where developers had to manually check response.next_request or implement custom logic to handle redirects when follow_redirects=False was set on the request.

Current Behaviour

  • raise_for_status() always raises HTTPStatusError for 3xx redirect responses
  • No way to control redirect handling behaviour in the method
  • Developers must implement workarounds or check response.next_request manually
  • Inconsistent with the principle of least surprise

Expected Behaviour

  • raise_for_status() automatically detects when redirects shouldn't raise exceptions
  • When follow_redirects=False is set on the request, redirect responses are treated as successful
  • No need for developers to implement custom redirect handling logic
  • Consistent behaviour that aligns with request configuration

Changes Made

- httpx/_models.py: Modified raise_for_status() method to check for next_request presence and return early for redirects when follow_redirects=False

  • tests/models/test_responses.py: Added test case to verify that redirect responses don't raise exceptions when follow_redirects=False
  • docs/quickstart.md: Updated documentation to explain the new redirect handling behaviour with clear examples

Checklist

[x] I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
[x] I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
[x] I've updated the documentation accordingly.

@him2994 him2994 changed the title Follow redirects support in raise for status Add follow_redirects parameter to Response.raise_for_status() method Aug 16, 2025
@him2994 him2994 changed the title Add follow_redirects parameter to Response.raise_for_status() method Update Response.raise_for_status() to respect follow_redirects parameter Aug 16, 2025
@Zaczero
Copy link
Contributor

Zaczero commented Aug 18, 2025

I think this is a breaking change. It also doesn't feel right to treat 3xx status codes as successful responses; only 2xx codes indicate success. Perhaps extending raise_for_status to accept a boolean argument like allow_redirects=True would be better, in my opinion? P.S. I am just a random user; I may be completely wrong.

@him2994
Copy link
Author

him2994 commented Aug 18, 2025

@Zaczero
You're correct that 3xx codes don't indicate success - they indicate redirection. However, the issue here is about intent vs. status:

  • When a developer sets follow_redirects=False, they're explicitly saying "I want to handle redirects manually"
  • In this context, a 3xx response is expected and desired behaviour, not an error condition
  • The current implementation forces developers to either:
    • Catch and ignore the exception (awkward)
    • Check response.next_request before calling raise_for_status() (inconsistent).
    • Skip raise_for_status() entirely (loses error checking for actual errors)

This is NOT a breaking change because the new behaviour only kicks in when developers explicitly opt in by setting follow_redirects=False on their requests, and all the current code continues to work identically.

I chose this approach because the behaviour automatically matches the request's redirect handling preference.

Thanks for looking into this.
Let me know your thoughts, and I can work on refining the approach further.

@Zaczero
Copy link
Contributor

Zaczero commented Aug 19, 2025

Hey!

When a developer sets follow_redirects=False, they're explicitly saying "I want to handle redirects manually"

follow_redirects=False is the default, so my understanding is that the intent is "I want to handle ready-to-use responses." Redirect responses do not contain actual data and are not ready-to-use.

Check response.next_request before calling raise_for_status() (inconsistent).

In the past I simply checked for redirects before raise_for_status and it worked okay (I used the .is_redirect property). What's inconsistent?

This is NOT a breaking change because the new behaviour only kicks in when developers explicitly opt in by setting follow_redirects=False on their requests, and all the current code continues to work identically.

follow_redirects: bool = False is the default when unset in AsyncClient's and Client's __init__.

@him2994
Copy link
Author

him2994 commented Aug 20, 2025

@Zaczero

Thanks for the Correction
You've saved me from implementing a feature that would have made the API worse, not better. The current behaviour is correct, and your approach of checking response.is_redirect is the right way to handle this.

I'd like to rethink whether there's a problem here that needs solving, or if the current API is already working as intended.

For now, closing the PR.

@him2994 him2994 closed this Aug 20, 2025
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.

2 participants