diff --git a/custom_components/nestquest/api_client.py b/custom_components/nestquest/api_client.py index 5f30d44..71fa309 100644 --- a/custom_components/nestquest/api_client.py +++ b/custom_components/nestquest/api_client.py @@ -16,9 +16,11 @@ network and NO aiohttp installed; production builds the client through :func:`client_from_entry`, which hands in Home Assistant's shared aiohttp session (``homeassistant.helpers.aiohttp_client .async_get_clientsession``). The module itself imports NEITHER -homeassistant NOR aiohttp at runtime (aiohttp appears only behind -``TYPE_CHECKING`` for annotations), so the client is usable anywhere — -the HA coupling lives in the one production factory. +homeassistant NOR aiohttp at module import (aiohttp appears only +behind ``TYPE_CHECKING`` for annotations, plus one lazy import inside +:meth:`NestQuestApiClient.stream_events` for its ``ClientTimeout``), so +the client is importable anywhere — the HA coupling lives in the one +production factory. Every failure is typed: a transport failure (connection refused, DNS failure, timeout — the exchange never produced a response) and every @@ -38,7 +40,11 @@ frame (``event: `` / ``data: `` lines separated by a blank line). Opening the stream gets the same one-timeout, typed-error contract as every other call; reading it has NO body-wide timeout (a healthy stream is long-lived — the subscription manager owns -reconnection and backoff). +reconnection and backoff). That includes the session's own default: +Home Assistant's shared session carries ``ClientTimeout(total=300)``, +which would cut a healthy stream every five minutes, so the stream +request overrides it with ``total=None`` (keeping a finite +``sock_connect``). """ from __future__ import annotations @@ -278,8 +284,19 @@ class NestQuestApiClient: (``aclose``, i.e. the consumer's task being cancelled) stops cleanly and closes the underlying response in every path. """ + # Lazy: aiohttp ships with Home Assistant, but importing it at + # module level would make the client unimportable without it. + import aiohttp + url = f"{self._base_url}{EVENTS_PATH}" headers = self._headers() + # The session's default total timeout (HA's shared session: + # 300 s) would cut a healthy stream mid-body, and events fired + # while reconnecting are not replayed — so the stream request + # is unbounded in total time; only connecting stays bounded. + stream_timeout = aiohttp.ClientTimeout( + total=None, sock_connect=self._timeout_seconds + ) # Opening the response gets ONE request timeout, exactly like # ``_request_json``; the read loop below runs WITHOUT one — a # body-wide timeout would kill a healthy long-lived stream. @@ -289,7 +306,7 @@ class NestQuestApiClient: try: async with asyncio.timeout(self._timeout_seconds): stream_cm = self._session.request( - "GET", url, headers=headers, json=None + "GET", url, headers=headers, json=None, timeout=stream_timeout ) response = await stream_cm.__aenter__() except asyncio.CancelledError: diff --git a/tests/conftest.py b/tests/conftest.py index 4ff6a74..f938f38 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -879,6 +879,29 @@ def set_coordinator_client(entry, client): return client +@dataclass(frozen=True) +class FakeClientTimeout: + """Stand-in for ``aiohttp.ClientTimeout`` (the fields the client sets).""" + + total: float | None = None + sock_connect: float | None = None + + +@pytest.fixture +def fake_aiohttp(monkeypatch): + """Install a stand-in ``aiohttp`` module for one test. + + aiohttp ships only with Home Assistant, so this mock-only harness + does not have it; the API client imports it lazily inside + ``stream_events`` (for ``ClientTimeout``). Tests driving the real + stream path request this fixture; the module is removed again when + the test ends. + """ + module = SimpleNamespace(ClientTimeout=FakeClientTimeout) + monkeypatch.setitem(sys.modules, "aiohttp", module) + return module + + @pytest.fixture(autouse=True) def coordinator_client_from_test_override(): """Resolve every entry's coordinator client through the test seam. diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 3082158..5a4016c 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -17,7 +17,7 @@ import types import pytest -from conftest import make_config_entry +from conftest import FakeClientTimeout, make_config_entry from custom_components.nestquest import api_client from custom_components.nestquest.api_client import ( @@ -33,6 +33,10 @@ from custom_components.nestquest.const import ( DEFAULT_PANEL_TOKEN, ) +# ``stream_events`` imports aiohttp lazily; the harness has none, so +# every test here runs with the stand-in module installed. +pytestmark = pytest.mark.usefixtures("fake_aiohttp") + BASE_URL = "http://api.test:8000" TOKEN = "test-panel-token" @@ -124,6 +128,7 @@ class StubTransport: "url": url, "headers": kwargs.get("headers"), "json": kwargs.get("json"), + "timeout": kwargs.get("timeout"), } ) if not self._outcomes: @@ -557,3 +562,24 @@ async def test_stream_events_invalid_json_frame_raises_typed() -> None: assert excinfo.value.status == 200 assert not excinfo.value.is_transport_error assert isinstance(excinfo.value.__cause__, ValueError) + + +async def test_stream_events_disables_the_session_total_timeout() -> None: + """The stream request overrides the session's default total timeout + (HA's shared session: 300 s, which would cut a healthy stream) with + ``total=None`` while keeping a finite connect timeout; the plain + request calls pass no per-request timeout.""" + transport = StubTransport( + StubStreamResponse(200, []), StubResponse(200, json.dumps(SNAPSHOT_PAYLOAD)) + ) + client = make_client(transport, timeout_seconds=7.0) + + frames = [frame async for frame in client.stream_events()] + await client.get_snapshot() + + assert frames == [] + stream_timeout = transport.calls[0]["timeout"] + assert isinstance(stream_timeout, FakeClientTimeout) + assert stream_timeout.total is None + assert stream_timeout.sock_connect == 7.0 + assert transport.calls[1]["timeout"] is None diff --git a/tests/test_sse_subscription.py b/tests/test_sse_subscription.py index a07069f..6b0e458 100644 --- a/tests/test_sse_subscription.py +++ b/tests/test_sse_subscription.py @@ -44,6 +44,10 @@ from custom_components.nestquest.api_client import ( ) from custom_components.nestquest.sse import NestQuestEventStream +# ``stream_events`` imports aiohttp lazily; the harness has none, so +# every test here runs with the stand-in module installed. +pytestmark = pytest.mark.usefixtures("fake_aiohttp") + BASE_URL = "http://api.test:8000" TOKEN = "test-panel-token"