Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions homeassistant/components/tibber/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
"""Support for Tibber."""

import asyncio
from dataclasses import dataclass, field
import logging
from typing import Final

import aiohttp
from aiohttp.client_exceptions import ClientError
Expand Down Expand Up @@ -36,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__)
Expand Down Expand Up @@ -76,6 +80,18 @@ 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(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()
Comment on lines +88 to +89

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, and worth spelling out. rt_disconnect() reaches _reset_connection(), which takes LOCK_CONNECT before _reset_connection_locked() cancels the watchdog. reconnect() holds that same lock, and gets to it through an unbounded token refresh. So if the watchdog is mid-reconnect when the entry unloads, the timeout cancels the lock acquisition rather than the disconnect, and the watchdog survives.

It's a deliberate trade rather than a fault this PR introduces. Before it, the unload awaited rt_disconnect() unbounded: the watchdog did always stop eventually, but a half-dead socket could block the unload indefinitely and leave the entry in FAILED_UNLOAD, which needs a Home Assistant restart to clear. This swaps a state the user can't recover from for a task that can leak in a narrow window, and I'd rather leak the task.

The force-stop path you describe belongs in pyTibber rather than here - the integration can't cancel the library's watchdog without reaching into private attributes. Part of it is already open as Danielhiversen/pyTibber#435, which makes the connect lock per-instance and bounds both wait_closed() and close_async(). That narrows the window without closing it: there's still no cancellation path that runs before the lock is taken, and the token refresh inside reconnect() is unbounded.

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."""
Expand Down Expand Up @@ -123,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 tibber_connection.rt_disconnect()
await entry.runtime_data.async_disconnect()

entry.async_on_unload(hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _close))

Expand Down Expand Up @@ -168,6 +184,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 config_entry.runtime_data.async_disconnect()
return unload_ok
12 changes: 9 additions & 3 deletions tests/components/tibber/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Expand Down
157 changes: 109 additions & 48 deletions tests/components/tibber/test_init.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
"""Test loading of the Tibber config entry."""

from unittest.mock import ANY, AsyncMock, MagicMock, patch
import asyncio
from collections.abc import Awaitable, Callable
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

Expand All @@ -21,70 +24,128 @@ 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.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"}
async def _hang_forever() -> None:
"""Simulate a realtime disconnect that never completes."""
await asyncio.Event().wait()

runtime = TibberRuntimeData(
session=session,

@pytest.mark.parametrize(
"side_effect",
[
pytest.param(_hang_forever, 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 | 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
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


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)
@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,
)

# 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_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
tibber_mock.set_access_token.reset_mock()
await _async_trigger_client_fetch(hass)

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()
tibber_client_cls.assert_called_once()
tibber_mock.set_access_token.assert_awaited_once_with("test-token")

session.async_ensure_token_valid.reset_mock()

cached_client = await runtime.async_get_client(hass)
@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"},
},
)

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("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:
Expand Down
Loading