diff --git a/app/main.py b/app/main.py index 73eade58..3c693e28 100644 --- a/app/main.py +++ b/app/main.py @@ -1399,9 +1399,34 @@ async def _retry_device_links() -> None: ) -def _unlinked_devices_warning(result: Any) -> str: - """The notice for deployed devices the provider says are not on the account.""" - ids = sorted(getattr(result, "unlinked_devices", None) or []) +async def _retrying_device_ids() -> dict[str, set[str]]: + """Per slug, the device ids _retry_device_links is still adding to the account. + + Only inside the retry window: a row the retry job never got to mark as given + up (the job failing every time) must not hide the notice for ever. A failed + read answers {}: the plain notice then shows, as it did before. + """ + try: + rows = await database.list_pending_device_links() + except Exception as exc: + logger.warning("Could not read the device registrations being retried: %s", exc) + return {} + window_start = _db_time(datetime.now(UTC) - DEVICE_LINK_GIVE_UP) + retrying: dict[str, set[str]] = {} + for row in rows: + if not row["gave_up"] and str(row.get("first_at") or "") > window_start: + retrying.setdefault(row["slug"], set()).add(row["device_id"]) + return retrying + + +def _unlinked_devices_warning(result: Any, retrying: set[str] | frozenset[str] = frozenset()) -> str: + """The notice for deployed devices the provider says are not on the account. + + A device in ``retrying`` is left out: the app is still adding it, so asking + the user to add it by hand is wrong, and the notice would take the day's + push from the give-up message that follows if the retries fail. + """ + ids = sorted(set(getattr(result, "unlinked_devices", None) or []) - set(retrying)) if not ids: return "" svc = catalog.get_service(result.platform) or {} @@ -1459,6 +1484,7 @@ async def _run_collection() -> None: if c.device_ids: device_checked.add(slug) results = await asyncio.gather(*(_collect_bounded(c) for c in collectors), return_exceptions=True) + retrying = await _retrying_device_ids() alerts: list[dict[str, str]] = [] # Platforms that were already failing before this run: used to detect a # recovery, so a service that breaks again later notifies again rather @@ -1531,7 +1557,7 @@ async def _run_collection() -> None: # A caveat on a SUCCESSFUL reading. It is shown, because the # figure is partial and the user should know, but it is not # a failure: the balance below is stored exactly as any other. - if unlinked := _unlinked_devices_warning(result): + if unlinked := _unlinked_devices_warning(result, retrying.get(result.platform, set())): result.warning = f"{result.warning} {unlinked}" if result.warning else unlinked if getattr(result, "warning", None): safe_warning = notify.redact(result.warning) diff --git a/tests/test_retrying_device_notice.py b/tests/test_retrying_device_notice.py new file mode 100644 index 00000000..02192a63 --- /dev/null +++ b/tests/test_retrying_device_notice.py @@ -0,0 +1,63 @@ +"""A device the app is still adding to the account gets no "add it by hand" notice. + +A new ProxyRack UUID is unknown for about five minutes, so the hourly device +check finds it "invalid" while _retry_device_links is still adding it. The +notice then told the user to add it by hand, and it took the day's notice push +for that service, so the give-up message a day later was never pushed. While a +registration is being retried the device is left out of the notice; once the +retries give up it is reported as before. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from unittest.mock import AsyncMock, patch + +from app.collectors.base import EarningsResult +from tests.test_unlinked_devices import UNLINKED, _run_loop + + +def _ago(hours: float) -> str: + return (datetime.now(UTC) - timedelta(hours=hours)).strftime("%Y-%m-%d %H:%M:%S") + + +NOW = _ago(0.1) + + +def _loop_with_pending(rows): + with patch("app.main.database.list_pending_device_links", new_callable=AsyncMock, **rows): + alerts, _, _ = _run_loop(EarningsResult(platform="earnapp", balance=1.0, unlinked_devices=[UNLINKED])) + return [a for a in alerts if a.get("kind") == "notice" and a["platform"] == "earnapp"] + + +def test_a_device_being_retried_gets_no_notice(): + pending = [{"slug": "earnapp", "device_id": UNLINKED, "gave_up": 0, "first_at": NOW}] + assert _loop_with_pending({"return_value": pending}) == [] + + +def test_a_device_whose_retries_gave_up_is_reported_again(): + pending = [{"slug": "earnapp", "device_id": UNLINKED, "gave_up": 1, "first_at": _ago(25)}] + (notice,) = _loop_with_pending({"return_value": pending}) + assert UNLINKED in notice["error"] + + +def test_a_retry_of_another_service_or_device_changes_nothing(): + pending = [ + {"slug": "proxyrack", "device_id": UNLINKED, "gave_up": 0, "first_at": NOW}, + {"slug": "earnapp", "device_id": "sdk-node-" + "c" * 32, "gave_up": 0, "first_at": NOW}, + ] + (notice,) = _loop_with_pending({"return_value": pending}) + assert UNLINKED in notice["error"] + + +def test_an_unreadable_retry_table_keeps_the_plain_notice(): + (notice,) = _loop_with_pending({"side_effect": RuntimeError("db locked")}) + assert UNLINKED in notice["error"] + + +def test_a_retry_left_past_its_window_no_longer_hides_the_notice(): + # The retry job never marked it given up (it failed every time); after the + # day the retries were allowed, the device is reported again. + pending = [{"slug": "earnapp", "device_id": UNLINKED, "gave_up": 0, "first_at": _ago(25)}] + (notice,) = _loop_with_pending({"return_value": pending}) + assert UNLINKED in notice["error"]