Merge pull request #450 from njloof/fix/signalr-disconnect-attributeerror

Fix AttributeError on SignalR disconnect during HA shutdown

Author: ic-dev21

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

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