Skip to content

Commit 5896072

Browse files
authored
fix: raise ISYConnectionError on SSL/TLS handshake failures (#507)
1 parent 64349f1 commit 5896072

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
@@ -298,6 +298,60 @@ async def test_request_client_response_error_with_ok404_returns_empty(
298298
assert result == ""
299299

300300

301+
async def test_request_ssl_error_always_raises_connection_error(
302+
conn: Connection,
303+
) -> None:
304+
"""SSL/TLS handshake failures are non-recoverable config mismatches
305+
(controller pinned below auto-floor TLS 1.2, or ``verify_ssl=True``
306+
against a self-signed cert). ``request`` must raise
307+
``ISYConnectionError`` rather than logging + returning ``None``,
308+
even on the retry path — retrying a handshake that failed for a
309+
protocol/cert reason won't help, and callers (HA Core) need a
310+
definitive failure to translate into ``ConfigEntryNotReady``
311+
instead of treating it as a transient miss.
312+
313+
The raise replaces the previous opaque
314+
``ClientOSError`` → DEBUG "ISY not ready or closed connection."
315+
branch that silently ate ``ClientConnectorSSLError`` (a subclass).
316+
The SSL detail rides along in ``__cause__`` and the exception
317+
message — no separate WARNING/ERROR log so we don't double up."""
318+
from unittest.mock import MagicMock
319+
320+
from pyisy.exceptions import ISYConnectionError
321+
322+
url = conn.compile_url(["status"])
323+
ssl_err = aiohttp.ClientConnectorSSLError(
324+
MagicMock(),
325+
ssl.SSLError(1, "[SSL: UNSUPPORTED_PROTOCOL] unsupported protocol"),
326+
)
327+
with aioresponses() as mocked:
328+
mocked.get(url, exception=ssl_err)
329+
with pytest.raises(ISYConnectionError, match="SSL/TLS error") as excinfo:
330+
await conn.request(url)
331+
# Cause chain preserves the original aiohttp SSL error for
332+
# callers / log handlers that want to introspect it.
333+
assert isinstance(excinfo.value.__cause__, aiohttp.ClientSSLError)
334+
335+
336+
async def test_request_ssl_error_raises_on_test_connection_path(
337+
conn: Connection,
338+
) -> None:
339+
"""Same behavior on the ``retries=None`` path used by
340+
``test_connection`` / ``ISY.initialize`` — verifying the SSL branch
341+
is taken before the (formerly raise-on-retries-None) generic
342+
``ClientOSError`` branch."""
343+
from unittest.mock import MagicMock
344+
345+
from pyisy.exceptions import ISYConnectionError
346+
347+
url = conn.compile_url(["config"])
348+
ssl_err = aiohttp.ClientConnectorSSLError(MagicMock(), ssl.SSLError(1, "unsupported protocol"))
349+
with aioresponses() as mocked:
350+
mocked.get(url, exception=ssl_err)
351+
with pytest.raises(ISYConnectionError, match="SSL/TLS error"):
352+
await conn.request(url, retries=None)
353+
354+
301355
async def test_request_non_rest_url_does_not_crash(conn: Connection) -> None:
302356
"""Regression for #488: ``request()`` derives its debug-log endpoint
303357
from the URL by splitting on ``"rest"``. ``get_description()`` builds

0 commit comments

Comments
 (0)