From 0a5441779a04b464b9d517cd8cf1a9ac09d7c66d Mon Sep 17 00:00:00 2001 From: flan Date: Tue, 4 Aug 2026 16:04:27 +0000 Subject: [PATCH] Fix produce-box select cancelling config entry setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit async_added_to_hass listed the switchable boxes inline, one fetch per box popup, so on a slower connection the platform setup ran past SLOW_SETUP_MAX_WAIT and the whole config entry was cancelled into setup_error — every entity unavailable despite valid credentials. Move the listing to a background task so setup never blocks on it; the options fill in once it returns. Also bound every portal request to a 30s timeout so one hung request can no longer drag a refresh, or a first setup, past Home Assistant's own limits and fail it outright. Regression covered in tests/test_setup_hygiene.py. --- CHANGELOG.md | 18 +++++ custom_components/freshharvest/actions.py | 27 +++++--- custom_components/freshharvest/api.py | 26 +++++-- custom_components/freshharvest/manifest.json | 2 +- custom_components/freshharvest/select.py | 20 +++++- custom_components/freshharvest/todo.py | 3 +- tests/test_actions.py | 1 + tests/test_parser.py | 1 + tests/test_setup_hygiene.py | 73 ++++++++++++++++++++ 9 files changed, 152 insertions(+), 19 deletions(-) create mode 100644 tests/test_setup_hygiene.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b5718b7..ae30496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.4.1] - 2026-08-04 + +### Fixed + +- The produce-box select listed the boxes on offer from inside + `async_added_to_hass`, one fetch per box popup. That ran during entity setup, + so on a slower connection those fetches exceeded Home Assistant's + `SLOW_SETUP_MAX_WAIT`, the platform was cancelled, and the whole config entry + landed in `setup_error` — every entity `unavailable` despite valid + credentials. The listing now runs once in the background: the entity comes up + immediately with the current box as its option and the rest fill in when the + listing returns. Regression covered in `tests/test_setup_hygiene.py`. +- Every portal request now carries a 30-second timeout. The shared Home + Assistant session otherwise inherits aiohttp's five-minute default, long + enough for one hung request to drag a refresh — or a first setup — past Home + Assistant's own limits and fail it outright. A slow or unreachable site now + surfaces as a normal retry instead. + ## [0.4.0] - 2026-08-03 Control, not just reporting: the box can now be managed from Home Assistant. diff --git a/custom_components/freshharvest/actions.py b/custom_components/freshharvest/actions.py index 7799847..caa964b 100644 --- a/custom_components/freshharvest/actions.py +++ b/custom_components/freshharvest/actions.py @@ -25,11 +25,18 @@ import time from dataclasses import dataclass, field from datetime import date +import aiohttp from bs4 import BeautifulSoup from yarl import URL -from .api import BASE, USER_AGENT, FreshHarvestClient, FreshHarvestError +from .api import ( + BASE, + REQUEST_TIMEOUT, + USER_AGENT, + FreshHarvestClient, + FreshHarvestError, +) _LOGGER = logging.getLogger(__name__) @@ -319,13 +326,17 @@ class FreshHarvestActions(BasketMixin): async def _post(self, path: str, payload: dict[str, str]) -> str: session = self._client._session # noqa: SLF001 — same package - async with session.post( - BASE.join(URL(path)), - data=payload, - headers={"User-Agent": USER_AGENT}, - ) as resp: - resp.raise_for_status() - return await resp.text() + try: + async with session.post( + BASE.join(URL(path)), + data=payload, + headers={"User-Agent": USER_AGENT}, + timeout=REQUEST_TIMEOUT, + ) as resp: + resp.raise_for_status() + return await resp.text() + except (aiohttp.ClientError, TimeoutError) as err: + raise FreshHarvestError(f"post to {path} failed: {err}") from err # ------------------------------------------------------------------ skip diff --git a/custom_components/freshharvest/api.py b/custom_components/freshharvest/api.py index 28d2419..0650eb7 100644 --- a/custom_components/freshharvest/api.py +++ b/custom_components/freshharvest/api.py @@ -33,6 +33,14 @@ USER_AGENT = ( "Chrome/126.0 Safari/537.36" ) +# Bound every portal request. The shared Home Assistant session otherwise +# inherits aiohttp's five-minute default, long enough for one hung request to +# drag a refresh — or, during setup, an entire config entry — past Home +# Assistant's own timeouts and into a hard failure. Thirty seconds sits far +# above the portal's normal response yet still fails fast when it is +# unreachable, so a slow site becomes a retry rather than a broken entry. +REQUEST_TIMEOUT = aiohttp.ClientTimeout(total=30, connect=10) + # Hidden anti-replay fields, minted per session on each GET of the login form. _HIDDEN_RE = re.compile( r"name='(?PLoginSecurity|SubmitToken)'[^>]*value='(?P[^']*)'" @@ -315,11 +323,16 @@ class FreshHarvestClient: self._authenticated = False async def _get(self, path: str) -> str: - async with self._session.get( - BASE.join(URL(path)), headers={"User-Agent": USER_AGENT} - ) as resp: - resp.raise_for_status() - return await resp.text() + try: + async with self._session.get( + BASE.join(URL(path)), + headers={"User-Agent": USER_AGENT}, + timeout=REQUEST_TIMEOUT, + ) as resp: + resp.raise_for_status() + return await resp.text() + except (aiohttp.ClientError, TimeoutError) as err: + raise FreshHarvestError(f"request for {path} failed: {err}") from err async def async_login(self) -> None: """Run the two-step handshake: fetch tokens, then post credentials. @@ -347,10 +360,11 @@ class FreshHarvestClient: BASE.join(URL(LOGIN_SUBMIT)), data=payload, headers={"User-Agent": USER_AGENT}, + timeout=REQUEST_TIMEOUT, ) as resp: resp.raise_for_status() body = await resp.text() - except aiohttp.ClientError as err: + except (aiohttp.ClientError, TimeoutError) as err: raise FreshHarvestError(f"login request failed: {err}") from err if not self._signed_in(body): diff --git a/custom_components/freshharvest/manifest.json b/custom_components/freshharvest/manifest.json index 507caea..97521c9 100644 --- a/custom_components/freshharvest/manifest.json +++ b/custom_components/freshharvest/manifest.json @@ -12,5 +12,5 @@ "requirements": [ "beautifulsoup4>=4.12" ], - "version": "0.4.0" + "version": "0.4.1" } diff --git a/custom_components/freshharvest/select.py b/custom_components/freshharvest/select.py index 12bc6cc..66aa8af 100644 --- a/custom_components/freshharvest/select.py +++ b/custom_components/freshharvest/select.py @@ -20,6 +20,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback from . import FreshHarvestConfigEntry from .api import FreshHarvestError +from .const import DOMAIN from .coordinator import FreshHarvestCoordinator from .entity import FreshHarvestEntity @@ -79,13 +80,26 @@ class FreshHarvestBoxSelect(FreshHarvestEntity, SelectEntity): async def async_added_to_hass(self) -> None: await super().async_added_to_hass() - # Listing costs a fetch per box popup, so it happens once on setup - # rather than on every coordinator refresh. + # Listing the boxes on offer costs one fetch per box popup. Doing it + # here inline blocked entity setup: on a slower connection those fetches + # ran past Home Assistant's SLOW_SETUP_MAX_WAIT and the whole config + # entry was cancelled into "setup_error", leaving every entity + # unavailable despite valid credentials. It now runs once, in the + # background — the entity comes up immediately with the current box as + # its only option and the rest appear when the listing returns. + self.coordinator.config_entry.async_create_background_task( + self.hass, self._async_load_options(), f"{DOMAIN}_list_baskets" + ) + + async def _async_load_options(self) -> None: + """Fetch the switchable boxes once and publish them as options.""" try: baskets = await self.coordinator.actions.async_list_baskets() - self._options = [b.name for b in baskets] except FreshHarvestError as err: _LOGGER.warning("could not list produce boxes: %s", err) + return + self._options = [b.name for b in baskets] + self.async_write_ha_state() async def async_select_option(self, option: str) -> None: """Switch the next delivery to this box.""" diff --git a/custom_components/freshharvest/todo.py b/custom_components/freshharvest/todo.py index b1e2199..2516027 100644 --- a/custom_components/freshharvest/todo.py +++ b/custom_components/freshharvest/todo.py @@ -39,7 +39,7 @@ from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddEntitiesCallback from . import FreshHarvestConfigEntry -from .api import FreshHarvestError +from .api import REQUEST_TIMEOUT, FreshHarvestError from .coordinator import FreshHarvestCoordinator from .entity import FreshHarvestEntity @@ -129,6 +129,7 @@ class FreshHarvestBox(FreshHarvestEntity, TodoListEntity): "X-Algolia-Application-Id": app, "Content-Type": "application/json", }, + timeout=REQUEST_TIMEOUT, ) as resp: resp.raise_for_status() hits = (await resp.json()).get("hits") or [] diff --git a/tests/test_actions.py b/tests/test_actions.py index 25ba0cb..170095b 100644 --- a/tests/test_actions.py +++ b/tests/test_actions.py @@ -20,6 +20,7 @@ def _load(): stub = types.ModuleType("aiohttp") stub.ClientSession = object stub.ClientError = Exception + stub.ClientTimeout = lambda *args, **kwargs: None sys.modules["aiohttp"] = stub sys.path.insert(0, str(COMPONENT)) pkg = types.ModuleType("fh_pkg") diff --git a/tests/test_parser.py b/tests/test_parser.py index fefcd53..264679a 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -26,6 +26,7 @@ def _load_api(): stub = types.ModuleType("aiohttp") stub.ClientSession = object stub.ClientError = Exception + stub.ClientTimeout = lambda *args, **kwargs: None sys.modules["aiohttp"] = stub if "yarl" not in sys.modules: try: diff --git a/tests/test_setup_hygiene.py b/tests/test_setup_hygiene.py new file mode 100644 index 0000000..fdf03fa --- /dev/null +++ b/tests/test_setup_hygiene.py @@ -0,0 +1,73 @@ +"""No entity may block its own setup on a portal round-trip. + +Written after a real bug: the produce-box select listed the boxes on offer from +inside `async_added_to_hass`, one fetch per popup. On a slower connection those +fetches ran past Home Assistant's SLOW_SETUP_MAX_WAIT, the platform setup was +cancelled, and the whole config entry landed in `setup_error` — every entity +unavailable despite valid credentials. + +Every portal call an entity makes goes through `self.coordinator.actions` or +`self.coordinator.client`, so awaiting either inside `async_added_to_hass` is +the shape of that bug. These parse the sources with `ast` rather than importing +them, so the suite still runs without Home Assistant installed. +""" + +from __future__ import annotations + +import ast +from pathlib import Path + +COMPONENT = Path(__file__).parent.parent / "custom_components" / "freshharvest" +# Attribute names that only exist to reach the freshharvest.com portal. +NETWORK_ATTRS = {"actions", "client"} + + +def _attr_chain(node: ast.AST) -> set[str]: + """Every attribute/name along a call's dotted func, e.g. a.b.c() -> {a,b,c}.""" + names: set[str] = set() + while isinstance(node, ast.Attribute): + names.add(node.attr) + node = node.value + if isinstance(node, ast.Name): + names.add(node.id) + return names + + +def _added_to_hass(filename: str) -> ast.AsyncFunctionDef | None: + tree = ast.parse((COMPONENT / filename).read_text(encoding="utf-8")) + for node in ast.walk(tree): + if ( + isinstance(node, ast.AsyncFunctionDef) + and node.name == "async_added_to_hass" + ): + return node + return None + + +PLATFORMS = ["sensor.py", "binary_sensor.py", "switch.py", "button.py", + "select.py", "todo.py"] + + +def test_no_platform_awaits_the_portal_during_setup(): + """`async_added_to_hass` must not await a coordinator.actions/client call.""" + offenders = [] + for filename in PLATFORMS: + func = _added_to_hass(filename) + if func is None: + continue + for node in ast.walk(func): + if isinstance(node, ast.Await) and isinstance(node.value, ast.Call): + if _attr_chain(node.value.func) & NETWORK_ATTRS: + offenders.append(f"{filename}:{node.lineno}") + assert not offenders, ( + "portal round-trip awaited during entity setup (blocks setup, can " + f"exceed SLOW_SETUP_MAX_WAIT): {offenders}" + ) + + +def test_select_still_loads_its_options_off_the_setup_path(): + """The fix must defer the listing, not delete it: the select still fetches + its options, just in a background task rather than inline in setup.""" + source = (COMPONENT / "select.py").read_text(encoding="utf-8") + assert "async_create_background_task" in source + assert "async_list_baskets" in source