fix: raise ISYConnectionError on SSL/TLS handshake failures#507
Merged
Conversation
aiohttp.ClientConnectorSSLError is a subclass of ClientOSError, so the
existing catch-all branch silently classified TLS handshake failures as
generic "ISY not ready or closed connection." debug — leaving callers
with no signal to look at TLS settings and the retry path returning a
silent None that looks like a transient miss.
Catch ClientSSLError (covers both ClientConnectorSSLError and
ClientConnectorCertificateError) before the generic ClientOSError
branch and always raise ISYConnectionError with the SSL detail in the
exception message. The original aiohttp error rides along in the
``__cause__`` chain. No separate WARNING/ERROR log so the failure
surfaces exactly once.
Always raise (not just on retries=None) because an SSL handshake
failure isn't a transient network issue — it's a config mismatch
(controller pinned below the ``tls_ver='auto'`` floor of TLS 1.2, or
``verify_ssl=True`` against the controller's self-signed cert) that
won't recover from retry. Callers (HA Core) need a definitive failure
to translate into ConfigEntryNotReady, not silent retries.
Smoke-tested against an ISY-994 manually downgraded to TLS 1.1 with
``MinProtocol=TLSv1.2`` on the host: previously surfaced as the opaque
debug + a generic ISYConnectionError from test_connection's "Could not
connect" wrapper; now raises ``ISYConnectionError("SSL/TLS error:
... [SSL: UNSUPPORTED_PROTOCOL] ...")`` directly with the
ClientConnectorSSLError preserved in __cause__.
74c959f to
ce6be08
Compare
This was referenced May 8, 2026
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
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
aiohttp.ClientConnectorSSLErroris a subclass ofClientOSError, so the existing catch-all branch silently classified TLS handshake failures as a genericDEBUG ISY not ready or closed connection.— and on the retry path returned a silentNonethat looks indistinguishable from a transient miss.This PR catches
aiohttp.ClientSSLError(covers bothClientConnectorSSLErrorandClientConnectorCertificateError) before the genericClientOSErrorbranch and always raisesISYConnectionErrorwith the SSL detail in the message. The original aiohttp error rides along in__cause__. No separate WARNING/ERROR log — the failure surfaces exactly once.Why always raise (not just on
retries=None)An SSL handshake failure isn't a transient network blip — it's a config mismatch:
tls_ver='auto'floor of TLS 1.2 (e.g. ISY-994 manually downgraded to TLS 1.1, or modern OpenSSL withMinProtocol=TLSv1.2)verify_ssl=Trueagainst the controller's self-signed cert (ClientConnectorCertificateError)Neither recovers from retry. Callers (HA Core) need a definitive failure to translate into
ConfigEntryNotReady, not silent retries.Before / after (ISY-994 at TLS 1.1, modern OpenSSL host)
Before — silent debug, then a generic wrapper error from
test_connection:After — raises directly with SSL context, original cause preserved:
Test plan
pytest -q(438 passed)MinProtocol=TLSv1.2on the hosttest_request_ssl_error_always_raises_connection_error— covers retry pathtest_request_ssl_error_raises_on_test_connection_path— coversretries=Noneinit path__cause__carries the originalClientSSLErrorCompatibility note
ISYConnectionErroris the same exception type that was raised previously on theretries=None(init) path — so theISY.initialize()contract is unchanged, just with a more specific message. The behavioral shift is for non-initrequest()calls that previously returnedNoneon SSL failure: those now raise. In practice that path is reached only if a long-lived session sees its TLS context become invalid mid-flight (cert rotation, etc.), which warrants a definitive failure rather than a silent retry loop.🤖 Generated with Claude Code