fix: Prevent addon probe from failing config entry
Extract WSLink add-on probe and treat expected network errors and NoURLAvailableError as "offline" instead of raising. Log unexpected errors and avoid raising ConfigEntryNotReady from diagnostics probes. Add tests for probe robustness, add additional sensitive purge keys, and fix config_flow host fallback handling.ecowitt_support
parent
793ea1991a
commit
a09e2aec9f
|
|
@ -264,8 +264,7 @@ class ConfigOptionsFlowHandler(OptionsFlow):
|
|||
if user_input is None:
|
||||
url: URL = URL(get_url(self.hass))
|
||||
|
||||
if not url.host:
|
||||
url.host = "UNKNOWN"
|
||||
host = url.host or "UNKNOWN"
|
||||
|
||||
ecowitt_schema = {
|
||||
vol.Required(
|
||||
|
|
@ -282,7 +281,7 @@ class ConfigOptionsFlowHandler(OptionsFlow):
|
|||
step_id="ecowitt",
|
||||
data_schema=vol.Schema(ecowitt_schema),
|
||||
description_placeholders={
|
||||
"url": url.host,
|
||||
"url": host,
|
||||
"port": str(url.port),
|
||||
"webhook_id": webhook,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -98,6 +98,10 @@ DEFAULT_URL = "/weatherstation/updateweatherstation.php"
|
|||
PURGE_DATA: Final = [
|
||||
"ID",
|
||||
"PASSWORD",
|
||||
"wsid",
|
||||
"wspw",
|
||||
"passkey",
|
||||
"PASSKEY",
|
||||
"action",
|
||||
"rtfreq",
|
||||
"realtime",
|
||||
|
|
|
|||
|
|
@ -18,10 +18,9 @@ from asyncio import timeout
|
|||
from copy import deepcopy
|
||||
from datetime import timedelta
|
||||
import logging
|
||||
from typing import Any
|
||||
from typing import Any, Final
|
||||
|
||||
import aiohttp
|
||||
from aiohttp import ClientConnectionError
|
||||
import aiohttp.web
|
||||
from aiohttp.web_exceptions import HTTPUnauthorized
|
||||
from py_typecheck import checked, checked_or
|
||||
|
|
@ -30,7 +29,7 @@ from homeassistant.components.http import KEY_AUTHENTICATED
|
|||
from homeassistant.components.network import async_get_source_ip
|
||||
from homeassistant.core import HomeAssistant
|
||||
from homeassistant.helpers.aiohttp_client import async_get_clientsession
|
||||
from homeassistant.helpers.network import get_url
|
||||
from homeassistant.helpers.network import NoURLAvailableError, get_url
|
||||
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
|
||||
from homeassistant.util import dt as dt_util
|
||||
|
||||
|
|
@ -59,6 +58,14 @@ _LOGGER = logging.getLogger(__name__)
|
|||
_REAL_PROTOCOLS: frozenset[str] = frozenset({"wu", "wslink", "ecowitt"})
|
||||
_LEGACY_PROTOCOLS: frozenset[str] = frozenset({"wu", "wslink"})
|
||||
|
||||
# Expected ways the optional add-on probe can fail. All of them simply mean
|
||||
# "add-on not reachable" and are logged at debug level (see `_async_update_data`).
|
||||
# - TimeoutError : `asyncio.timeout` expiry, e.g. a firewall dropping packets
|
||||
# - aiohttp.ClientError: connection refused, TLS failures, bad response, ...
|
||||
# - OSError : raw socket errors not wrapped by aiohttp
|
||||
# - NoURLAvailableError: HA cannot resolve its own URL
|
||||
_PROBE_ERRORS: Final = (TimeoutError, aiohttp.ClientError, OSError, NoURLAvailableError)
|
||||
|
||||
|
||||
def _configured_protocol(config: SWSConfigEntry) -> str:
|
||||
"""Return the primary configured protocol (wu / wslink / ecowitt).
|
||||
|
|
@ -248,55 +255,90 @@ class HealthCoordinator(DataUpdateCoordinator):
|
|||
last_protocol if accepted and last_protocol in _REAL_PROTOCOLS else configured_protocol
|
||||
)
|
||||
|
||||
async def _async_update_data(self) -> dict[str, Any]:
|
||||
"""Refresh add-on health metadata from the WSLink proxy.
|
||||
async def _probe_addon(self, addon: dict[str, Any]) -> None:
|
||||
"""Fill `addon` with live WSLink proxy metadata.
|
||||
|
||||
The proxy add-on can front any protocol (WU / WSLink / Ecowitt), so the probe
|
||||
is not gated on a specific protocol option - it always runs.
|
||||
May raise: turning a failed probe into `online: False` is the caller's job.
|
||||
Fields are written as they are resolved, so a failure part-way through still
|
||||
leaves the snapshot with everything learned up to that point.
|
||||
"""
|
||||
session = async_get_clientsession(self.hass, False)
|
||||
url = get_url(self.hass)
|
||||
ip = await async_get_source_ip(self.hass)
|
||||
|
||||
ip = await async_get_source_ip(self.hass)
|
||||
port = checked_or(self.config.options.get(WSLINK_ADDON_PORT), int, 443)
|
||||
|
||||
health_url = f"https://{ip}:{port}/healthz"
|
||||
info_url = f"https://{ip}:{port}/status/internal"
|
||||
|
||||
data = deepcopy(self.data)
|
||||
addon = data["addon"]
|
||||
addon["health_url"] = health_url
|
||||
addon["info_url"] = info_url
|
||||
addon["home_assistant_url"] = url
|
||||
addon["home_assistant_source_ip"] = str(ip)
|
||||
addon["online"] = False
|
||||
|
||||
# Informational only, and independently fallible (`NoURLAvailableError`),
|
||||
# so it must not prevent the reachability probe below from running.
|
||||
try:
|
||||
async with timeout(5), session.get(health_url) as response:
|
||||
addon["online"] = checked(response.status, int) == 200
|
||||
except ClientConnectionError:
|
||||
addon["online"] = False
|
||||
addon["home_assistant_url"] = get_url(self.hass)
|
||||
except NoURLAvailableError:
|
||||
_LOGGER.debug("No Home Assistant URL available for the health snapshot")
|
||||
|
||||
async with timeout(5), session.get(health_url) as response:
|
||||
addon["online"] = checked(response.status, int) == 200
|
||||
|
||||
if not addon["online"]:
|
||||
return
|
||||
|
||||
raw_status: dict[str, Any] | None = None
|
||||
if addon["online"]:
|
||||
try:
|
||||
async with timeout(5), session.get(info_url) as info_response:
|
||||
if checked(info_response.status, int) == 200:
|
||||
raw_status = await info_response.json(content_type=None)
|
||||
except (ClientConnectionError, aiohttp.ContentTypeError, ValueError):
|
||||
raw_status = None
|
||||
try:
|
||||
async with timeout(5), session.get(info_url) as info_response:
|
||||
if checked(info_response.status, int) == 200:
|
||||
# A non-dict body (or malformed JSON) is treated as "no status".
|
||||
raw_status = checked(await info_response.json(content_type=None), dict[str, Any])
|
||||
except (*_PROBE_ERRORS, ValueError):
|
||||
raw_status = None
|
||||
|
||||
addon["raw_status"] = raw_status
|
||||
if raw_status:
|
||||
addon["name"] = raw_status.get("addon")
|
||||
addon["version"] = raw_status.get("version")
|
||||
addon["listen_port"] = raw_status.get("listen", {}).get("port")
|
||||
addon["tls"] = raw_status.get("listen", {}).get("tls")
|
||||
addon["upstream_ha_port"] = raw_status.get("upstream", {}).get("ha_port")
|
||||
addon["paths"] = {
|
||||
"wslink": raw_status.get("paths", {}).get("wslink", WSLINK_URL),
|
||||
"wu": raw_status.get("paths", {}).get("wu", DEFAULT_URL),
|
||||
}
|
||||
if not raw_status:
|
||||
return
|
||||
|
||||
listen = checked_or(raw_status.get("listen"), dict[str, Any], {})
|
||||
upstream = checked_or(raw_status.get("upstream"), dict[str, Any], {})
|
||||
paths = checked_or(raw_status.get("paths"), dict[str, Any], {})
|
||||
|
||||
addon["name"] = raw_status.get("addon")
|
||||
addon["version"] = raw_status.get("version")
|
||||
addon["listen_port"] = listen.get("port")
|
||||
addon["tls"] = listen.get("tls")
|
||||
addon["upstream_ha_port"] = upstream.get("ha_port")
|
||||
addon["paths"] = {
|
||||
"wslink": paths.get("wslink", WSLINK_URL),
|
||||
"wu": paths.get("wu", DEFAULT_URL),
|
||||
}
|
||||
|
||||
async def _async_update_data(self) -> dict[str, Any]:
|
||||
"""Refresh add-on health metadata from the WSLink proxy.
|
||||
|
||||
The proxy add-on can front any protocol (WU / WSLink / Ecowitt), so the probe
|
||||
is not gated on a specific protocol option - it always runs.
|
||||
|
||||
The add-on is *optional* and this coordinator only produces diagnostics, so the
|
||||
probe must never fail the update. `async_config_entry_first_refresh` turns any
|
||||
exception raised here into `ConfigEntryNotReady`, which would take the station
|
||||
webhook - the actual job of this integration - down with it. An unreachable or
|
||||
misbehaving add-on is therefore recorded as `online: False`, never raised.
|
||||
"""
|
||||
data = deepcopy(self.data)
|
||||
addon = data["addon"]
|
||||
addon["online"] = False
|
||||
addon["raw_status"] = None
|
||||
|
||||
try:
|
||||
await self._probe_addon(addon)
|
||||
except _PROBE_ERRORS as err:
|
||||
_LOGGER.debug("WSLink add-on probe failed (%s): %s", type(err).__name__, err)
|
||||
addon["online"] = False
|
||||
except Exception: # noqa: BLE001 - diagnostics must never fail the config entry
|
||||
_LOGGER.exception("Unexpected error while probing the WSLink add-on")
|
||||
addon["online"] = False
|
||||
|
||||
self._refresh_summary(data)
|
||||
return self._commit(data)
|
||||
|
|
|
|||
|
|
@ -30,6 +30,8 @@ from pytest_homeassistant_custom_component.common import MockConfigEntry
|
|||
|
||||
from custom_components.sws12500 import health_coordinator as hc, health_sensor as hs
|
||||
from custom_components.sws12500.const import (
|
||||
API_ID,
|
||||
API_KEY,
|
||||
DEFAULT_URL,
|
||||
DOMAIN,
|
||||
ECOWITT_ENABLED,
|
||||
|
|
@ -46,12 +48,23 @@ from custom_components.sws12500.data import SWSRuntimeData
|
|||
from custom_components.sws12500.health_coordinator import HealthCoordinator
|
||||
from custom_components.sws12500.routes import Routes
|
||||
from homeassistant.components.http import KEY_AUTHENTICATED
|
||||
from homeassistant.config_entries import ConfigEntryState
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers / fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _ProbeRouterStub:
|
||||
"""Minimal aiohttp router stub for the end-to-end setup test."""
|
||||
|
||||
def add_get(self, path: str, handler: Any, **_kwargs: Any) -> Any:
|
||||
return SimpleNamespace(method="GET")
|
||||
|
||||
def add_post(self, path: str, handler: Any, **_kwargs: Any) -> Any:
|
||||
return SimpleNamespace(method="POST")
|
||||
|
||||
|
||||
def _make_entry(options: dict[str, Any] | None = None) -> MockConfigEntry:
|
||||
"""Create a config entry usable by the coordinator constructor."""
|
||||
return MockConfigEntry(domain=DOMAIN, data={}, options=options or {})
|
||||
|
|
@ -523,9 +536,7 @@ def test_update_ingress_result_explicit_reason(hass, entry) -> None:
|
|||
_attach_runtime_data(entry, coordinator)
|
||||
|
||||
request = SimpleNamespace(path=DEFAULT_URL, method="GET")
|
||||
coordinator.update_ingress_result(
|
||||
request, accepted=False, authorized=None, reason="unauthorized"
|
||||
)
|
||||
coordinator.update_ingress_result(request, accepted=False, authorized=None, reason="unauthorized")
|
||||
|
||||
assert coordinator.data["last_ingress"]["reason"] == "unauthorized"
|
||||
assert coordinator.data["integration_status"] == "degraded"
|
||||
|
|
@ -540,12 +551,8 @@ def test_update_forwarding(hass, entry) -> None:
|
|||
coordinator = HealthCoordinator(hass, entry)
|
||||
_attach_runtime_data(entry, coordinator)
|
||||
|
||||
windy = SimpleNamespace(
|
||||
enabled=True, last_status="ok", last_error=None, last_attempt_at="2026-06-20T10:00:00"
|
||||
)
|
||||
pocasi = SimpleNamespace(
|
||||
enabled=False, last_status="disabled", last_error="oops", last_attempt_at=None
|
||||
)
|
||||
windy = SimpleNamespace(enabled=True, last_status="ok", last_error=None, last_attempt_at="2026-06-20T10:00:00")
|
||||
pocasi = SimpleNamespace(enabled=False, last_status="disabled", last_error="oops", last_attempt_at=None)
|
||||
|
||||
coordinator.update_forwarding(windy, pocasi)
|
||||
|
||||
|
|
@ -756,3 +763,135 @@ def test_sensor_unique_id_and_category() -> None:
|
|||
sensor = hs.HealthDiagnosticSensor(coordinator, _description("active_protocol"))
|
||||
assert sensor.unique_id == "active_protocol_health"
|
||||
assert sensor.entity_category == hs.EntityCategory.DIAGNOSTIC
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Regression: the optional add-on probe must never fail the config entry
|
||||
#
|
||||
# `async_config_entry_first_refresh` converts *any* exception escaping
|
||||
# `_async_update_data` into `ConfigEntryNotReady`. Since the WSLink proxy add-on is
|
||||
# optional, an unreachable/misbehaving add-on used to take the whole integration -
|
||||
# including the station webhook - down with it.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"failure",
|
||||
[
|
||||
pytest.param(TimeoutError(), id="asyncio-timeout"),
|
||||
pytest.param(ClientConnectionError("refused"), id="connection-refused"),
|
||||
pytest.param(aiohttp.ClientResponseError(None, (), status=500), id="client-response-error"),
|
||||
pytest.param(aiohttp.ClientError("generic"), id="generic-client-error"),
|
||||
pytest.param(OSError("raw socket"), id="raw-oserror"),
|
||||
],
|
||||
)
|
||||
async def test_probe_failure_never_raises(hass, monkeypatch, failure) -> None:
|
||||
"""Any expected probe failure is recorded as offline, not raised."""
|
||||
entry = _make_entry({WSLINK_ADDON_PORT: 8443})
|
||||
coordinator = HealthCoordinator(hass, entry)
|
||||
_attach_runtime_data(entry, coordinator)
|
||||
|
||||
_patch_network(monkeypatch, _FakeSession({"/healthz": failure}))
|
||||
|
||||
data = await coordinator._async_update_data()
|
||||
|
||||
assert data["addon"]["online"] is False
|
||||
assert data["addon"]["raw_status"] is None
|
||||
# Metadata resolved before the failing request is still reported.
|
||||
assert data["addon"]["health_url"] == "https://1.2.3.4:8443/healthz"
|
||||
assert data["addon"]["home_assistant_source_ip"] == "1.2.3.4"
|
||||
|
||||
|
||||
async def test_probe_unexpected_error_never_raises(hass, monkeypatch, caplog) -> None:
|
||||
"""Even an unforeseen error is contained (and logged) rather than propagated."""
|
||||
entry = _make_entry()
|
||||
coordinator = HealthCoordinator(hass, entry)
|
||||
_attach_runtime_data(entry, coordinator)
|
||||
|
||||
_patch_network(monkeypatch, _FakeSession({"/healthz": RuntimeError("boom")}))
|
||||
|
||||
data = await coordinator._async_update_data()
|
||||
|
||||
assert data["addon"]["online"] is False
|
||||
assert "Unexpected error while probing" in caplog.text
|
||||
|
||||
|
||||
async def test_probe_survives_missing_ha_url(hass, monkeypatch) -> None:
|
||||
"""`get_url` raising NoURLAvailableError must not skip the reachability probe."""
|
||||
entry = _make_entry()
|
||||
coordinator = HealthCoordinator(hass, entry)
|
||||
_attach_runtime_data(entry, coordinator)
|
||||
|
||||
session = _FakeSession({"/healthz": _FakeResponse(200), "/status/internal": _FakeResponse(404)})
|
||||
_patch_network(monkeypatch, session)
|
||||
|
||||
def _no_url(_hass):
|
||||
raise hc.NoURLAvailableError
|
||||
|
||||
monkeypatch.setattr(hc, "get_url", _no_url)
|
||||
|
||||
data = await coordinator._async_update_data()
|
||||
|
||||
# The add-on was still probed successfully despite HA not knowing its own URL.
|
||||
assert data["addon"]["online"] is True
|
||||
|
||||
|
||||
async def test_probe_non_dict_status_body(hass, monkeypatch) -> None:
|
||||
"""A non-dict /status/internal body must not blow up metadata parsing."""
|
||||
entry = _make_entry()
|
||||
coordinator = HealthCoordinator(hass, entry)
|
||||
_attach_runtime_data(entry, coordinator)
|
||||
|
||||
session = _FakeSession(
|
||||
{
|
||||
"/healthz": _FakeResponse(200),
|
||||
"/status/internal": _FakeResponse(200, json_data=["not", "a", "dict"]),
|
||||
}
|
||||
)
|
||||
_patch_network(monkeypatch, session)
|
||||
|
||||
data = await coordinator._async_update_data()
|
||||
|
||||
assert data["addon"]["online"] is True
|
||||
assert data["addon"]["raw_status"] is None
|
||||
assert data["addon"]["name"] is None
|
||||
|
||||
|
||||
async def test_setup_succeeds_when_addon_probe_fails(hass, enable_custom_integrations, monkeypatch) -> None:
|
||||
"""End to end: a dead add-on must not put the config entry into SETUP_RETRY."""
|
||||
hass.http = SimpleNamespace(app=SimpleNamespace(router=_ProbeRouterStub()))
|
||||
|
||||
entry = MockConfigEntry(domain=DOMAIN, data={}, options={API_ID: "id", API_KEY: "key"})
|
||||
entry.add_to_hass(hass)
|
||||
|
||||
async def _timeout(_self, _addon):
|
||||
raise TimeoutError
|
||||
|
||||
monkeypatch.setattr(HealthCoordinator, "_probe_addon", _timeout)
|
||||
|
||||
assert await hass.config_entries.async_setup(entry.entry_id)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
assert entry.state is ConfigEntryState.LOADED
|
||||
assert entry.runtime_data.health_coordinator.data["addon"]["online"] is False
|
||||
|
||||
|
||||
async def test_probe_healthz_non_200_skips_info_call(hass, monkeypatch) -> None:
|
||||
"""Add-on reachable but unhealthy: report offline and skip the info endpoint."""
|
||||
entry = _make_entry()
|
||||
coordinator = HealthCoordinator(hass, entry)
|
||||
_attach_runtime_data(entry, coordinator)
|
||||
|
||||
session = _FakeSession(
|
||||
{
|
||||
"/healthz": _FakeResponse(503),
|
||||
"/status/internal": _FakeResponse(200, json_data={"addon": "should-not-be-read"}),
|
||||
}
|
||||
)
|
||||
_patch_network(monkeypatch, session)
|
||||
|
||||
data = await coordinator._async_update_data()
|
||||
|
||||
assert data["addon"]["online"] is False
|
||||
assert data["addon"]["raw_status"] is None
|
||||
assert data["addon"]["name"] is None
|
||||
|
|
|
|||
Loading…
Reference in New Issue