httpx: Should error responses raise an HTTPError by default?

I raised this question on the chat, but I think it’s worth a proper issue to discuss options…

Current situation and pain points

Currently, to be notified of an error response we need to either:

Check the .status_code manually:

if response.status_code == 404:
    print("Not found")

if response.is_error:
    print("Error")

Call .raise_for_status():

response.raise_for_status()  # Raises an HTTPError

This is mostly in line with what Requests does, but I’d argue we can do better.

In particular, there’s a very basic use case we could address better: make a request, then handle any 4xx/5xx.

There are mostly two ways to do this currently:

Inspect the .status_code:

import httpx

r = httpx.get("https://httpbin.org/status/500")
if r.status_code == 500:
    ...

Call .raise_for_status() to get a proper HTTPError:

import httpx

def fetch(url):
    r = httpx.get(url)
    r.raise_for_status()
    return r

try:
    r = fetch("https://httpbin.org/status/500")
except httpx.HTTPError as exc:
    if exc.response.status_code == 500:
        ...

I think we can argue this looks somewhat clunky. It forces the user to add a .raise_for_status() line basically everywhere — probably as a wrapper around an HTTPX call — otherwise they’d start processing responses they think are valid, when they’re actually errors.

Possible future situation

What if request methods automatically raised an HTTPError if we get an error response? Sounds sensible?

import httpx

try:
    r = httpx.get("https://httpbin.org/status/500")
except httpx.HTTPError as exc:
    print(exc.response.status_code)
    # Deal with 500...

This would allow users to have to think about error handling, as they’d be getting HTTPError exceptions thrown at them when hosts aren’t able to satisfy the request. We can also easily add a “manual mode” without breaking backwards compat after 1.0…

Planning

If we think this is reasonable, the way I’d see us implement this is:

Switch to raise an error as the default behavior before 1.0.
This would be done via a raise_for_status=True flag, probably on the client constructor at first.
Switch the docs to document the new default behavior, and document raise_for_status=False + response.raise_for_status/response.is_error as a more low-level/manual way of handling things.
Later on (i.e. after 1.0), we could extend this to allow a Callable[[int], bool] too, allowing to do things like raise_for_status=lambda s: 500 <= s < 600 so that 4xx error responses don’t raise an HTTPError. (Sounds fancy, but I can see this feature request turning up at some point.)

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 4
Comments: 19 (15 by maintainers)

Most upvoted comments

TL;DR:

Client(event_hooks={'response': [Response.raise_for_status]}

client.event_hooks['response'].append(Response.raise_for_status)

You don’t have to declare raise_on_4xx_5xx() like in docs example.

Bobronium on May 12, 2022

I cannot understand why you backed off your “ready for production” pull request? It feels like an intuitive approach using AsyncClient(raise_for_status=True), and helps to clear a lot of DRY code.

Even the competitors let the user choose https://docs.aiohttp.org/en/stable/client_reference.html …

I don’t think It will bother someone adding a raise_for_status at the False default state and let the user choose.

If you don’t have time, I can volunteer and do that, but I see the PR is already written and closed.

emil45 on Feb 2, 2021

I might leave response.raise_for_status() for requests compatibility, but the flag would be raise_on_error_status=, since it’s a diversion from requests’ interface, no need to remain compliant.

I’d say they should both be the same if a flag is added. It just adds unnecessary confusion otherwise; things are usually named differently because they do at least slightly different things. If HTTPX deviates from requests in this at all, it should probably be to raise without bothering with a flag/method. Or at least have that as a future design goal.

thebigmunch on Jan 14, 2020

Would it be possible, or sensical, to have a unique Exception for HTTP Status Code Errors? This is somewhat orthogonal to the specific request, and may make sense to be a separate issue. For example:

try:
    r = httpx.get("https://httpbin.org/status/404")
    r.raise_for_status()
except httpx.HTTPError as error:
   print(error.response.status_code)

That’ll work great and tell you it got a 404 error…unless it timed out.

try:
    r = httpx.get("https://httpbin.org/status/404", timeout=0.0001)
    r.raise_for_status()
except httpx.HTTPError as error:
   print(error.response.status_code)

That’ll raise an AttributeError because a timeout doesn’t have a response, and a NoneType can’t have a status_code! You can pretty easily inspect for response is not None or something like that, but it’d also be nice if you could do something like this:

try:
    r = httpx.get("https://httpbin.org/status/404", timeout=0.0001)
    r.raise_for_status()
except httpx.HTTPStatusError as error:
   # class HTTPStatusError(HTTPError):
   print(error.response.status_code)
except httpx.TimeoutException as error:
   print("Connection Timed Out!")

iwoloschin on Jan 15, 2020

Seems like https://www.python-httpx.org/advanced/#event-hooks fully solves this issue, doesn’t it?

oprypin on Mar 6, 2021

So, I think we’re starting to settle down as a “no” on this? Or at least as something not crucial/possible to add incrementally anytime in the future, such as a flag to turn raise-on-error-response on… So, closing this for now…

As for differentiating between non-response errors, and response errors, we can still do…

try:
    r = httpx.get("https://httpbin.org/status/404")
except httpx.HTTPError as exc:
    print(f"Something happened while getting the response: {exc!r}")
else:
    r.raise_for_status()  # Let error responses bubble up.

florimondmanca on Jan 20, 2020