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.ecowitt_support
parent
a09e2aec9f
commit
a9098555ba
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Reference in New Issue