diff --git a/custom_components/sws12500/config_flow.py b/custom_components/sws12500/config_flow.py index 3543447..bb3a3d0 100644 --- a/custom_components/sws12500/config_flow.py +++ b/custom_components/sws12500/config_flow.py @@ -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, }, diff --git a/custom_components/sws12500/const.py b/custom_components/sws12500/const.py index 37a484d..a274675 100644 --- a/custom_components/sws12500/const.py +++ b/custom_components/sws12500/const.py @@ -98,6 +98,10 @@ DEFAULT_URL = "/weatherstation/updateweatherstation.php" PURGE_DATA: Final = [ "ID", "PASSWORD", + "wsid", + "wspw", + "passkey", + "PASSKEY", "action", "rtfreq", "realtime", diff --git a/custom_components/sws12500/health_coordinator.py b/custom_components/sws12500/health_coordinator.py index 44b22c0..a41a051 100644 --- a/custom_components/sws12500/health_coordinator.py +++ b/custom_components/sws12500/health_coordinator.py @@ -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) diff --git a/tests/test_health.py b/tests/test_health.py index 38736ea..d6e594c 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -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