Merge fix/select-setup-nonblocking: keep config entry setup off the portal round-trip
This commit is contained in:
@@ -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/),
|
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).
|
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
|
## [0.4.0] - 2026-08-03
|
||||||
|
|
||||||
Control, not just reporting: the box can now be managed from Home Assistant.
|
Control, not just reporting: the box can now be managed from Home Assistant.
|
||||||
|
|||||||
@@ -25,11 +25,18 @@ import time
|
|||||||
from dataclasses import dataclass, field
|
from dataclasses import dataclass, field
|
||||||
from datetime import date
|
from datetime import date
|
||||||
|
|
||||||
|
import aiohttp
|
||||||
from bs4 import BeautifulSoup
|
from bs4 import BeautifulSoup
|
||||||
|
|
||||||
from yarl import URL
|
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__)
|
_LOGGER = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -319,13 +326,17 @@ class FreshHarvestActions(BasketMixin):
|
|||||||
|
|
||||||
async def _post(self, path: str, payload: dict[str, str]) -> str:
|
async def _post(self, path: str, payload: dict[str, str]) -> str:
|
||||||
session = self._client._session # noqa: SLF001 — same package
|
session = self._client._session # noqa: SLF001 — same package
|
||||||
async with session.post(
|
try:
|
||||||
BASE.join(URL(path)),
|
async with session.post(
|
||||||
data=payload,
|
BASE.join(URL(path)),
|
||||||
headers={"User-Agent": USER_AGENT},
|
data=payload,
|
||||||
) as resp:
|
headers={"User-Agent": USER_AGENT},
|
||||||
resp.raise_for_status()
|
timeout=REQUEST_TIMEOUT,
|
||||||
return await resp.text()
|
) 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
|
# ------------------------------------------------------------------ skip
|
||||||
|
|
||||||
|
|||||||
@@ -33,6 +33,14 @@ USER_AGENT = (
|
|||||||
"Chrome/126.0 Safari/537.36"
|
"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 anti-replay fields, minted per session on each GET of the login form.
|
||||||
_HIDDEN_RE = re.compile(
|
_HIDDEN_RE = re.compile(
|
||||||
r"name='(?P<name>LoginSecurity|SubmitToken)'[^>]*value='(?P<value>[^']*)'"
|
r"name='(?P<name>LoginSecurity|SubmitToken)'[^>]*value='(?P<value>[^']*)'"
|
||||||
@@ -315,11 +323,16 @@ class FreshHarvestClient:
|
|||||||
self._authenticated = False
|
self._authenticated = False
|
||||||
|
|
||||||
async def _get(self, path: str) -> str:
|
async def _get(self, path: str) -> str:
|
||||||
async with self._session.get(
|
try:
|
||||||
BASE.join(URL(path)), headers={"User-Agent": USER_AGENT}
|
async with self._session.get(
|
||||||
) as resp:
|
BASE.join(URL(path)),
|
||||||
resp.raise_for_status()
|
headers={"User-Agent": USER_AGENT},
|
||||||
return await resp.text()
|
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:
|
async def async_login(self) -> None:
|
||||||
"""Run the two-step handshake: fetch tokens, then post credentials.
|
"""Run the two-step handshake: fetch tokens, then post credentials.
|
||||||
@@ -347,10 +360,11 @@ class FreshHarvestClient:
|
|||||||
BASE.join(URL(LOGIN_SUBMIT)),
|
BASE.join(URL(LOGIN_SUBMIT)),
|
||||||
data=payload,
|
data=payload,
|
||||||
headers={"User-Agent": USER_AGENT},
|
headers={"User-Agent": USER_AGENT},
|
||||||
|
timeout=REQUEST_TIMEOUT,
|
||||||
) as resp:
|
) as resp:
|
||||||
resp.raise_for_status()
|
resp.raise_for_status()
|
||||||
body = await resp.text()
|
body = await resp.text()
|
||||||
except aiohttp.ClientError as err:
|
except (aiohttp.ClientError, TimeoutError) as err:
|
||||||
raise FreshHarvestError(f"login request failed: {err}") from err
|
raise FreshHarvestError(f"login request failed: {err}") from err
|
||||||
|
|
||||||
if not self._signed_in(body):
|
if not self._signed_in(body):
|
||||||
|
|||||||
@@ -12,5 +12,5 @@
|
|||||||
"requirements": [
|
"requirements": [
|
||||||
"beautifulsoup4>=4.12"
|
"beautifulsoup4>=4.12"
|
||||||
],
|
],
|
||||||
"version": "0.4.0"
|
"version": "0.4.1"
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
|
|||||||
|
|
||||||
from . import FreshHarvestConfigEntry
|
from . import FreshHarvestConfigEntry
|
||||||
from .api import FreshHarvestError
|
from .api import FreshHarvestError
|
||||||
|
from .const import DOMAIN
|
||||||
from .coordinator import FreshHarvestCoordinator
|
from .coordinator import FreshHarvestCoordinator
|
||||||
from .entity import FreshHarvestEntity
|
from .entity import FreshHarvestEntity
|
||||||
|
|
||||||
@@ -79,13 +80,26 @@ class FreshHarvestBoxSelect(FreshHarvestEntity, SelectEntity):
|
|||||||
|
|
||||||
async def async_added_to_hass(self) -> None:
|
async def async_added_to_hass(self) -> None:
|
||||||
await super().async_added_to_hass()
|
await super().async_added_to_hass()
|
||||||
# Listing costs a fetch per box popup, so it happens once on setup
|
# Listing the boxes on offer costs one fetch per box popup. Doing it
|
||||||
# rather than on every coordinator refresh.
|
# 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:
|
try:
|
||||||
baskets = await self.coordinator.actions.async_list_baskets()
|
baskets = await self.coordinator.actions.async_list_baskets()
|
||||||
self._options = [b.name for b in baskets]
|
|
||||||
except FreshHarvestError as err:
|
except FreshHarvestError as err:
|
||||||
_LOGGER.warning("could not list produce boxes: %s", 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:
|
async def async_select_option(self, option: str) -> None:
|
||||||
"""Switch the next delivery to this box."""
|
"""Switch the next delivery to this box."""
|
||||||
|
|||||||
@@ -39,7 +39,7 @@ from homeassistant.exceptions import HomeAssistantError
|
|||||||
from homeassistant.helpers.entity_platform import AddEntitiesCallback
|
from homeassistant.helpers.entity_platform import AddEntitiesCallback
|
||||||
|
|
||||||
from . import FreshHarvestConfigEntry
|
from . import FreshHarvestConfigEntry
|
||||||
from .api import FreshHarvestError
|
from .api import REQUEST_TIMEOUT, FreshHarvestError
|
||||||
from .coordinator import FreshHarvestCoordinator
|
from .coordinator import FreshHarvestCoordinator
|
||||||
from .entity import FreshHarvestEntity
|
from .entity import FreshHarvestEntity
|
||||||
|
|
||||||
@@ -129,6 +129,7 @@ class FreshHarvestBox(FreshHarvestEntity, TodoListEntity):
|
|||||||
"X-Algolia-Application-Id": app,
|
"X-Algolia-Application-Id": app,
|
||||||
"Content-Type": "application/json",
|
"Content-Type": "application/json",
|
||||||
},
|
},
|
||||||
|
timeout=REQUEST_TIMEOUT,
|
||||||
) as resp:
|
) as resp:
|
||||||
resp.raise_for_status()
|
resp.raise_for_status()
|
||||||
hits = (await resp.json()).get("hits") or []
|
hits = (await resp.json()).get("hits") or []
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ def _load():
|
|||||||
stub = types.ModuleType("aiohttp")
|
stub = types.ModuleType("aiohttp")
|
||||||
stub.ClientSession = object
|
stub.ClientSession = object
|
||||||
stub.ClientError = Exception
|
stub.ClientError = Exception
|
||||||
|
stub.ClientTimeout = lambda *args, **kwargs: None
|
||||||
sys.modules["aiohttp"] = stub
|
sys.modules["aiohttp"] = stub
|
||||||
sys.path.insert(0, str(COMPONENT))
|
sys.path.insert(0, str(COMPONENT))
|
||||||
pkg = types.ModuleType("fh_pkg")
|
pkg = types.ModuleType("fh_pkg")
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ def _load_api():
|
|||||||
stub = types.ModuleType("aiohttp")
|
stub = types.ModuleType("aiohttp")
|
||||||
stub.ClientSession = object
|
stub.ClientSession = object
|
||||||
stub.ClientError = Exception
|
stub.ClientError = Exception
|
||||||
|
stub.ClientTimeout = lambda *args, **kwargs: None
|
||||||
sys.modules["aiohttp"] = stub
|
sys.modules["aiohttp"] = stub
|
||||||
if "yarl" not in sys.modules:
|
if "yarl" not in sys.modules:
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in New Issue
Block a user