From 1310c4bc35e3b18effadb9379dca6fe4797a2be5 Mon Sep 17 00:00:00 2001 From: honzup <5564623+honzup@users.noreply.github.com> Date: Thu, 16 Jul 2026 09:49:26 +0200 Subject: [PATCH 1/4] Do not refresh OAuth token during Tibber unload async_unload_entry fetched a client via async_get_client, which performs a network OAuth token refresh and, when the token rotated, a full websocket reset, reconnect and resubscribe - all while tearing the entry down. Any exception escaping the unload path wedges the entry into the non-recoverable FAILED_UNLOAD state, and the EVENT_HOMEASSISTANT_STOP listener awaited rt_disconnect without a timeout. Disconnect the cached client instead, exposed via a read-only property on TibberRuntimeData, bound the disconnect with a 10 second timeout and log instead of raising so unload cannot fail because a dead websocket would not close politely. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DERobfZZh3npFRbwpdxzz2 --- homeassistant/components/tibber/__init__.py | 24 ++++++++++++++++--- tests/components/tibber/test_init.py | 26 +++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/tibber/__init__.py b/homeassistant/components/tibber/__init__.py index bec7c562e0f017..af16f021308ee4 100644 --- a/homeassistant/components/tibber/__init__.py +++ b/homeassistant/components/tibber/__init__.py @@ -1,5 +1,6 @@ """Support for Tibber.""" +import asyncio from dataclasses import dataclass, field import logging @@ -52,6 +53,11 @@ class TibberRuntimeData: price_coordinator: TibberPriceCoordinator | None = field(default=None) _client: tibber.Tibber | None = None + @property + def client(self) -> tibber.Tibber | None: + """Return the cached Tibber client, if any.""" + return self._client + async def _async_get_access_token(self) -> str: """Return a valid Tibber access token.""" await self.session.async_ensure_token_valid() @@ -77,6 +83,19 @@ async def async_get_client(self, hass: HomeAssistant) -> tibber.Tibber: return self._client +async def _async_disconnect_client(client: tibber.Tibber | None) -> None: + """Disconnect the realtime connection without raising.""" + if client is None: + return + try: + async with asyncio.timeout(10): + await client.rt_disconnect() + except Exception: # noqa: BLE001 + _LOGGER.warning( + "Error disconnecting the Tibber realtime connection", exc_info=True + ) + + async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: """Set up the Tibber component.""" @@ -123,7 +142,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: TibberConfigEntry) -> bo tibber_connection = await entry.runtime_data.async_get_client(hass) async def _close(event: Event) -> None: - await tibber_connection.rt_disconnect() + await _async_disconnect_client(tibber_connection) entry.async_on_unload(hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _close)) @@ -168,6 +187,5 @@ async def async_unload_entry( if unload_ok := await hass.config_entries.async_unload_platforms( config_entry, PLATFORMS ): - tibber_connection = await config_entry.runtime_data.async_get_client(hass) - await tibber_connection.rt_disconnect() + await _async_disconnect_client(config_entry.runtime_data.client) return unload_ok diff --git a/tests/components/tibber/test_init.py b/tests/components/tibber/test_init.py index d1a94401d48500..2fd1167a6dfe01 100644 --- a/tests/components/tibber/test_init.py +++ b/tests/components/tibber/test_init.py @@ -21,6 +21,32 @@ async def test_entry_unload( entry = hass.config_entries.async_entry_for_domain_unique_id(DOMAIN, "tibber") assert entry.state is ConfigEntryState.LOADED + mock_tibber_setup.set_access_token.reset_mock() + await hass.config_entries.async_unload(entry.entry_id) + mock_tibber_setup.rt_disconnect.assert_called_once() + mock_tibber_setup.set_access_token.assert_not_called() + await hass.async_block_till_done(wait_background_tasks=True) + assert entry.state is ConfigEntryState.NOT_LOADED + + +@pytest.mark.parametrize( + "side_effect", + [ + pytest.param(TimeoutError(), id="timeout"), + pytest.param(Exception("Disconnect failed"), id="exception"), + ], +) +async def test_entry_unload_rt_disconnect_error( + recorder_mock: Recorder, + hass: HomeAssistant, + mock_tibber_setup: MagicMock, + side_effect: Exception, +) -> None: + """Test the entry unloads even if disconnecting the client fails.""" + entry = hass.config_entries.async_entry_for_domain_unique_id(DOMAIN, "tibber") + assert entry.state is ConfigEntryState.LOADED + + mock_tibber_setup.rt_disconnect.side_effect = side_effect await hass.config_entries.async_unload(entry.entry_id) mock_tibber_setup.rt_disconnect.assert_called_once() await hass.async_block_till_done(wait_background_tasks=True) From cf63edfe0e6fed9528ebd347f367c6cbd9955347 Mon Sep 17 00:00:00 2001 From: honzup <5564623+honzup@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:42:24 +0200 Subject: [PATCH 2/4] Exercise real timeout deadline in Tibber unload test Address the Copilot review comment on #176595: raising a preconstructed TimeoutError only tested exception suppression and would still pass without the timeout bound. Make rt_disconnect hang on an event that is never set and patch the disconnect timeout, now extracted into the DISCONNECT_TIMEOUT constant, to 0.05 seconds so the asyncio.timeout deadline genuinely expires and cancels it. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DERobfZZh3npFRbwpdxzz2 --- homeassistant/components/tibber/__init__.py | 5 ++++- tests/components/tibber/test_init.py | 14 +++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/tibber/__init__.py b/homeassistant/components/tibber/__init__.py index af16f021308ee4..e92ef7fd20400e 100644 --- a/homeassistant/components/tibber/__init__.py +++ b/homeassistant/components/tibber/__init__.py @@ -3,6 +3,7 @@ import asyncio from dataclasses import dataclass, field import logging +from typing import Final import aiohttp from aiohttp.client_exceptions import ClientError @@ -37,6 +38,8 @@ PLATFORMS = [Platform.BINARY_SENSOR, Platform.NOTIFY, Platform.SENSOR] +DISCONNECT_TIMEOUT: Final = 10 + CONFIG_SCHEMA = cv.config_entry_only_config_schema(DOMAIN) _LOGGER = logging.getLogger(__name__) @@ -88,7 +91,7 @@ async def _async_disconnect_client(client: tibber.Tibber | None) -> None: if client is None: return try: - async with asyncio.timeout(10): + async with asyncio.timeout(DISCONNECT_TIMEOUT): await client.rt_disconnect() except Exception: # noqa: BLE001 _LOGGER.warning( diff --git a/tests/components/tibber/test_init.py b/tests/components/tibber/test_init.py index 2fd1167a6dfe01..047c3ee9d664ef 100644 --- a/tests/components/tibber/test_init.py +++ b/tests/components/tibber/test_init.py @@ -1,5 +1,7 @@ """Test loading of the Tibber config entry.""" +import asyncio +from collections.abc import Awaitable, Callable from unittest.mock import ANY, AsyncMock, MagicMock, patch import pytest @@ -29,10 +31,15 @@ async def test_entry_unload( assert entry.state is ConfigEntryState.NOT_LOADED +async def _hang_forever() -> None: + """Simulate a realtime disconnect that never completes.""" + await asyncio.Event().wait() + + @pytest.mark.parametrize( "side_effect", [ - pytest.param(TimeoutError(), id="timeout"), + pytest.param(_hang_forever, id="timeout"), pytest.param(Exception("Disconnect failed"), id="exception"), ], ) @@ -40,14 +47,15 @@ async def test_entry_unload_rt_disconnect_error( recorder_mock: Recorder, hass: HomeAssistant, mock_tibber_setup: MagicMock, - side_effect: Exception, + side_effect: Exception | Callable[[], Awaitable[None]], ) -> None: """Test the entry unloads even if disconnecting the client fails.""" entry = hass.config_entries.async_entry_for_domain_unique_id(DOMAIN, "tibber") assert entry.state is ConfigEntryState.LOADED mock_tibber_setup.rt_disconnect.side_effect = side_effect - await hass.config_entries.async_unload(entry.entry_id) + with patch("homeassistant.components.tibber.DISCONNECT_TIMEOUT", 0.05): + await hass.config_entries.async_unload(entry.entry_id) mock_tibber_setup.rt_disconnect.assert_called_once() await hass.async_block_till_done(wait_background_tasks=True) assert entry.state is ConfigEntryState.NOT_LOADED From e6627fd533c05adbbcba375f0192ab1ae68f79ce Mon Sep 17 00:00:00 2001 From: honzup <5564623+honzup@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:47:30 +0200 Subject: [PATCH 3/4] Disconnect Tibber client via runtime data action method --- homeassistant/components/tibber/__init__.py | 32 +++++++++------------ tests/components/tibber/test_init.py | 32 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/homeassistant/components/tibber/__init__.py b/homeassistant/components/tibber/__init__.py index e92ef7fd20400e..68405a4b608b80 100644 --- a/homeassistant/components/tibber/__init__.py +++ b/homeassistant/components/tibber/__init__.py @@ -56,11 +56,6 @@ class TibberRuntimeData: price_coordinator: TibberPriceCoordinator | None = field(default=None) _client: tibber.Tibber | None = None - @property - def client(self) -> tibber.Tibber | None: - """Return the cached Tibber client, if any.""" - return self._client - async def _async_get_access_token(self) -> str: """Return a valid Tibber access token.""" await self.session.async_ensure_token_valid() @@ -85,18 +80,17 @@ async def async_get_client(self, hass: HomeAssistant) -> tibber.Tibber: await self._client.set_access_token(access_token) return self._client - -async def _async_disconnect_client(client: tibber.Tibber | None) -> None: - """Disconnect the realtime connection without raising.""" - if client is None: - return - try: - async with asyncio.timeout(DISCONNECT_TIMEOUT): - await client.rt_disconnect() - except Exception: # noqa: BLE001 - _LOGGER.warning( - "Error disconnecting the Tibber realtime connection", exc_info=True - ) + async def async_disconnect(self) -> None: + """Disconnect the cached realtime connection without raising.""" + if self._client is None: + return + try: + async with asyncio.timeout(DISCONNECT_TIMEOUT): + await self._client.rt_disconnect() + except Exception: # noqa: BLE001 + _LOGGER.warning( + "Error disconnecting the Tibber realtime connection", exc_info=True + ) async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: @@ -145,7 +139,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: TibberConfigEntry) -> bo tibber_connection = await entry.runtime_data.async_get_client(hass) async def _close(event: Event) -> None: - await _async_disconnect_client(tibber_connection) + await entry.runtime_data.async_disconnect() entry.async_on_unload(hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _close)) @@ -190,5 +184,5 @@ async def async_unload_entry( if unload_ok := await hass.config_entries.async_unload_platforms( config_entry, PLATFORMS ): - await _async_disconnect_client(config_entry.runtime_data.client) + await config_entry.runtime_data.async_disconnect() return unload_ok diff --git a/tests/components/tibber/test_init.py b/tests/components/tibber/test_init.py index 047c3ee9d664ef..90356f3e486a65 100644 --- a/tests/components/tibber/test_init.py +++ b/tests/components/tibber/test_init.py @@ -105,6 +105,38 @@ async def test_data_api_runtime_creates_client(hass: HomeAssistant) -> None: assert cached_client is client +@pytest.mark.usefixtures("recorder_mock") +async def test_async_get_client_propagates_rotated_token(hass: HomeAssistant) -> None: + """Ensure a rotated OAuth token is pushed to the cached client on next fetch.""" + session = MagicMock() + session.async_ensure_token_valid = AsyncMock() + session.token = {CONF_ACCESS_TOKEN: "token-1"} + + runtime = TibberRuntimeData( + session=session, + ) + + with patch("homeassistant.components.tibber.tibber.Tibber") as mock_client_cls: + mock_client = MagicMock() + mock_client.set_access_token = AsyncMock() + mock_client_cls.return_value = mock_client + + await runtime.async_get_client(hass) + mock_client_cls.assert_called_once_with( + access_token="token-1", + websession=ANY, + time_zone=ANY, + ssl=ANY, + refresh_access_token=ANY, + ) + + session.token = {CONF_ACCESS_TOKEN: "token-2"} + await runtime.async_get_client(hass) + + mock_client_cls.assert_called_once() + mock_client.set_access_token.assert_awaited_once_with("token-2") + + @pytest.mark.usefixtures("recorder_mock") async def test_data_api_runtime_missing_token_raises(hass: HomeAssistant) -> None: """Ensure missing tokens trigger reauthentication.""" From de33c6f80e1ca7841fea95a4f2e3dbf27f14e8cd Mon Sep 17 00:00:00 2001 From: honzup <5564623+honzup@users.noreply.github.com> Date: Mon, 27 Jul 2026 08:34:14 +0200 Subject: [PATCH 4/4] Test Tibber client handling through the integration Address joostlek's review on #176595: the runtime data tests reached past the integration and asserted on TibberRuntimeData internals. Drive them through config entry setup instead, so they exercise the same surface a user does. The token rotation test now updates the stored OAuth token on the config entry and triggers a client fetch through the notify entity, rather than constructing a runtime object and calling async_get_client directly. The missing token test asserts the entry ends in SETUP_ERROR with a reauth flow, which is the observable outcome, instead of catching ConfigEntryAuthFailed from the helper. A coordinator tick cannot drive these tests: the mocked client reports no homes, so nothing subscribes to the Data API coordinator and it never schedules a periodic refresh. Split the patched Tibber class out of the tibber_mock fixture as tibber_client_cls so tests can assert on construction. tibber_mock still yields the client instance, so other tests are unaffected. --- tests/components/tibber/conftest.py | 12 ++- tests/components/tibber/test_init.py | 153 +++++++++++++-------------- 2 files changed, 83 insertions(+), 82 deletions(-) diff --git a/tests/components/tibber/conftest.py b/tests/components/tibber/conftest.py index 7304803dc74753..3bf00d9a16a76a 100644 --- a/tests/components/tibber/conftest.py +++ b/tests/components/tibber/conftest.py @@ -220,8 +220,8 @@ def config_entry(hass: HomeAssistant) -> MockConfigEntry: @pytest.fixture -def tibber_mock() -> AsyncGenerator[MagicMock]: - """Patch the Tibber libraries used by the integration.""" +def tibber_client_cls() -> AsyncGenerator[MagicMock]: + """Patch the Tibber class used by the integration.""" unique_user_id = "unique_user_id" title = "title" @@ -246,7 +246,13 @@ def tibber_mock() -> AsyncGenerator[MagicMock]: data_api_mock.get_userinfo = AsyncMock() tibber_mock.data_api = data_api_mock - yield tibber_mock + yield mock_tibber + + +@pytest.fixture +def tibber_mock(tibber_client_cls: MagicMock) -> MagicMock: + """Return the patched Tibber client instance.""" + return tibber_client_cls.return_value @pytest.fixture diff --git a/tests/components/tibber/test_init.py b/tests/components/tibber/test_init.py index 90356f3e486a65..066727462bd7b1 100644 --- a/tests/components/tibber/test_init.py +++ b/tests/components/tibber/test_init.py @@ -2,16 +2,17 @@ import asyncio from collections.abc import Awaitable, Callable -from unittest.mock import ANY, AsyncMock, MagicMock, patch +import time +from unittest.mock import ANY, MagicMock, patch import pytest from homeassistant.components.recorder import Recorder -from homeassistant.components.tibber import DOMAIN, TibberRuntimeData -from homeassistant.config_entries import ConfigEntryState +from homeassistant.components.tibber import DOMAIN +from homeassistant.components.tibber.const import AUTH_IMPLEMENTATION +from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState from homeassistant.const import CONF_ACCESS_TOKEN from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ConfigEntryAuthFailed from tests.common import MockConfigEntry @@ -61,96 +62,90 @@ async def test_entry_unload_rt_disconnect_error( assert entry.state is ConfigEntryState.NOT_LOADED -@pytest.mark.usefixtures("recorder_mock") -async def test_data_api_runtime_creates_client(hass: HomeAssistant) -> None: - """Ensure the data API runtime creates and caches the client.""" - session = MagicMock() - session.async_ensure_token_valid = AsyncMock() - session.token = {CONF_ACCESS_TOKEN: "access-token"} - - runtime = TibberRuntimeData( - session=session, +async def _async_trigger_client_fetch(hass: HomeAssistant) -> None: + """Make the integration fetch its client through a public entity action.""" + await hass.services.async_call( + "notify", + "send_message", + {"entity_id": "notify.tibber", "message": "message"}, + blocking=True, ) - with patch("homeassistant.components.tibber.tibber.Tibber") as mock_client_cls: - mock_client = MagicMock() - mock_client.set_access_token = AsyncMock() - mock_client_cls.return_value = mock_client - - client = await runtime.async_get_client(hass) - - mock_client_cls.assert_called_once_with( - access_token="access-token", - websession=ANY, - time_zone=ANY, - ssl=ANY, - refresh_access_token=ANY, - ) - session.async_ensure_token_valid.assert_awaited_once() - mock_client.set_access_token.assert_not_awaited() - assert client is mock_client - refresh_access_token = mock_client_cls.call_args.kwargs["refresh_access_token"] - session.async_ensure_token_valid.reset_mock() - assert await refresh_access_token() == "access-token" - session.async_ensure_token_valid.assert_awaited_once() - - session.async_ensure_token_valid.reset_mock() +@pytest.mark.usefixtures("recorder_mock", "mock_tibber_setup") +async def test_client_created_once_and_reused( + hass: HomeAssistant, + tibber_client_cls: MagicMock, + tibber_mock: MagicMock, +) -> None: + """Ensure setup builds one authenticated client that later fetches reuse.""" + tibber_client_cls.assert_called_once_with( + access_token="test-token", + websession=ANY, + time_zone=ANY, + ssl=ANY, + refresh_access_token=ANY, + ) - cached_client = await runtime.async_get_client(hass) + # The library refreshes its own realtime token through this callback. + refresh_access_token = tibber_client_cls.call_args.kwargs["refresh_access_token"] + assert await refresh_access_token() == "test-token" - mock_client_cls.assert_called_once() - session.async_ensure_token_valid.assert_awaited_once() - mock_client.set_access_token.assert_awaited_once_with("access-token") - assert cached_client is client + tibber_mock.set_access_token.reset_mock() + await _async_trigger_client_fetch(hass) + tibber_client_cls.assert_called_once() + tibber_mock.set_access_token.assert_awaited_once_with("test-token") -@pytest.mark.usefixtures("recorder_mock") -async def test_async_get_client_propagates_rotated_token(hass: HomeAssistant) -> None: - """Ensure a rotated OAuth token is pushed to the cached client on next fetch.""" - session = MagicMock() - session.async_ensure_token_valid = AsyncMock() - session.token = {CONF_ACCESS_TOKEN: "token-1"} - runtime = TibberRuntimeData( - session=session, +@pytest.mark.usefixtures("recorder_mock", "mock_tibber_setup") +async def test_rotated_token_pushed_to_cached_client( + hass: HomeAssistant, + config_entry: MockConfigEntry, + tibber_client_cls: MagicMock, + tibber_mock: MagicMock, +) -> None: + """Ensure a rotated OAuth token reaches the cached client on the next fetch.""" + hass.config_entries.async_update_entry( + config_entry, + data={ + **config_entry.data, + "token": {**config_entry.data["token"], CONF_ACCESS_TOKEN: "token-2"}, + }, ) - with patch("homeassistant.components.tibber.tibber.Tibber") as mock_client_cls: - mock_client = MagicMock() - mock_client.set_access_token = AsyncMock() - mock_client_cls.return_value = mock_client - - await runtime.async_get_client(hass) - mock_client_cls.assert_called_once_with( - access_token="token-1", - websession=ANY, - time_zone=ANY, - ssl=ANY, - refresh_access_token=ANY, - ) + tibber_mock.set_access_token.reset_mock() + await _async_trigger_client_fetch(hass) - session.token = {CONF_ACCESS_TOKEN: "token-2"} - await runtime.async_get_client(hass) + tibber_client_cls.assert_called_once() + tibber_mock.set_access_token.assert_awaited_once_with("token-2") - mock_client_cls.assert_called_once() - mock_client.set_access_token.assert_awaited_once_with("token-2") - -@pytest.mark.usefixtures("recorder_mock") -async def test_data_api_runtime_missing_token_raises(hass: HomeAssistant) -> None: - """Ensure missing tokens trigger reauthentication.""" - session = MagicMock() - session.async_ensure_token_valid = AsyncMock() - session.token = {} - - runtime = TibberRuntimeData( - session=session, +@pytest.mark.usefixtures("recorder_mock", "tibber_mock", "setup_credentials") +async def test_setup_missing_access_token_triggers_reauth( + hass: HomeAssistant, +) -> None: + """Ensure an OAuth token without an access token triggers reauthentication.""" + entry = MockConfigEntry( + domain=DOMAIN, + data={ + AUTH_IMPLEMENTATION: DOMAIN, + "token": { + "refresh_token": "refresh-token", + "token_type": "Bearer", + "expires_at": time.time() + 3600, + }, + }, + unique_id="tibber", ) + entry.add_to_hass(hass) - with pytest.raises(ConfigEntryAuthFailed): - await runtime.async_get_client(hass) - session.async_ensure_token_valid.assert_awaited_once() + await hass.config_entries.async_setup(entry.entry_id) + + assert entry.state is ConfigEntryState.SETUP_ERROR + flows = hass.config_entries.flow.async_progress() + assert len(flows) == 1 + assert flows[0]["context"]["source"] == SOURCE_REAUTH async def test_setup_requires_data_api_reauth(hass: HomeAssistant) -> None: