Skip to content

Commit 74c959f

Browse files
committed
fix: raise ISYConnectionError on SSL/TLS handshake failures
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__.
1 parent d60907d commit 74c959f

2 files changed

Lines changed: 70 additions & 0 deletions

File tree

pyisy/connection.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,22 @@ async def request(
212212

213213
except TimeoutError:
214214
_LOGGER.warning("Timeout while trying to connect to the ISY.")
215+
except aiohttp.ClientSSLError as err:
216+
# SSL/TLS handshake failure. Subclass of ``ClientOSError``, so
217+
# the broader branch below would otherwise eat it silently as
218+
# a generic "ISY not ready or closed connection." debug.
219+
# Almost always one of:
220+
# * controller pinned below the ``tls_ver='auto'`` floor of
221+
# TLS 1.2 (e.g. an ISY-994 manually downgraded to 1.1, or
222+
# a modern OpenSSL distro with ``MinProtocol=TLSv1.2``).
223+
# * ``verify_ssl=True`` against the controller's self-signed
224+
# cert (``ClientConnectorCertificateError``).
225+
# Always raise — retrying won't recover from a config
226+
# mismatch, and callers (HA Core) need a definitive failure
227+
# to translate into ``ConfigEntryNotReady`` rather than a
228+
# silent ``None`` that looks like a transient miss. The SSL
229+
# detail rides along in the exception chain.
230+
raise ISYConnectionError(f"SSL/TLS error: {err}") from err
215231
except (
216232
aiohttp.ClientOSError,
217233
aiohttp.ServerDisconnectedError,

tests/test_connection_extras.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,60 @@ async def test_request_client_response_error_returns_none(conn: Connection) -> N
259259
assert result is None
260260

261261

262+
async def test_request_ssl_error_always_raises_connection_error(
263+
conn: Connection,
264+
) -> None:
265+
"""SSL/TLS handshake failures are non-recoverable config mismatches
266+
(controller pinned below auto-floor TLS 1.2, or ``verify_ssl=True``
267+
against a self-signed cert). ``request`` must raise
268+
``ISYConnectionError`` rather than logging + returning ``None``,
269+
even on the retry path — retrying a handshake that failed for a
270+
protocol/cert reason won't help, and callers (HA Core) need a
271+
definitive failure to translate into ``ConfigEntryNotReady``
272+
instead of treating it as a transient miss.
273+
274+
The raise replaces the previous opaque
275+
``ClientOSError`` → DEBUG "ISY not ready or closed connection."
276+
branch that silently ate ``ClientConnectorSSLError`` (a subclass).
277+
The SSL detail rides along in ``__cause__`` and the exception
278+
message — no separate WARNING/ERROR log so we don't double up."""
279+
from unittest.mock import MagicMock
280+
281+
from pyisy.exceptions import ISYConnectionError
282+
283+
url = conn.compile_url(["status"])
284+
ssl_err = aiohttp.ClientConnectorSSLError(
285+
MagicMock(),
286+
ssl.SSLError(1, "[SSL: UNSUPPORTED_PROTOCOL] unsupported protocol"),
287+
)
288+
with aioresponses() as mocked:
289+
mocked.get(url, exception=ssl_err)
290+
with pytest.raises(ISYConnectionError, match="SSL/TLS error") as excinfo:
291+
await conn.request(url)
292+
# Cause chain preserves the original aiohttp SSL error for
293+
# callers / log handlers that want to introspect it.
294+
assert isinstance(excinfo.value.__cause__, aiohttp.ClientSSLError)
295+
296+
297+
async def test_request_ssl_error_raises_on_test_connection_path(
298+
conn: Connection,
299+
) -> None:
300+
"""Same behavior on the ``retries=None`` path used by
301+
``test_connection`` / ``ISY.initialize`` — verifying the SSL branch
302+
is taken before the (formerly raise-on-retries-None) generic
303+
``ClientOSError`` branch."""
304+
from unittest.mock import MagicMock
305+
306+
from pyisy.exceptions import ISYConnectionError
307+
308+
url = conn.compile_url(["config"])
309+
ssl_err = aiohttp.ClientConnectorSSLError(MagicMock(), ssl.SSLError(1, "unsupported protocol"))
310+
with aioresponses() as mocked:
311+
mocked.get(url, exception=ssl_err)
312+
with pytest.raises(ISYConnectionError, match="SSL/TLS error"):
313+
await conn.request(url, retries=None)
314+
315+
262316
async def test_request_non_rest_url_does_not_crash(conn: Connection) -> None:
263317
"""Regression for #488: ``request()`` derives its debug-log endpoint
264318
from the URL by splitting on ``"rest"``. ``get_description()`` builds

0 commit comments

Comments
 (0)