Httpx: Exceptions - Documentation, Hierarchy & Naming.

Created on 13 May 2020  Â·  12Comments  Â·  Source: encode/httpx

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?)

See also: #940, #884, #869.

discussion docs user-experience

Most helpful comment

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.

All 12 comments

Current hierarchy looks like this...

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

+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.

+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 https://github.com/encode/httpx/pull/884#issuecomment-634749263 which reassures me that catching HTTPError should catch those underlying timeout errors?

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?

@iwoloschin As per https://github.com/encode/httpx/pull/884#issuecomment-634749263 yes, any exception leaking from httpcore not wrapped by HTTPX should be considered a bug, which is also what this issue is about.

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.

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).

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.

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.

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

After https://github.com/encode/httpcore/pull/128 and https://github.com/encode/httpcore/pull/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

Now resolved.

Documentation upcoming in https://github.com/encode/httpx/pull/1138

Was this page helpful?
0 / 5 - 0 ratings