Exceptions - Documentation, Hierarchy & Naming. #949

tomchristie · 2020-05-13T12:05:01Z

We ought to comprehensively revisit our exceptions for 1.0, and make sure we're super happy we've got a public API we're happy to stick by for 1.0.

Naming.
The hierarchy.
Decent documentation.
.request/.response attributes (can we make either of these mandatory in some cases?)

Transport Errors, mapped from `httpcore`

We're currently synonym'ing these, but we might want to properly map them instead, so we can subclass our base class, and so that we can include a mandatory request=... parameter.

Also, we might want to additionally subclass the stdlib TimeoutError for the timeout types.

ProtocolError
ProxyError
TimeoutException Exists in httpcore, but not in httpx
- PoolTimeout
- ConnectTimeout
- ReadTimeout
- WriteTimeout
NetworkError
- ConnectError
- ReadError
- WriteError
- CloseError

Content Errors

DecodingError

Redirect Errors

RedirectError
- TooManyRedirects
- NotRedirectResponse

Stream Errors

We might want to additionally subclass the stdlib RuntimeError for (some of?) these types.

It's actually possible that we might want to just drop some of these from the API completely, and instead use RuntimeError, since not all of these ought to actually be handled exception cases, but are instead unconditionally a "the developer has broken something" case.

StreamError
- RequestBodyUnavailable
- StreamConsumed
- ResponseNotRead
- RequestNotRead
- ResponseClosed

Other cases...

InvalidURL
CookieConflict

httpx exceptions does not inherit from HTTPError anymore. This may break again in the future... encode/httpx#949 So this change: * removes the httpx monkeypatching * put all http related error in clients/http.py * adds catch HTTP Connection related Exception again The rest of the code does not import httpx for caching exception anymore. Fixes MERGIFY-ENGINE-1KZ MERGIFY-ENGINE-1KY

johnanthonyowens · 2020-05-31T04:43:00Z

+1 in favor of mapping the httpcore exceptions to a unified hierarchy of httpx exceptions. I have a library that I recently updated from httpx 0.12 to 0.13 and found that getting the equivalent of except HTTPError was tricky due to the fragmented exception hierarchy.

MicahLyle · 2020-06-01T21:24:59Z

+1 in favor of mapping the httpcore exceptions to a unified hierarchy of httpx exceptions. I have a library that I recently updated from httpx 0.12 to 0.13 and found that getting the equivalent of except HTTPError was tricky due to the fragmented exception hierarchy.

I've also run into the same thing. Before, I know that catching HTTPError would also catch TimeoutException. Now, a test case indicated that this isn't the case, and it seems like I'd have to catch a number of exceptions to reproduce the same previous behavior, as @johnanthonyowens said.

In general, it would be nice to have some sort of exception where I know that catching that exception would catch all httpx exceptions and underlying httpcore exceptions. My project is trio-based, where it's really important that certain exceptions are caught and handled properly (or ignored but clearly caught), otherwise other tasks may cancel.

Edit: I saw this comment #884 (comment) which reassures me that catching HTTPError should catch those underlying timeout errors?

iwoloschin · 2020-06-12T11:22:02Z

I just upgraded a project from httpx 0.12 to 0.13 and found a few things broke since HTTPError is not really the "Base class for all HTTPX exceptions" anymore. I assume this is a bug? I'm fine to drop back to 0.12 for the time being, and I'd rather do that than go find all of the cases in my code where catching HTTPError is no longer sufficient.

Also, it seems that I'm getting httpcore exceptions being raised, not httpx exceptions. I assume this is also a bug?

florimondmanca · 2020-06-15T18:46:38Z

@iwoloschin As per #884 (comment) yes, any exception leaking from httpcore not wrapped by HTTPX should be considered a bug, which is also what this issue is about.

florimondmanca · 2020-06-15T18:48:25Z

I'm labelling this as "bug" as well since obviously currently the "HTTPX may raise exceptions that aren't mapped under its base class" is being considered a problem now and should be addressed by reviewing the exception hierarchy.

I'm also wondering if we'd need a "base exception" in httpcore as well (we're subclassing from Exception for most cases there), because it would have made a workaround such as "catch httpx.HTTPError and httpcore.RequestError [or something]" possible.

martinka · 2020-06-18T12:23:37Z

I think its is probably related. the AsyncClient.get method current throws httpcore._exceptions.ReadTimeout rather than an exception that should be exposed. ( assuming _exceptions is supposed to be private).

tomchristie · 2020-07-21T11:41:13Z

I've been reviewing this lately, and I think I've got a clear idea how we could improve things a little.
Here's our current heirarchy...

HTTPError
- TimeoutException ☩
  - ConnectTimeout ☩
  - ReadTimeout ☩
  - WriteTimeout ☩
  - PoolTimeout ☩
- NetworkError ☩
  - ConnectError ☩
  - ReadError ☩
  - WriteError ☩
  - CloseError ☩
- ProxyError ☩
- ProtocolError ☩
- HTTPStatusError - Occurs during .raise_for_status
- DecodingError ✧
- RedirectError
  - TooManyRedirects ✧
  - NotRedirectResponse - Occurs if calling .next() without properly checking .is_redirect_response
- StreamError
  - RequestBodyUnavailable ✧
  - StreamConsumed - Occurs if attempting to iterate over the stream twice
  - ResponseNotRead - Occurs if accessing .content without having .read() the stream
  - RequestNotRead - Occurs if accessing .content without having .read() the stream
  - ResponseClosed - Occurs if attempting to read the stream after response is already closed
- InvalidURL ✧
- CookieConflict - Can occur with response.cookies.get(...)

☩: Maps with httpcore. Can occur during a .request.
✧: Can occur during a .request.

An important thing to note here is that there's some places where we're drawing the heirarchy along semantic lines, rather than behavioural lines. Ie. both TooManyRedirects and NotRedirectResponse are subclassed as a RedirectError, but behaviourally they're actually entirely different categories - the first cannot be pre-empted by the developer, and depends on the response, the second is the result of a code error. If it's raised it means your code is borken - it isn't ever something you'd want to treat in a catch-and-handle block.

With that in mind, here's how an alternate hierarchy could look...

RequestError
- TimeoutException ☩
  - ConnectTimeout ☩
  - ReadTimeout ☩
  - WriteTimeout ☩
  - PoolTimeout ☩
- NetworkError ☩
  - ConnectError ☩
  - ReadError ☩
  - WriteError ☩
  - CloseError ☩
- ProxyError ☩
- ProtocolError ☩
- DecodingError ✧
- TooManyRedirects ✧
- RequestBodyUnavailable ✧
- InvalidURL ✧
HTTPStatusError - Occurs during .raise_for_status
NotRedirectResponse - Occurs if calling .next() without properly checking .is_redirect_response
CookieConflict - Can occur with response.cookies.get(...)
StreamError
- StreamConsumed - Occurs if attempting to iterate over the stream twice
- ResponseNotRead - Occurs if accessing .content without having .read() the stream
- RequestNotRead - Occurs if accessing .content without having .read() the stream
- ResponseClosed - Occurs if attempting to read the stream after response is already closed

Note that in this case we do now have a RequestError class, which encompasses anything that could be raised during a .request() operation. We don't have a general purpose base-of-everything class, since it's not actually a useful thing to catch against.

We can be a bit more precise about exc.request and exc.response in this hierarchy too, and have request= be mandatory for RequestError, and have both request= and response= be mandatory for HTTPStatusError, NotRedirectResponse and CookieConflict. We probably just omit them on the StreamError cases, since those are all the result of programmatic bugs, rather than useful catch-and-handle cases.

There's a couple of minor tweaks we might need beyond that. We'd possibly need a URLParseError that isn't associated with a request, which can be raised if URL() is called directly. Not 100% sure about that yet. We'd probably also need to distinguish two separate classes an internal-only DecodingError which isn't associated with a request, and is raised by the decoder instances, and an external ResponseDecodingError, which would have mandatory request and response arguments, and is what is exposed to the end-user.

tomchristie · 2020-07-27T11:13:06Z

I'm milestoning this as 0.14.

That might be a push, I'm not sure, but the rationale for doing so is that post 0.14 everything else we've got left on the table is generally behavioural polishing (eg. charset handling, dropping HSTS, HTTP/2 as optional) rather than API changes.

I'd be okay with us dropping it from the milestone if needed.

florimondmanca · 2020-07-27T11:42:17Z

I agree that we most likely want this for 0.14. :-)

tomchristie · 2020-08-02T11:32:15Z

After encode/httpcore#128 and encode/httpcore#129 we're looking at this...

RequestError
- TransportError
  - TimeoutException
    · ConnectTimeout
    · ReadTimeout
    · WriteTimeout
    · PoolTimeout
  - NetworkError
    · ConnectError
    · ReadError
    · WriteError
    · CloseError
  - ProtocolError
    · LocalProtocolError
    · RemoteProtocolError
  - ProxyError
  - UnsupportedProtocol
- DecodingError
- TooManyRedirects
- RequestBodyUnavailable
HTTPStatusError
NotRedirectResponse
CookieConflict
StreamError
- StreamConsumed
- ResponseNotRead
- RequestNotRead
- ResponseClosed

tomchristie · 2020-08-06T13:14:48Z

Now resolved.

Documentation upcoming in #1138

tomchristie added the docs Changes to the documentation label May 13, 2020

tomchristie added this to the v1.0 milestone May 13, 2020

tomchristie mentioned this issue May 13, 2020

Missing documentation for Exceptions #940

Closed

sileht mentioned this issue May 25, 2020

fix(httpx): exception handling have changed Mergifyio/mergify#1113

Merged

florimondmanca mentioned this issue May 31, 2020

Improve error message in case of HTTP request error florimondmanca/asgi-sitemaps#1

Closed

florimondmanca added the bug Something isn't working label Jun 15, 2020

florimondmanca mentioned this issue Jun 15, 2020

0.13: some httpcore exceptions are missing from top level package #1023

Closed

2 tasks

johnanthonyowens mentioned this issue Jun 19, 2020

TimeoutException missing from package #1029

Closed

2 tasks

This was referenced Jul 2, 2020

Map HTTPCore exceptions #1044

Merged

Add exceptions missing from top-level package #1045

Merged

florimondmanca mentioned this issue Jul 16, 2020

Consider renaming HTTPError to RequestError #1064

Closed

florimondmanca added discussion user-experience Ensuring that users have a good experience using the library and removed bug Something isn't working labels Jul 16, 2020

florimondmanca mentioned this issue Jul 18, 2020

Raise dedicated exception in .raise_from_status() #1067

Closed

tomchristie mentioned this issue Jul 27, 2020

Version 1.0, working notes. #1092

Closed

11 tasks

tomchristie modified the milestones: v1.0, v0.14 Jul 27, 2020

tomchristie mentioned this issue Jul 27, 2020

Exception hierarchy #1095

Merged

florimondmanca mentioned this issue Aug 1, 2020

Document the exception hierarchy #1106

Closed

tomchristie mentioned this issue Aug 2, 2020

Version 0.14.0 #1083

Merged

tomchristie closed this as completed Aug 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exceptions - Documentation, Hierarchy & Naming. #949

Exceptions - Documentation, Hierarchy & Naming. #949

tomchristie commented May 13, 2020

tomchristie commented May 13, 2020

johnanthonyowens commented May 31, 2020

MicahLyle commented Jun 1, 2020 •

edited

Loading

iwoloschin commented Jun 12, 2020

florimondmanca commented Jun 15, 2020

florimondmanca commented Jun 15, 2020 •

edited

Loading

martinka commented Jun 18, 2020

tomchristie commented Jul 21, 2020 •

edited

Loading

tomchristie commented Jul 27, 2020

florimondmanca commented Jul 27, 2020

tomchristie commented Aug 2, 2020 •

edited

Loading

tomchristie commented Aug 6, 2020 •

edited

Loading

Exceptions - Documentation, Hierarchy & Naming. #949

Exceptions - Documentation, Hierarchy & Naming. #949

Comments

tomchristie commented May 13, 2020

tomchristie commented May 13, 2020

Transport Errors, mapped from httpcore

Content Errors

Redirect Errors

Stream Errors

johnanthonyowens commented May 31, 2020

MicahLyle commented Jun 1, 2020 • edited Loading

iwoloschin commented Jun 12, 2020

florimondmanca commented Jun 15, 2020

florimondmanca commented Jun 15, 2020 • edited Loading

martinka commented Jun 18, 2020

tomchristie commented Jul 21, 2020 • edited Loading

tomchristie commented Jul 27, 2020

florimondmanca commented Jul 27, 2020

tomchristie commented Aug 2, 2020 • edited Loading

tomchristie commented Aug 6, 2020 • edited Loading

Transport Errors, mapped from `httpcore`

MicahLyle commented Jun 1, 2020 •

edited

Loading

florimondmanca commented Jun 15, 2020 •

edited

Loading

tomchristie commented Jul 21, 2020 •

edited

Loading

tomchristie commented Aug 2, 2020 •

edited

Loading

tomchristie commented Aug 6, 2020 •

edited

Loading