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