[Feature Request]: raise httpx exception instead of buildin ConnectionResetError and ConnectionError

This is a feature requests instead of bug report.

I'm using requests in my web server to request upstream api. So I can handler all requests.RequestException in my middleware and return a 502 upstream error.

But after I migrated to httpx, httpx doesn't have such exception when issuing a http(s) request, it will raise httpx.ConnectTimeout, buildin ConnectionError and ConnectResetError. And last 2 exception are not only raised by httpx.

Maybe httpx should also wrap ConnectionError and other buildin connection exception?

@trim21

Hi @trim21,

I think we're going to need a bit more context to address this issue. :-)

What were you trying to do?
Can you share a minimal reproducible example?
A traceback of the error?

Thanks!

@florimondmanca

@florimondmanca Sorry, I submit this issue by accident and I'm still writting details.

@tomchristie

Thanks for updating the description. :) I'm not entirely sure on this myself, I'll ping @tomchristie on this one.

Probably because we don't capture any socket.error or socket.gaierror.

I remember that Trio already wraps errors such as ConnectionResetError into its own exceptions, eg BrokenResourceError. That means whatever wrapping we do needs to occur at the concurrency backend level. :)

@florimondmanca

@florimondmanca I'm sorry, I'm not familiar with trio. What's the relation between trio and httpx?

@trim21

@trim21 HTTPX supports the async/await syntax, which under the hood requires us to work with a library able to perform asynchronous operations. Right now we only support asyncio (which is part of the standard library), but we’ve got plans for supporting alternatives such as trio - see #120. The « concurrency backend » I mentioned in my comment is an internal interface we use to abstract our code away from the individual async libraries.

I guess the point in my comment was that which exceptions we need to wrap may depend on the async library in use, so we should do the wrapping at the concurrency backend level — provided we do want to wrap those exceptions.

@florimondmanca

@florimondmanca thanks for your explanation

will this be a feature of 1.0 ?

It'll probably be in before 1.0, just requires a contributor to do the work! :)

Yeah our guidelines here are probably roughly that all networking exception types should tend to be wrapped up in an HTTPX exception, although we're likely not at that point yet in several areas.

Hope your Hacktoberfest is going well. Can confirm this happens with all connection types. All I need to do is turn off WiFi to replicate with both sync:

>>> import httpx
>>> # With connection
>>> httpx.get("http://httpbin.org/status/200").status_code
200
>>> # Without connection
>>> httpx.get("http://httpbin.org/status/200").status_code
Traceback (most recent call last):
...
socket.gaierror: [Errno 8] nodename nor servname provided, or not known

And async:

>>> import asyncio
>>> import httpx
>>> async def fetch():
...   async with httpx.AsyncClient() as client:
...     return await client.get("http://httpbin.com/status/200")
... 
>>> # With connection
>>> asyncio.run(fetch()).status_code
200
>>> # Without connection
>>> asyncio.run(fetch()).status_code
Traceback (most recent call last):
...
socket.gaierror: [Errno 8] nodename nor servname provided, or not known

@flyinactor91 httpbin.com isn't a website. Do you mean https://httpbin.org?

@sethmlarson

@sethmlarson He wants httpx to raise socket.gaierror.

Lol. Yes I do. So much for typing things in again :)

I'm currently handling generic connection issues via except socket.gaierror for the time being. It works (and is stdlib), but a native httpx exception class or the stdlib ConnectionError would be preferred.

Did a bit of digging, and here's a list of cases it seems we'd want to wrap as HTTPX exceptions:

OSError: "This exception is raised when a system function returns a system-related error, including I/O failures". Also deals with socket.error and its subclasses such as socket.gaierror mentioned earlier in this thread.
ConnectionError: built-in base class for connection-related issues (including BrokenPipeError, ConnectionResetError and ConnectionRefusedError).
trio.BrokenResourceError: "Raised when an attempt to use a resource fails due to external circumstances" - basically trio's own wrapping of the above exceptions.

Note: asyncio doesn't seem to have network-related exceptions that we should handle -- see asyncio exceptions. (There's IncompleteReadError, but it's raised by reader methods we don't use.)

@florimondmanca

@florimondmanca Fantastic, thanks! It might be good to also cross-reference against exactly what exception heirarchy/wrapping users get with urllib3/requests.

@tomchristie

@tomchristie Sure! So it turns out to be a bit messy, but here's what I found…

`urllib3.exceptions`

There's a generic ProtocolError (previously named ConnectionError), but it's not a consistent base class for network-related exceptions. Eg. NewConnectionError isn't a subclass of ConnectionError, although it's wrapper for ConnectionRefusedError which is a subclass of the built-in ConnectionError.

Some finer-grained network-related exceptions exists as well:

urllib3.exceptions.NewConnectionError: a wrapper for ConnectionRefusedError
urllib3.exceptions.IncompleteRead: a wrapper for httplib.IncompleteRead

`requests.exceptions`

Requests uses a wrap-all requests.exceptions.ConnectionError. Most of the exception handling seems to take place in HTTPAdapter.send(). The adapter catches various urllib3 exceptions raised by conn.urlopen() and other calls, and raises an appropriate subclass of requests.exceptions.ConnectionError, e.g. ProxyError, SSLError, or ConnectionError itself.

I think going the route a wrap-all exception in the client's .send() method sounds sensible enough. I wouldn't follow Requests' naming convention though, because ConnectionError is a built-in. So e.g. NetworkError.

I think going the route a wrap-all exception in the client's .send() method sounds sensible enough.

The only tricky bit is streaming responses, because in that case we're doing I/O outside of .send().

Which means we may want to perform wrapping inside Response.aiter_raw() and Response.aclose() as well?

Really we want to wrap as close to the exception as possible.
Ie. Specifying what exceptions we expect from our backend .read(), .write() and connection methods, and making sure we're wrapping at that level.

Navigation Menu

Repository navigation

[Feature Request]: raise httpx exception instead of buildin ConnectionResetError and ConnectionError #349

`urllib3.exceptions`

`requests.exceptions`

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[Feature Request]: raise httpx exception instead of buildin ConnectionResetError and ConnectionError #349

Description

Activity

florimondmanca commented on Sep 16, 2019

trim21 commented on Sep 16, 2019

florimondmanca commented on Sep 16, 2019

sethmlarson commented on Sep 16, 2019

florimondmanca commented on Sep 17, 2019

trim21 commented on Sep 17, 2019

florimondmanca commented on Sep 17, 2019

trim21 commented on Sep 17, 2019

trim21 commented on Sep 21, 2019

sethmlarson commented on Sep 21, 2019

lovelydinosaur commented on Sep 27, 2019

devdupont commented on Oct 26, 2019

sethmlarson commented on Oct 26, 2019

trim21 commented on Oct 26, 2019

devdupont commented on Oct 26, 2019

florimondmanca commented on Jan 1, 2020

lovelydinosaur commented on Jan 1, 2020

florimondmanca commented on Jan 1, 2020

urllib3.exceptions

requests.exceptions

florimondmanca commented on Jan 1, 2020

lovelydinosaur commented on Jan 1, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`urllib3.exceptions`

`requests.exceptions`