Improving exception handling, HTTPError is misleading.

Hi,

HTTPError is the base class exception, it have HTTP while it also handle not HTTP issue, like Connection issue (NetworkError, ...) and so on.

On the other side, we have no exception defined for real HTTP client side error (4XX) and HTTP service side error (5XX).

For exemple, requests have ConnectionError (no status_code) and HTTPError (with status_code), both inherit from RequestException.
aiohttp offers even more precise exceptions, all HTTP codes have an associated exceptions:

The code end up often with ton of thing like this:

try:
   ...
exception httpx.HTTPError as e:
     if e.response and e.reponse.status_code:
         if e.reponse.status_code == 404:
             ... handle my expected error
         elif e.reponse.status_code == 422:
             ... handle my expected error
     raise

I think something like this would be clearer :

try:
   ...
exception httpx.HTTPNotFound as e:
     ... handle my expected error
exception httpx.HTTPUnprocessableEntity as e:
     ... handle my expected error

I will be happy to add this exception handling, are your interested in contribution like this ?

Thanks for opening this! It's a tricky topic.

There's a major piece of background info in an existing issue, where we discussed whether 4xx's and 5xx's should raise exceptions: #752. The motivation was close (but not as specific) as the one you're sharing here. Basically the idea would be to add an extra layer of abstraction to be able to work at the HTTP level instead of the response level.

One improvement suggested in #752 (comment) was to introduce a specific HTTPStatusError. Provided we keep .raise_for_status(), this would be a somewhat compatible with Requests, at least not completely breaking:

try:
    r = httpx.get("https://httpbin.org/status/404", timeout=0.0001)
    r.raise_for_status()
except httpx.HTTPStatusError as error:
    if error.response.status_code == 404:
        ...  # Handle my expected 404 error
    elif error.response.status_code == 503:
        ...  # Handle my expected 503 error
    else:
        raise
except httpx.TimeoutException as error:
    print(error) # Has no `.response` attribute

TBH I'm tempted to like this idea. :-)

@sileht

@sileht For the snippet you showed here:

I think something like this would be clearer

This wouldn't be feasible because we can't sensibly have one exception class for each HTTP status code. Which is why I'm tempted to think that an HTTPStatusError might make sense.

Another interesting point you highlighted: is HTTPError a good name for a base exception class? Couldn't it be…

httpx.HTTPXError? (too redundant? isn't this too similar visually to HTTPError?)
httpx.RequestError (after all, we can only have exceptions when making an HTTP request?)
httpx.Error (too generic?)

Of these three, httpx.RequestError is the one I'd prefer.

So, 2 possible (maybe?) points of action here:

Introduce httpx.HTTPStatusError (if that name sounds okay).
Rename httpx.HTTPError to httpx.RequestError (or whatever we'd agree on).

@tomchristie

Interesting in hearing thoughts from @tomchristie, @StephenBrown2 or others about this. :-) I think there's definitely room for improvement.

I like httpx.HTTPStatusError, I think this is the minimum to do.

I also prefer httpx.RequestError instead of httpx.HTTPError, too.

This wouldn't be feasible because we can't sensibly have one exception class for each HTTP status code. Which is why I'm tempted to think that an HTTPStatusError might make sense.

We already maintain all status in _status_codes.py, we just need to generate all exceptions base on this list. Obvious all theses exceptions will inherit from a base HTTPStattusError.

Rename httpx.HTTPError to httpx.RequestError

This makes sense to me, but I guess we will have to keep HTTPError for compatibility purpose ?

I like httpx.HTTPStatusError, I think this is the minimum to do.

I also like httpx.HTTPStatusError, but I think it's probably reasonable to leave it at that.

This wouldn't be feasible because we can't sensibly have one exception class for each HTTP status code. Which is why I'm tempted to think that an HTTPStatusError might make sense.

We already maintain all status in _status_codes.py, we just need to generate all exceptions base on this list. Obvious all theses exceptions will inherit from a base HTTPStattusError.

That would be how it would work, but I don't believe any other http library implements errors this way. It might provide slightly better convenience, but often one wants to respond to a class of errors, e.g. 40X errors or 50X errors, which one could still do.

IMO:

try:
    r = httpx.get("https://httpbin.org/status/404", timeout=0.0001)
    r.raise_for_status()
except httpx.HTTPStatusError as error:
    if error.response.status_code == 404:
        ...  # Handle my expected 404 error
    elif error.response.status_code == 503:
        ...  # Handle my expected 503 error
    else:
        raise
except httpx.TimeoutException as error:
    print(error) # Has no `.response` attribute

is just as readable as:

try:
    r = httpx.get("https://httpbin.org/status/404", timeout=0.0001)
    r.raise_for_status()
except httpx.HTTP404StatusError as error:
    ...  # Handle my expected 404 error
except httpx.HTTP503StatusError as error:
    ...  # Handle my expected 503 error
except httpx.TimeoutException as error:
    print(error) # Has no `.response` attribute

or better:

def handle_http_error(err):
	status = err.response.status_code
    if status == 404:
        ...  # Handle my expected 404 error
    elif status == 503:
        ...  # Handle my expected 503 error
	elif 400 <= status < 500:
		...  # Handle a range of 400 errors
	elif 500 <= status < 600:
		...  # Handle a range of 500 errors
    else:
        raise err

try:
    r = httpx.get("https://httpbin.org/status/404", timeout=0.0001)
    r.raise_for_status()
except httpx.HTTPStatusError as error:
	handle_http_error(error)
except httpx.TimeoutException as error:
    print(error) # Has no `.response` attribute

Also, how would one name the errors? I used the status code, but they technically have wordy names. Do we use both? Just words?

I also prefer httpx.RequestError instead of httpx.HTTPError, too.

Rename httpx.HTTPError to httpx.RequestError

This makes sense to me, but I guess we will have to keep HTTPError for compatibility purpose ?

I agree with renaming httpx.HTTPError to httpx.RequestError and keeping HTTPError as an alias of RequestError for backwards compatibility, unless we're less concerned about that pre-1.0, I believe the base class has changed names once or twice already.

feat: introduce HTTPStatusError

feat: rename HTTPError to RequestError

feat: introduce HTTPStatusError

feat: rename HTTPError to RequestError

feat: introduce HTTPStatusError

feat: rename HTTPError to RequestError

Look like some consensus starts to appear for HTTPStatusError, so I started #869

That would be how it would work, but I don't believe any other http library implements errors this way. It might provide slightly better convenience, but often one wants to respond to a class of errors, e.g. 40X errors or 50X errors, which one could still do.

I only known aiohttp doing that, requests, urllib3, httpclient doesn't.

Also, how would one name the errors? I used the status code, but they technically have wordy names. Do we use both? Just words?

Using status code as name doesn't make sense to me as it doesn't add value compared as using response.status_code == 422

Only something like httpx.HTTPNotFound httpx.HTTPUnprocessableEntity, httpx.HTTPNotImplemented, etc... makes more sense to me.

Introducing all these exceptions will be neat, but not indispensable.

feat: introduce HTTPStatusError

feat: rename HTTPError to RequestError

feat: introduce HTTPStatusError

feat: rename HTTPError to RequestError

Closing in favor of #1064. We can discuss the httpx.HTTPStatusError idea in a separate issue too. :-) Thanks all!

Navigation Menu

Repository navigation

Improving exception handling, HTTPError is misleading. #867

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Improving exception handling, HTTPError is misleading. #867

Description

Activity

florimondmanca commented on Mar 18, 2020

florimondmanca commented on Mar 18, 2020

florimondmanca commented on Mar 18, 2020

sileht commented on Mar 19, 2020

StephenBrown2 commented on Mar 19, 2020

sileht commented on Mar 21, 2020

florimondmanca commented on Jul 16, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions