Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exception re-raise in AsyncIOStream.start_tls #677

Closed
wants to merge 1 commit into from

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented May 5, 2023

In the previous implementation, the traceback would show "During handling of the above exception, another exception occurred" and include an extra frame.

In the previous implementation, the traceback would show "An exception occured while handling exception"
@zanieb zanieb changed the title Fix exception reraise in AsyncIOStream.start_tls Fix exception re-raise in AsyncIOStream.start_tls May 5, 2023
@tomchristie
Copy link
Member

That's interesting, didn't realise there's a different behaviour between those two cases.

How much work is it to demonstrate the before/after of the traceback for this?

@zanieb
Copy link
Contributor Author

zanieb commented May 9, 2023

This singular change is a small step in the right direction so it's not quite "fixed" in an example. Maybe I should open a pull request that changes all of these at once? Here are some examples:

WRONG: Displays that an exception occurred while handling an exception

try:
    raise ValueError("1")
except Exception as exc:
    raise ValueError("2")
Traceback (most recent call last):
  File "/Users/mz/dev/prefect/example.py", line 2, in <module>
    raise ValueError("1")
ValueError: 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/mz/dev/prefect/example.py", line 4, in <module>
    raise ValueError("2")
ValueError: 2

RIGHT: Exception chained to parent

try:
    raise ValueError("1")
except Exception as exc:
    raise ValueError("2") from exc
Traceback (most recent call last):
  File "/Users/mz/dev/prefect/example.py", line 2, in <module>
    raise ValueError("1")
ValueError: 1

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

Traceback (most recent call last):
  File "/Users/mz/dev/prefect/example.py", line 4, in <module>
    raise ValueError("2") from exc
ValueError: 2

WRONG: Extra frame included in traceback

try:
    raise ValueError("1")
except Exception as exc:
    raise exc
Traceback (most recent call last):
  File "/Users/mz/dev/prefect/example.py", line 4, in <module>
    raise exc
  File "/Users/mz/dev/prefect/example.py", line 2, in <module>
    raise ValueError("1")
ValueError: 1

RIGHT: No frame added when the exception is re-raised

try:
    raise ValueError("1")
except Exception as exc:
    raise
Traceback (most recent call last):
  File "/Users/mz/dev/prefect/example.py", line 2, in <module>
    raise ValueError("1")
ValueError: 1

@tomchristie
Copy link
Member

tomchristie commented May 10, 2023

Maybe I should open a pull request that changes all of these at once?

I think we should deal with these on a case-by-case basis. That'll give us better reviews.

This change looks correct to me, yes. Could you also add the sync and trio cases to this pull request?

@tomchristie
Copy link
Member

Hrm, not sure I wasn't able to verify this one...

>>> httpcore.request("GET", "https://142.250.200.36:443")
Traceback (most recent call last):
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/_exceptions.py", line 10, in map_exceptions
    yield
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/backends/sync.py", line 62, in start_tls
    raise exc
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/backends/sync.py", line 57, in start_tls
    sock = ssl_context.wrap_socket(
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/ssl.py", line 517, in wrap_socket
    return self.sslsocket_class._create(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/ssl.py", line 1075, in _create
    self.do_handshake()
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/ssl.py", line 1346, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate (_ssl.c:992)

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

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/_api.py", line 39, in request
    return pool.request(
           ^^^^^^^^^^^^^
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/_sync/interfaces.py", line 43, in request
    response = self.handle_request(request)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/_sync/connection_pool.py", line 253, in handle_request
    raise exc
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/_sync/connection_pool.py", line 237, in handle_request
    response = connection.handle_request(request)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/_sync/connection.py", line 86, in handle_request
    raise exc
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/_sync/connection.py", line 63, in handle_request
    stream = self._connect(request)
             ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/_sync/connection.py", line 141, in _connect
    stream = stream.start_tls(**kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/backends/sync.py", line 54, in start_tls
    with map_exceptions(exc_map):
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/contextlib.py", line 155, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/Users/tomchristie/GitHub/encode/httpcore/httpcore/_exceptions.py", line 14, in map_exceptions
    raise to_exc(exc) from exc
httpcore.ConnectError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate (_ssl.c:992)

Let's start with #678 which does look verifiably correct to me, and then go from there.

@zanieb zanieb closed this May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants