diff --git a/api_accounts.py b/api_accounts.py index 09664f2..07a6cab 100644 --- a/api_accounts.py +++ b/api_accounts.py @@ -292,7 +292,7 @@ async def push_test( "url": f"{BASE}/alerts", "tag": "thermograph-test", } - sent, pruned = 0, 0 + sent, pruned, failed = 0, 0, 0 for row in rows: info = {"endpoint": row.endpoint, "keys": {"p256dh": row.p256dh, "auth": row.auth}} # pywebpush blocks on network I/O — keep it off the event loop. @@ -302,6 +302,10 @@ async def push_test( elif result == "gone": await session.delete(row) pruned += 1 + else: + failed += 1 if pruned: await session.commit() - return {"devices": len(rows), "sent": sent, "pruned": pruned} + # `failed` (delivery rejected — e.g. VAPID key mismatch) is surfaced so the client + # can tell "request accepted" apart from "actually delivered". + return {"devices": len(rows), "sent": sent, "pruned": pruned, "failed": failed} diff --git a/push.py b/push.py index 64ee963..7b6c58a 100644 --- a/push.py +++ b/push.py @@ -25,6 +25,8 @@ import logging import os import threading +import audit + from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import ec from pywebpush import WebPushException, webpush @@ -114,8 +116,14 @@ def send(subscription_info: dict, payload: dict) -> str: status = getattr(getattr(e, "response", None), "status_code", None) if status in (404, 410): return "gone" # endpoint retired — caller should delete it + # 401/403 = VAPID key mismatch, etc. Record it where it's visible (the errors + # JSONL / dashboard), not just the system journal. log.warning("web push failed (status=%s): %s", status, e) + audit.log_event("error", {"phase": "push", "status": status, + "endpoint": (subscription_info.get("endpoint") or "")[:120]}) return "error" - except Exception: # noqa: BLE001 - network/encoding errors must not propagate + except Exception as e: # noqa: BLE001 - network/encoding errors must not propagate log.warning("web push send raised", exc_info=True) + audit.log_event("error", {"phase": "push", "error": repr(e), + "endpoint": (subscription_info.get("endpoint") or "")[:120]}) return "error" diff --git a/tests/test_api_accounts.py b/tests/test_api_accounts.py index 9209dfb..efbc916 100644 --- a/tests/test_api_accounts.py +++ b/tests/test_api_accounts.py @@ -133,7 +133,7 @@ def test_push_subscribe_upsert_test_unsubscribe(monkeypatch): r = c.post(f"{V2}/push/test") assert r.status_code == 202 - assert r.json() == {"devices": 1, "sent": 1, "pruned": 0} + assert r.json() == {"devices": 1, "sent": 1, "pruned": 0, "failed": 0} assert sent == [_SUB["endpoint"]] assert c.request("DELETE", f"{V2}/push/subscribe", @@ -148,6 +148,19 @@ def test_push_test_prunes_gone_endpoint(monkeypatch): _login(c, "push-gone@example.com") assert c.post(f"{V2}/push/subscribe", json=_SUB).status_code == 201 r = c.post(f"{V2}/push/test") - assert r.json() == {"devices": 1, "sent": 0, "pruned": 1} + assert r.json() == {"devices": 1, "sent": 0, "pruned": 1, "failed": 0} # the gone endpoint was removed, so a second pass sees no devices assert c.post(f"{V2}/push/test").json()["devices"] == 0 + + +def test_push_test_reports_failed_delivery(monkeypatch): + # A rejected delivery (e.g. VAPID mismatch) must be reported, not silently dropped: + # the endpoint keeps returning 202 but with sent:0 / failed:N so the client can tell. + import push + monkeypatch.setattr(push, "send", lambda info, payload: "error") + c = _client() + _login(c, "push-fail@example.com") + assert c.post(f"{V2}/push/subscribe", json=_SUB).status_code == 201 + r = c.post(f"{V2}/push/test") + assert r.status_code == 202 + assert r.json() == {"devices": 1, "sent": 0, "pruned": 0, "failed": 1}