From a9098555bac567cb4ce6a51930d6f3adb2dfcf22 Mon Sep 17 00:00:00 2001 From: SchiZzA Date: Sat, 25 Jul 2026 19:18:07 +0200 Subject: [PATCH] fix(Pocasi): Add retry limit and response handling Add POCASI_CZ_MAX_RETRIES and map HTTP status codes to explicit send outcomes. Use the status (not the empty body) to classify sends as "ok", "auth_error", or "unexpected_response". Introduce _disable_pocasi to persistently turn off resending and record errors. Count unexpected responses and disable resends after the retry limit; adjust client-error handling to reuse the same disable logic. Update tests and config flow expectations to match the new behavior. --- custom_components/sws12500/const.py | 1 + custom_components/sws12500/pocasti_cz.py | 97 +++++++++++--------- tests/test_config_flow.py | 36 ++++---- tests/test_pocasi_push.py | 109 +++++++++++++++++++---- 4 files changed, 164 insertions(+), 79 deletions(-) diff --git a/custom_components/sws12500/const.py b/custom_components/sws12500/const.py index a274675..548f873 100644 --- a/custom_components/sws12500/const.py +++ b/custom_components/sws12500/const.py @@ -177,6 +177,7 @@ WINDY_URL = "https://stations.windy.com/api/v2/observation/update" POCASI_CZ_URL: Final = "http://ms.pocasimeteo.cz" POCASI_CZ_SEND_MINIMUM: Final = 12 # minimal time to resend data +POCASI_CZ_MAX_RETRIES: Final = 3 # failed sends in a row before resending is disabled WSLINK: Final = "wslink" diff --git a/custom_components/sws12500/pocasti_cz.py b/custom_components/sws12500/pocasti_cz.py index 872146c..90a8c32 100644 --- a/custom_components/sws12500/pocasti_cz.py +++ b/custom_components/sws12500/pocasti_cz.py @@ -20,6 +20,7 @@ from .const import ( POCASI_CZ_API_KEY, POCASI_CZ_ENABLED, POCASI_CZ_LOGGER_ENABLED, + POCASI_CZ_MAX_RETRIES, POCASI_CZ_SEND_INTERVAL, POCASI_CZ_SUCCESS, POCASI_CZ_UNEXPECTED, @@ -31,21 +32,12 @@ from .utils import anonymize, update_options _LOGGER = logging.getLogger(__name__) - -class PocasiNotInserted(Exception): - """NotInserted state.""" - - -class PocasiSuccess(Exception): - """WindySucces state.""" - - -class PocasiApiKeyError(Exception): - """Windy API Key error.""" +# Outcome of a single send, derived from the HTTP status (see `verify_response`). +type PocasiResult = Literal["ok", "auth_error", "unexpected_response"] class PocasiPush: - """Push data to Windy.""" + """Forward station payloads to the Pocasi Meteo server.""" def __init__(self, hass: HomeAssistant, config: ConfigEntry) -> None: """Init.""" @@ -63,19 +55,32 @@ class PocasiPush: self.log = self.config.options.get(POCASI_CZ_LOGGER_ENABLED) self.invalid_response_count = 0 - def verify_response( - self, - response: str, - ) -> PocasiNotInserted | PocasiSuccess | PocasiApiKeyError | None: - """Verify answer form server.""" + def verify_response(self, status: int, body: str) -> PocasiResult: + """Classify a send by its HTTP status. + + The server returns no meaningful body, so the status code is all we have: + - 2xx -> "ok" + - 401 / 403 -> "auth_error" (bad ID/key combination) + - anything else -> "unexpected_response", counted toward the retry limit + """ if self.log: - _LOGGER.debug("Pocasi CZ responded: %s", response) + _LOGGER.debug("Pocasi CZ responded: [%s] %s", status, body) - # Server does not provide any responses. - # This is placeholder if future state is changed + if 200 <= status < 300: + return "ok" + if status in (401, 403): + return "auth_error" + return "unexpected_response" - return None + async def _disable_pocasi(self, reason: str) -> None: + """Turn resending off and persist it, so it survives a restart.""" + + self.enabled = False + self.last_error = reason + + if not await update_options(self.hass, self.config, POCASI_CZ_ENABLED, False): + _LOGGER.debug("Failed to set Pocasi Meteo options to false.") async def push_data_to_server(self, data: dict[str, Any], mode: Literal["WU", "WSLINK"]): """Pushes weather data to server.""" @@ -136,24 +141,35 @@ class PocasiPush: ) try: async with session.get(request_url, params=_data) as resp: - status = await resp.text() - try: - self.verify_response(status) + result = self.verify_response(resp.status, await resp.text()) + http_status = resp.status - except PocasiApiKeyError: - # log despite of settings - self.last_status = "auth_error" - self.last_error = POCASI_INVALID_KEY - self.enabled = False - _LOGGER.critical(POCASI_INVALID_KEY) - await update_options(self.hass, self.config, POCASI_CZ_ENABLED, False) - except PocasiSuccess: - self.last_status = "ok" - self.last_error = None - if self.log: - _LOGGER.info(POCASI_CZ_SUCCESS) - else: - self.last_status = "ok" + if result == "ok": + self.invalid_response_count = 0 + self.last_status = "ok" + self.last_error = None + if self.log: + _LOGGER.info(POCASI_CZ_SUCCESS) + + elif result == "auth_error": + # Credentials will not fix themselves - disable immediately instead of + # burning through the retry budget. Logged regardless of the log setting. + self.last_status = "auth_error" + _LOGGER.critical(POCASI_INVALID_KEY) + await self._disable_pocasi(POCASI_INVALID_KEY) + + else: + self.last_status = "unexpected_response" + self.last_error = f"Unexpected HTTP status {http_status} from Pocasi Meteo." + self.invalid_response_count += 1 + _LOGGER.warning( + "Unexpected HTTP status %s from Pocasi Meteo. Retries before disabling resend: %s", + http_status, + POCASI_CZ_MAX_RETRIES - self.invalid_response_count, + ) + if self.invalid_response_count >= POCASI_CZ_MAX_RETRIES: + _LOGGER.critical(POCASI_CZ_UNEXPECTED) + await self._disable_pocasi(POCASI_CZ_UNEXPECTED) except ClientError as ex: self.last_status = "client_error" @@ -162,10 +178,9 @@ class PocasiPush: self.last_error = type(ex).__name__ _LOGGER.critical("Invalid response from Pocasi Meteo: %s", str(ex)) self.invalid_response_count += 1 - if self.invalid_response_count >= 3: + if self.invalid_response_count >= POCASI_CZ_MAX_RETRIES: _LOGGER.critical(POCASI_CZ_UNEXPECTED) - self.enabled = False - await update_options(self.hass, self.config, POCASI_CZ_ENABLED, False) + await self._disable_pocasi(POCASI_CZ_UNEXPECTED) self.last_update = dt_util.utcnow() self.next_update = dt_util.utcnow() + timedelta(seconds=self._interval) diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 0fc71e1..d68fd79 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -361,33 +361,27 @@ async def test_options_flow_ecowitt_uses_get_url_placeholders_and_webhook_defaul ) entry.add_to_hass(hass) - init = await hass.config_entries.options.async_init(entry.entry_id) - assert init["type"] == "menu" + # A URL without a host degrades to the "UNKNOWN" placeholder rather than raising. + # (`yarl.URL.host` is a read-only cached property, so the fallback has to build a + # new value instead of assigning to it.) This runs in its own flow because showing + # the form advances past the menu. + hostless_init = await hass.config_entries.options.async_init(entry.entry_id) + assert hostless_init["type"] == "menu" - # NOTE: - # The integration currently attempts to mutate `yarl.URL.host` when it is missing: - # - # url: URL = URL(get_url(self.hass)) - # if not url.host: - # url.host = "UNKNOWN" - # - # With current yarl versions, `URL.host` is a cached, read-only property, so this - # raises `AttributeError: cached property is read-only`. - # - # We assert that behavior explicitly to keep coverage deterministic and document the - # runtime incompatibility. If the integration code is updated to handle missing hosts - # without mutation (e.g. using `url.raw_host` or building placeholders without setting - # attributes), this assertion should be updated accordingly. with patch( "custom_components.sws12500.config_flow.get_url", return_value="http://", ): - with pytest.raises(AttributeError): - await hass.config_entries.options.async_configure( - init["flow_id"], user_input={"next_step_id": "ecowitt"} - ) + hostless = await hass.config_entries.options.async_configure( + hostless_init["flow_id"], user_input={"next_step_id": "ecowitt"} + ) + assert hostless["type"] == "form" + assert (hostless.get("description_placeholders") or {})["url"] == "UNKNOWN" + + # A normal URL fills real placeholders and completes the flow. + init = await hass.config_entries.options.async_init(entry.entry_id) + assert init["type"] == "menu" - # Second call uses a normal URL and completes the flow. with patch( "custom_components.sws12500.config_flow.get_url", return_value="http://example.local:8123", diff --git a/tests/test_pocasi_push.py b/tests/test_pocasi_push.py index 58df5f9..8eb67b4 100644 --- a/tests/test_pocasi_push.py +++ b/tests/test_pocasi_push.py @@ -15,19 +15,21 @@ from custom_components.sws12500.const import ( POCASI_CZ_API_KEY, POCASI_CZ_ENABLED, POCASI_CZ_LOGGER_ENABLED, + POCASI_CZ_MAX_RETRIES, POCASI_CZ_SEND_INTERVAL, POCASI_CZ_UNEXPECTED, POCASI_CZ_URL, POCASI_INVALID_KEY, WSLINK_URL, ) -from custom_components.sws12500.pocasti_cz import PocasiApiKeyError, PocasiPush, PocasiSuccess +from custom_components.sws12500.pocasti_cz import PocasiPush from homeassistant.util import dt as dt_util @dataclass(slots=True) class _FakeResponse: text_value: str + status: int = 200 async def text(self) -> str: return self.text_value @@ -182,31 +184,28 @@ async def test_push_data_to_server_calls_verify_response(monkeypatch, hass): ) monkeypatch.setattr("custom_components.sws12500.pocasti_cz.anonymize", lambda d: d) - verify = MagicMock(return_value=None) + verify = MagicMock(return_value="ok") monkeypatch.setattr(pp, "verify_response", verify) await pp.push_data_to_server({"x": 1}, "WU") - verify.assert_called_once_with("OK") + verify.assert_called_once_with(200, "OK") @pytest.mark.asyncio -async def test_push_data_to_server_api_key_error_disables_feature(monkeypatch, hass): +@pytest.mark.parametrize("status", [401, 403]) +async def test_push_data_to_server_auth_error_disables_feature(monkeypatch, hass, status): + """A 401/403 disables resending immediately - credentials will not self-heal.""" entry = _make_entry() pp = PocasiPush(hass, entry) pp.next_update = dt_util.utcnow() - timedelta(seconds=1) - session = _FakeSession(response=_FakeResponse("OK")) + session = _FakeSession(response=_FakeResponse("", status=status)) monkeypatch.setattr( "custom_components.sws12500.pocasti_cz.async_get_clientsession", lambda _h: session, ) monkeypatch.setattr("custom_components.sws12500.pocasti_cz.anonymize", lambda d: d) - def _raise(_status: str): - raise PocasiApiKeyError - - monkeypatch.setattr(pp, "verify_response", _raise) - update_options = AsyncMock(return_value=True) monkeypatch.setattr( "custom_components.sws12500.pocasti_cz.update_options", update_options @@ -223,6 +222,8 @@ async def test_push_data_to_server_api_key_error_disables_feature(monkeypatch, h POCASI_INVALID_KEY in str(c.args[0]) for c in crit.call_args_list if c.args ) update_options.assert_awaited_once_with(hass, entry, POCASI_CZ_ENABLED, False) + assert pp.enabled is False + assert pp.last_status == "auth_error" @pytest.mark.asyncio @@ -230,24 +231,58 @@ async def test_push_data_to_server_success_logs_when_logger_enabled(monkeypatch, entry = _make_entry(logger=True) pp = PocasiPush(hass, entry) pp.next_update = dt_util.utcnow() - timedelta(seconds=1) + # A previous failure must be cleared by a successful send. + pp.invalid_response_count = 2 - session = _FakeSession(response=_FakeResponse("OK")) + session = _FakeSession(response=_FakeResponse("OK", status=200)) monkeypatch.setattr( "custom_components.sws12500.pocasti_cz.async_get_clientsession", lambda _h: session, ) monkeypatch.setattr("custom_components.sws12500.pocasti_cz.anonymize", lambda d: d) - def _raise_success(_status: str): - raise PocasiSuccess - - monkeypatch.setattr(pp, "verify_response", _raise_success) - info = MagicMock() monkeypatch.setattr("custom_components.sws12500.pocasti_cz._LOGGER.info", info) await pp.push_data_to_server({"x": 1}, "WU") + info.assert_called() + assert pp.last_status == "ok" + assert pp.last_error is None + assert pp.invalid_response_count == 0 + + +@pytest.mark.asyncio +async def test_push_data_to_server_server_error_disables_after_max_retries(monkeypatch, hass): + """HTTP 500 is no longer silently reported as success; it counts toward the limit.""" + entry = _make_entry() + pp = PocasiPush(hass, entry) + + session = _FakeSession(response=_FakeResponse("", status=500)) + monkeypatch.setattr( + "custom_components.sws12500.pocasti_cz.async_get_clientsession", + lambda _h: session, + ) + monkeypatch.setattr("custom_components.sws12500.pocasti_cz.anonymize", lambda d: d) + + update_options = AsyncMock(return_value=True) + monkeypatch.setattr( + "custom_components.sws12500.pocasti_cz.update_options", update_options + ) + + for _ in range(POCASI_CZ_MAX_RETRIES - 1): + pp.next_update = dt_util.utcnow() - timedelta(seconds=1) + await pp.push_data_to_server({"x": 1}, "WU") + + assert pp.last_status == "unexpected_response" + assert pp.enabled is True + update_options.assert_not_awaited() + + pp.next_update = dt_util.utcnow() - timedelta(seconds=1) + await pp.push_data_to_server({"x": 1}, "WU") + + assert pp.enabled is False + update_options.assert_awaited_once_with(hass, entry, POCASI_CZ_ENABLED, False) @pytest.mark.asyncio @@ -295,5 +330,45 @@ def test_verify_response_logs_debug_when_logger_enabled(monkeypatch, hass): dbg = MagicMock() monkeypatch.setattr("custom_components.sws12500.pocasti_cz._LOGGER.debug", dbg) - assert pp.verify_response("anything") is None + assert pp.verify_response(200, "anything") == "ok" + dbg.assert_called() + + +@pytest.mark.parametrize( + ("status", "expected"), + [ + (200, "ok"), + (204, "ok"), + (299, "ok"), + (401, "auth_error"), + (403, "auth_error"), + (400, "unexpected_response"), + (404, "unexpected_response"), + (500, "unexpected_response"), + (503, "unexpected_response"), + ], +) +def test_verify_response_status_mapping(hass, status, expected): + """Every send outcome is derived from the HTTP status, not from the (empty) body.""" + pp = PocasiPush(hass, _make_entry()) + assert pp.verify_response(status, "") == expected + + +@pytest.mark.asyncio +async def test_disable_pocasi_logs_when_option_write_fails(monkeypatch, hass): + """A failed option write is logged but still leaves resending off in memory.""" + entry = _make_entry() + pp = PocasiPush(hass, entry) + + monkeypatch.setattr( + "custom_components.sws12500.pocasti_cz.update_options", + AsyncMock(return_value=False), + ) + dbg = MagicMock() + monkeypatch.setattr("custom_components.sws12500.pocasti_cz._LOGGER.debug", dbg) + + await pp._disable_pocasi("because") + + assert pp.enabled is False + assert pp.last_error == "because" dbg.assert_called()