From da21a5cce9b66a922bb923241c1d6a683a420f16 Mon Sep 17 00:00:00 2001 From: njloof Date: Sat, 5 Sep 2026 07:36:59 -0700 Subject: [PATCH] Fix AttributeError on SignalR disconnect during HA shutdown SignalRHub.disconnect() called self._client.stop(), but pysignalr.SignalRClient (>=1.3.0) has no stop() or close() method -- its run() coroutine blocks until cancelled and is designed to be stopped by cancelling the task awaiting it (see WebsocketTransport.run(), an unconditional reconnect loop with no external stop hook). This raised AttributeError on every graceful Home Assistant shutdown/restart, which then cascaded into "RuntimeError: aclose(): asynchronous generator is already running" from the interrupted cleanup. SignalRHub now records the task running run() via asyncio.current_task() and disconnect() cancels and awaits that task instead of calling the nonexistent client method. Impact was limited to shutdown: the AttributeError fired only from the EVENT_HOMEASSISTANT_STOP listener in custom_components/hilo, did not prevent Home Assistant from stopping, and did not affect the next startup (a fresh SignalRClient is always created). Cosmetic but real -- flooded logs on every restart. Added tests/test_signalr.py covering disconnect() cancelling a live run() task and disconnect() being a no-op when no task is running. Confirmed the new test fails with the original AttributeError against the pre-fix code and passes against the fix. Verified: full test suite (66 tests) passes, ruff check/format clean, mypy clean, bandit and codespell clean on the changed files. --- pyhilo/signalr.py | 24 +++++++++++++--- tests/test_signalr.py | 66 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 tests/test_signalr.py diff --git a/pyhilo/signalr.py b/pyhilo/signalr.py index 8ad6fa1..74b85bd 100644 --- a/pyhilo/signalr.py +++ b/pyhilo/signalr.py @@ -72,7 +72,7 @@ class SignalRHub: Lifecycle: ``run()`` — negotiate fresh token, build client, block until disconnect ``invoke()`` — send a hub method invocation - ``disconnect()`` — stop the transport cleanly + ``disconnect()`` — cancel the task awaiting ``run()`` to stop cleanly """ def __init__( @@ -85,6 +85,7 @@ def __init__( """ self._negotiate = negotiate_callback self._client: Optional[SignalRClient] = None + self._task: Optional[asyncio.Task] = None self._connect_callbacks: list[Callable[..., Any]] = [] self._disconnect_callbacks: list[Callable[..., Any]] = [] self._event_callbacks: list[Callable[..., Any]] = [] @@ -177,10 +178,12 @@ async def _handler(arguments: Any) -> None: self._client.on_close(self._on_close) self._client.on_error(self._on_error) + self._task = asyncio.current_task() try: await self._client.run() finally: self._client = None + self._task = None async def invoke(self, method: str, args: list[Any]) -> None: """Invoke a hub method on the server. @@ -195,10 +198,23 @@ async def invoke(self, method: str, args: list[Any]) -> None: await self._client.send(method, args) async def disconnect(self) -> None: - """Request the client to stop.""" - if self._client is not None: + """Request the client to stop. + + ``pysignalr.SignalRClient`` has no ``stop()``/``close()`` method -- + its ``run()`` coroutine blocks until the connection ends and is + meant to be cancelled by the caller (see pysignalr's + ``WebsocketTransport.run()``, an unconditional reconnect loop with + no external stop hook). Cancel the task that's awaiting ``run()`` + instead of calling a nonexistent client method. + """ + task = self._task + if task is not None: LOG.info("SignalRHub: disconnecting") - await self._client.stop() + task.cancel() + try: + await task + except asyncio.CancelledError: + pass # ------------------------------------------------------------------ # Internal pysignalr hooks diff --git a/tests/test_signalr.py b/tests/test_signalr.py new file mode 100644 index 0000000..1effeab --- /dev/null +++ b/tests/test_signalr.py @@ -0,0 +1,66 @@ +import asyncio + +import pytest + +from pyhilo.signalr import SignalRHub + + +async def _fake_negotiate() -> tuple[str, str]: + return ("wss://example.invalid/hub", "fake-token") + + +@pytest.mark.asyncio +async def test_disconnect_cancels_running_task(monkeypatch: pytest.MonkeyPatch) -> None: + """disconnect() must cancel the task running run(), not call client.stop(). + + pysignalr.SignalRClient has no stop()/close() method -- its run() + coroutine blocks until cancelled. Calling a nonexistent stop() method + raised AttributeError on every Home Assistant shutdown. + """ + + client_connected = asyncio.Event() + + class FakeSignalRClient: + def __init__(self, *args: object, **kwargs: object) -> None: + self._message_handlers: dict = {} + + def on_open(self, callback: object) -> None: + pass + + def on_close(self, callback: object) -> None: + pass + + def on_error(self, callback: object) -> None: + pass + + async def run(self) -> None: + # Block forever, like the real pysignalr transport's reconnect loop. + client_connected.set() + await asyncio.Event().wait() + + monkeypatch.setattr("pyhilo.signalr.SignalRClient", FakeSignalRClient) + monkeypatch.setattr("pyhilo.signalr.ssl.create_default_context", lambda: object()) + + hub = SignalRHub(negotiate_callback=_fake_negotiate) + task = asyncio.create_task(hub.run()) + + # Wait until run() has reached the blocking await inside + # FakeSignalRClient.run() (negotiation and the executor hop for the SSL + # context are real awaits, so a fixed number of sleep(0)s is fragile). + await asyncio.wait_for(client_connected.wait(), timeout=5) + assert hub.connected + + # Must not raise -- this previously called the nonexistent + # SignalRClient.stop() and raised AttributeError. + await hub.disconnect() + + assert task.done() + assert not hub.connected + + +@pytest.mark.asyncio +async def test_disconnect_without_a_running_task_is_a_no_op() -> None: + """disconnect() before run() has ever been called should not raise.""" + hub = SignalRHub(negotiate_callback=_fake_negotiate) + await hub.disconnect() + assert not hub.connected