Merge remote-tracking branch 'origin/main' into claude/ruff-pyright-ci-stage-gxph0t
This commit is contained in:
@@ -5,7 +5,8 @@ from __future__ import annotations
|
||||
import logging
|
||||
|
||||
from homeassistant.config_entries import ConfigEntry
|
||||
from homeassistant.core import HomeAssistant
|
||||
from homeassistant.const import EVENT_HOMEASSISTANT_STOP
|
||||
from homeassistant.core import Event, HomeAssistant
|
||||
from homeassistant.exceptions import ConfigEntryNotReady
|
||||
from homeassistant.helpers import device_registry as dr
|
||||
|
||||
@@ -21,8 +22,38 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
|
||||
try:
|
||||
await coordinator.async_config_entry_first_refresh()
|
||||
except Exception as err:
|
||||
# `_poll_once` deliberately leaves the session up on a `TimeoutError`
|
||||
# (the transfer may just be slow -- see its docstring), so a refresh
|
||||
# that fails that way ends here with a live, bound UDP socket that
|
||||
# nothing would ever close. HA retries setup on its own backoff with
|
||||
# a *new* coordinator, and each device's source port is fixed by
|
||||
# design (`_local_source_port`), so an abandoned socket squats the
|
||||
# exact port the next attempt binds -- SO_REUSEADDR lets that bind
|
||||
# succeed, leaving two sockets racing for the device's datagrams.
|
||||
await coordinator.async_close()
|
||||
raise ConfigEntryNotReady(f"Cannot connect to device: {err}") from err
|
||||
hass.data[DOMAIN][entry.entry_id] = coordinator
|
||||
|
||||
# Send the DTLS close_notify on Core shutdown, not just on unload (issue
|
||||
# #254). `async_close` otherwise only runs via `async_unload_entry`, and
|
||||
# HA does not unload entries on a plain Core restart -- so a restart left
|
||||
# the previous run's association orphaned on the appliance, which is what
|
||||
# makes the *next* run's handshake time out. Complements the fixed source
|
||||
# port, which covers the unclean-exit case this cannot (see
|
||||
# `_local_source_port`).
|
||||
#
|
||||
# A coroutine listener, not one that spawns its own task: the event bus
|
||||
# runs it as a hass-tracked job, so the close is awaited by the
|
||||
# `async_block_till_done()` inside `hass.async_stop`. A detached task
|
||||
# would likely be cancelled mid-shutdown -- the exact no-close_notify
|
||||
# case this exists to prevent.
|
||||
async def _async_close_on_stop(_event: Event) -> None:
|
||||
await coordinator.async_close()
|
||||
|
||||
entry.async_on_unload(
|
||||
hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _async_close_on_stop)
|
||||
)
|
||||
|
||||
await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
|
||||
return True
|
||||
|
||||
|
||||
@@ -826,7 +826,21 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
|
||||
normal on a flaky device; a run of them is a real problem. Any
|
||||
other exception (a `ConnectionError`, an explicitly closed
|
||||
session) is unambiguous and always reconnects immediately.
|
||||
|
||||
Never defers before the first successful discovery (issue #254).
|
||||
Deferring is a *mid-session* judgement call — "keep the entities we
|
||||
already have and try again next cycle" — which is only coherent once
|
||||
there are entities to keep. Pre-discovery the same exception type
|
||||
means something else entirely: `_poll_once` calls `_connect_session`,
|
||||
so `connect()`'s own handshake timeout surfaces here as a
|
||||
`TimeoutError` too, and that is a dead connection, not a slow
|
||||
transfer. Deferring it returned an empty dict instead of raising,
|
||||
which `DataUpdateCoordinator` counts as a successful first refresh —
|
||||
and since platforms enumerate `bound` exactly once, the entry loaded
|
||||
with zero entities and stayed that way until a manual reload.
|
||||
"""
|
||||
if not self._discovered:
|
||||
return False
|
||||
if not isinstance(e, TimeoutError):
|
||||
return False
|
||||
if self._observe.mode == MODE_OBSERVE and self._observe.recently_notified():
|
||||
@@ -893,7 +907,15 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
|
||||
except Exception as e2:
|
||||
self._log.error("poll failed after reconnect: %s", e2)
|
||||
snapshot = self._cache.snapshot()
|
||||
if snapshot:
|
||||
# `self._discovered` is the same precondition
|
||||
# `_defer_reconnect_for` applies (issue #254): returning
|
||||
# degraded-but-successful data is only meaningful once
|
||||
# there are bound entities to carry it. Pre-discovery the
|
||||
# cache happens to always be empty -- every apply() site
|
||||
# is gated on post-discovery state -- so this arm is
|
||||
# unreachable then, but that is a non-local accident
|
||||
# across four call sites, not something to rely on.
|
||||
if self._discovered and snapshot:
|
||||
self._log.debug("Full error:", exc_info=e2)
|
||||
return flatten(self.bound, snapshot)
|
||||
raise UpdateFailed(f"poll failed after reconnect: {e2}") from e2
|
||||
|
||||
@@ -7,6 +7,8 @@ from unittest.mock import AsyncMock, patch
|
||||
|
||||
import cbor2
|
||||
import pytest
|
||||
from homeassistant.config_entries import ConfigEntryState
|
||||
from homeassistant.const import EVENT_HOMEASSISTANT_STOP
|
||||
from homeassistant.core import HomeAssistant
|
||||
from homeassistant.exceptions import ServiceValidationError
|
||||
from homeassistant.helpers import issue_registry as ir
|
||||
@@ -1281,3 +1283,116 @@ def test_local_source_port_non_ipv4_falls_back_to_hash() -> None:
|
||||
port = _local_source_port("some-hostname")
|
||||
assert port == _local_source_port("some-hostname")
|
||||
assert DTLS_LOCAL_PORT_BASE <= port <= DTLS_LOCAL_PORT_BASE + 0xFF
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# Issue #254: a pre-discovery poll failure must never look like a
|
||||
# successful first refresh. See `_defer_reconnect_for` for the mechanism.
|
||||
# ----------------------------------------------------------------------
|
||||
|
||||
_HANDSHAKE_TIMEOUT = TimeoutError('DTLS handshake timed out')
|
||||
|
||||
|
||||
async def test_first_refresh_timeout_recovers_via_reconnect(
|
||||
hass: HomeAssistant, mock_entry, fridge_resources
|
||||
) -> None:
|
||||
"""A pre-discovery handshake timeout reconnects instead of deferring."""
|
||||
with (
|
||||
patch('custom_components.localthings.coordinator.LocalThingsCoordinator._connect_session'),
|
||||
patch(
|
||||
'custom_components.localthings.coordinator.LocalThingsCoordinator._poll_once',
|
||||
side_effect=[_HANDSHAKE_TIMEOUT, fridge_resources],
|
||||
) as mock_poll,
|
||||
patch('custom_components.localthings.coordinator.LocalThingsCoordinator._close_session'),
|
||||
):
|
||||
await hass.config_entries.async_setup(mock_entry.entry_id)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
# The reconnect inside the first refresh is what makes this recoverable
|
||||
# without HA having to fail and re-run setup at all.
|
||||
assert mock_poll.call_count == 2
|
||||
assert mock_entry.state is ConfigEntryState.LOADED
|
||||
coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id]
|
||||
assert coordinator.bound, "discovery must have run before platforms are forwarded"
|
||||
# The regression is entity-visible, not just coordinator state: platforms
|
||||
# enumerate `bound` once, so an empty `bound` at forward time leaves the
|
||||
# device with no entities until a manual reload.
|
||||
assert hass.states.async_all(), "device came up with zero entities"
|
||||
# The reconnect handled this failure, so it must not also be charged
|
||||
# against the mid-session timeout budget.
|
||||
assert coordinator._consecutive_poll_timeouts == 0
|
||||
|
||||
|
||||
async def test_first_refresh_persistent_timeout_fails_setup(
|
||||
hass: HomeAssistant, mock_entry
|
||||
) -> None:
|
||||
"""When the reconnect times out too, the first refresh must fail so HA
|
||||
retries on its backoff -- not load an entity-less entry. The session it
|
||||
left open is closed on the way out (`_poll_once` keeps it up on a
|
||||
`TimeoutError`, and the source port is fixed per device)."""
|
||||
with (
|
||||
patch('custom_components.localthings.coordinator.LocalThingsCoordinator._connect_session'),
|
||||
patch(
|
||||
'custom_components.localthings.coordinator.LocalThingsCoordinator._poll_once',
|
||||
side_effect=_HANDSHAKE_TIMEOUT,
|
||||
),
|
||||
patch('custom_components.localthings.coordinator.LocalThingsCoordinator._close_session'),
|
||||
# Asserted on `async_close` rather than `_close_session`: the
|
||||
# reconnect path calls the latter on its own, so it is already true
|
||||
# whether or not setup cleans up after itself.
|
||||
patch.object(
|
||||
LocalThingsCoordinator, 'async_close', autospec=True,
|
||||
) as mock_async_close,
|
||||
):
|
||||
await hass.config_entries.async_setup(mock_entry.entry_id)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
assert mock_entry.state is ConfigEntryState.SETUP_RETRY
|
||||
assert not hass.states.async_all()
|
||||
mock_async_close.assert_awaited_once()
|
||||
|
||||
|
||||
async def test_post_discovery_timeout_is_still_deferred(
|
||||
hass: HomeAssistant, mock_entry, mock_coordinator_session
|
||||
) -> None:
|
||||
"""The gate is scoped to the pre-discovery case only: once discovery has
|
||||
run, a lone poll timeout must still be absorbed without a reconnect (the
|
||||
slow-blockwise-transfer behavior `_POLL_TIMEOUT_LIMIT` exists for)."""
|
||||
await hass.config_entries.async_setup(mock_entry.entry_id)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id]
|
||||
assert coordinator._discovered
|
||||
|
||||
with (
|
||||
patch(
|
||||
'custom_components.localthings.coordinator.LocalThingsCoordinator._poll_once',
|
||||
side_effect=TimeoutError('GET /device/0 block 11 timeout'),
|
||||
),
|
||||
patch(
|
||||
'custom_components.localthings.coordinator.LocalThingsCoordinator._close_session',
|
||||
) as mock_close,
|
||||
):
|
||||
await coordinator.async_request_refresh()
|
||||
await hass.async_block_till_done()
|
||||
|
||||
mock_close.assert_not_called()
|
||||
assert coordinator._consecutive_poll_timeouts == 1
|
||||
assert coordinator.last_update_success
|
||||
|
||||
|
||||
async def test_close_notify_sent_on_homeassistant_stop(
|
||||
hass: HomeAssistant, mock_entry, mock_coordinator_session
|
||||
) -> None:
|
||||
"""A plain Core restart must close the DTLS session: HA does not unload
|
||||
entries on restart, so `async_unload_entry` never runs and the
|
||||
association would be left orphaned on the appliance."""
|
||||
await hass.config_entries.async_setup(mock_entry.entry_id)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id]
|
||||
with patch.object(coordinator, 'async_close', new=AsyncMock()) as mock_close:
|
||||
hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
mock_close.assert_awaited_once()
|
||||
|
||||
Reference in New Issue
Block a user