encode / httpcore

A minimal HTTP client. ⚙️
https://www.encode.io/httpcore/
BSD 3-Clause "New" or "Revised" License
465 stars 105 forks source link

Raise exception with cause exception #915

Open CzBiX opened 5 months ago

CzBiX commented 5 months ago

Summary

In the current logic, the original exception is discarded, which makes it hard to determine the thrown exception more precisely.

Checklist

Use case

try:
  resp = pool.request('GET', 'https://example.com')
except ConnectError as error:
  # before
  assert error.__cause__ is None
  # after
  assert isinstance(error.__cause__, ssl.SSLError)
tomchristie commented 4 months ago

Pull requests framed like this create more work for the maintainer than necessary. An improvement would be to include a concise example of the behavior both before and after the change. Ie. It'd help more to focus on the behavior rather than the code.

CzBiX commented 4 months ago

hi @tomchristie, here is the updated use case :

try:
  resp = pool.request('GET', 'https://example.com')
except ConnectError as error:
  # before
  assert error.__cause__ is None
  # after
  assert isinstance(error.__cause__, ssl.SSLError)
tomchristie commented 4 months ago

Can you demo the traceback in both cases?

I'm reluctant to bring this in too easily... raise exc from None was a deliberate design decision, can you link back to the history on it?

CzBiX commented 4 months ago

This change was introduced in the #880. I can't see this change fixing any bugs or causing any bugs though.

From my point of view, it just influence the information used for debug and trace, and this change rather breaks the coherence of traceback. Note the extra "line 216".

File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection_pool.py", line 216, in handle_request raise exc from None

Here is the traceback before and after my PR: before:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/interfaces.py", line 43, in request
    response = self.handle_request(request)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection_pool.py", line 216, in handle_request
    raise exc from None
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection_pool.py", line 196, in handle_request
    response = connection.handle_request(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection.py", line 99, in handle_request
    raise exc
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection.py", line 76, in handle_request
    stream = self._connect(request)
             ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection.py", line 154, in _connect
    stream = stream.start_tls(**kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/httpcore/_backends/sync.py", line 152, in start_tls
    with map_exceptions(exc_map):
  File "/usr/local/lib/python3.12/contextlib.py", line 158, in __exit__
    self.gen.throw(value)
  File "/usr/local/lib/python3.12/site-packages/httpcore/_exceptions.py", line 14, in map_exceptions
    raise to_exc(exc) from exc
httpcore.ConnectError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1000)

after:

Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/httpcore/_exceptions.py", line 10, in map_exceptions
    yield
  File "/usr/local/lib/python3.12/site-packages/httpcore/_backends/sync.py", line 168, in start_tls
    raise exc
  File "/usr/local/lib/python3.12/site-packages/httpcore/_backends/sync.py", line 163, in start_tls
    sock = ssl_context.wrap_socket(
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/ssl.py", line 455, in wrap_socket
    return self.sslsocket_class._create(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/ssl.py", line 1042, in _create
    self.do_handshake()
  File "/usr/local/lib/python3.12/ssl.py", line 1320, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1000)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/interfaces.py", line 43, in request
    response = self.handle_request(request)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection_pool.py", line 196, in handle_request
    response = connection.handle_request(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection.py", line 99, in handle_request
    raise exc
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection.py", line 76, in handle_request
    stream = self._connect(request)
             ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/httpcore/_sync/connection.py", line 154, in _connect
    stream = stream.start_tls(**kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/httpcore/_backends/sync.py", line 152, in start_tls
    with map_exceptions(exc_map):
  File "/usr/local/lib/python3.12/contextlib.py", line 158, in __exit__
    self.gen.throw(value)
  File "/usr/local/lib/python3.12/site-packages/httpcore/_exceptions.py", line 14, in map_exceptions
    raise to_exc(exc) from exc
httpcore.ConnectError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1000)
MarkusSintonen commented 4 months ago

This would be great to fix! We have also hit this issue so many times where we dont see anything about the actual cause for different network issues. (We are using Sentry to monitor the errors and we dont see much of details about the problems as httpcore hides these annoyingly.)

We could also enable this linting rule https://docs.astral.sh/ruff/rules/raise-without-from-inside-except/ under [tool.ruff.lint]. 🙏