raise_for_status does not raise for redirects

According [to the code](https://github.com/kennethreitz/requests/blob/master/requests/models.py#L648) it no longer raises for redirects, this used to be an option called `allow_redirects`. 

Now it doesn't raise for 3xx responses but [the docs say](http://docs.python-requests.org/en/latest/user/quickstart/#response-status-codes) :

> If we made a bad request (non-200 response), we can raise it with  Response.raise_for_status():


I contend the docs are poorly phrased. Redirects are expected and a natural part of HTTP, and they should not cause exceptions.


The better doc would be:

> If we made a bad request (4xx response), we can raise it with Response.raise_for_status():

We might want to include 5xx responses there too, though they aren't strictly our fault.


> I contend the docs are poorly phrased. Redirects are expected and a natural part of HTTP, and they should not cause exceptions.

The old behavior of `allow_redirects` covered this quite nicely I thought.


If you're going to throw exceptions on redirects, why can't you set `allow_redirects` to `False` on the original HTTP request?


Hmm. Actually, I think I'm -0.5 on reintroducing the old behaviour. It doesn't cost much and doesn't clutter the API much. We're in [feature freeze](http://docs.python-requests.org/en/latest/dev/todo/#feature-freeze), but if @kennethreitz decides to we might put this back. I'm leaving this for him. =)


Thanks for pointing out `allow_redirects` on the request. That's probably suitable for us too! I'm happy with there being _one_ way to do this.


@mjallday well there should only be one obvious way to do it. ;)


not 100% obvious. `allow_redirects` doesn't disallow, it only stops you [from _following_ redirects](https://github.com/kennethreitz/requests/blob/master/requests/sessions.py#L426), so the old behavior was to throw an exception, now I need to look for an exception and raise it myself if I have a status code in `REDIRECT_STATI`.

IMO the better suggestion would be to either allow me to override the `raise_for_status` method with the HTTP codes that are un-acceptable to me or make it a class variable e.g. `Response.RAISE_FOR_STATUS_STATI` (tho with less retarded naming)


Ah I see the issue. @Lukasa's comment could be losely misunderstood as meaning setting `allow_redirects` to False would raise an exception when a redirect was found. A pattern that could work for you in it's stead is this:

``` python
r = session.request(method, url, # etc.
                    allow_redirects=False
                    )

r.raise_for_status() # Will catch 400...500 error codes
if r.status_code >= 300:
    raise RedirectFoundError(r)

return r
```

Where I'm making an assumption that you're returning the response without any evidence of that ;)


@sigmavirus24 you've implemented pretty much exactly what I have on my screen right now!

Feel free to close this ticket if you don't think the variable naming is misleading, my immediate concern is addressed.


@mjallday I'm psychic but don't tell anyone. ;)

Yeah, I think I might re-ticket this to see if we should raise exceptions for stati >= 500 too though. That seems like something worth discussing more.


@sigmavirus24: [We already do](https://github.com/kennethreitz/requests/blob/master/requests/models.py#L656).


> We might want to include 5xx responses there too, though they aren't strictly our fault.

Made me think we didn't.


Yeah, I wrote that comment before I actually checked the code.


On Sat, Mar 02, 2013 at 06:28:48AM -0800, Cory Benfield wrote:

> Yeah, I wrote that comment before I actually checked the code.

Yeah and I should have checked too ;)