SSLError - #1156
Open
ccbest wants to merge 5 commits into
Open
Conversation
Adds `SSLError` to both `httpcore2` and `httpx2`, raised when a TLS handshake fails. It subclasses `ConnectError`, so code that already catches `ConnectError` is unaffected, while code that wants to distinguish a TLS failure from a TCP failure now can. This also brings the hierarchy in line with the libraries the change is meant to ease migration from: `requests.exceptions.SSLError` subclasses `ConnectionError`, and `aiohttp.ClientSSLError` subclasses `ClientConnectorError`. On the `trio` backend a failed handshake arrives wrapped in a `trio.BrokenResourceError`, which carries no message of its own, so the error previously surfaced as a `ConnectError` with an empty string. The underlying `ssl.SSLError` is now recovered from `__cause__`, which both types the error correctly and restores the message. Closes pydantic#854
The other exception classes in `httpcore2._exceptions` are bare `pass`, and httpcore2 docstrings are not rendered anywhere in the docs. The rationale for the parent class lives on the `httpx2` counterpart, which is rendered.
Re-raising trio's own `ssl.SSLError` created a reference cycle: trio sets
`BrokenResourceError.__cause__` to the ssl error, so `raise cause from exc`
pointed the ssl error back at the `BrokenResourceError`. Code walking
`__cause__ or __context__` — a common pattern in logging and error
reporting — would not terminate.
Raise a new `SSLError` carrying the original message instead. The chain
is now acyclic and in causal order:
httpx2.SSLError -> httpcore2.SSLError -> trio.BrokenResourceError
-> ssl.SSLCertVerificationError -> None
Merging this PR will not alter performance
Comparing 
Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…Error and change test assertions to look for lower-case reason text
Contributor
Author
|
CICD appears to be failing due to an unrelated issue with websockets in testing. The previous run passed all six python versions - the latest commit with the failures doesn't touch any websocket code and I wasn't able to reproduce the failure locally. Could a maintainer re-run the failed jobs? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds SSLError to both httpcore2 and httpx2, raised when a TLS handshake fails, per issue #854. Also fixes a related bug where a failed handshake on the trio backend surfaced with no error message at all.
Checklist
Why ConnectError instead of RequestError
The exception mapper would first match to NetworkError and ConnectError, so SSLError would never actually surface if it directly inherited from RequestError. The alternative would cause a breaking change as ConnectError, NetworkError, and TransportError would stop catching TLS failures.
Inheriting from ConnectError mirrors requests.exceptions.SSLError (subclasses ConnectionError) and aiohttp.ClientSSLError (subclasses ClientConnectorError) behavior.
trioError Message Fixtrio reports a failed handshake as trio.BrokenResourceError that stringifies to "". A trio certificate error would surface as ConnectError('') with no description to indicate the failure was TLS related.
Comparison I tested -
After this PR all three backends report identically.
Follow up:
I noticed that
retries=will also retry a TLS handshake failure, which is consistent withurllib3's behavior but is probably not optimal. This PR would enable httpx2 to shortcut a certificate failure by defining a further CertificateError subclass while allowing transient SSL errors (SSLEOFError, TLSV1_ALERT_INTERNAL_ERROR) to retry. I opened a corresponding discussion in #1157 .AI Disclosure: Claude Opus 5 assisted in drafting this PR; all claims and every line of code was verified by a human (myself)