rbrooks/WeatherBot

Watch

Fork 0

Notification delivery is at-most-once — failed dispatches are never retried (design + outbox) #50

New issue

Closed

opened 2026-07-18 05:31:16 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-07-18 05:31:16 +00:00

Contributor

All three dispatch paths commit the dedup SentAlert before attempting
delivery (app/services/spc_processor.py:481-547 and :568-609;
app/services/alert_processor.py:656-659 fire-and-forget). A transient channel
failure is logged/recorded but the committed dedup row suppresses any retry —
the notification is permanently dropped. Decide the delivery semantics, then
implement:

Option A (recommended): delivery-outbox table keyed (alert record, channel)
with status/attempts/last_error; scheduler job retries pending/failed with
backoff until a max age; dispatch tasks tracked (fixes the GC-able
fire-and-forget tasks too). Option B (minimal): on failure, mark the
per-channel dedup row retryable with an attempts cap.

Rationale for scheduling with v1.2.0: the formatter migration (#14) already
touches every notifier; doing the dispatch-semantics change in the same wave
avoids rewriting the notifier boundary twice. Pull earlier if the silent-drop
rate observed in diagnostics warrants it.

Acceptance criteria:

A transient per-channel failure results in a later successful delivery
Retries are bounded (attempts + max age) and observable per channel
No duplicate delivery for the same (alert, channel) on retry
Warning-class NWS alerts covered first

Filed from the 2026-07-17 codebase audit (docs/.internal/report-2026-07-17.md), finding F-11.

All three dispatch paths commit the dedup `SentAlert` before attempting delivery (app/services/spc_processor.py:481-547 and :568-609; app/services/alert_processor.py:656-659 fire-and-forget). A transient channel failure is logged/recorded but the committed dedup row suppresses any retry — the notification is permanently dropped. Decide the delivery semantics, then implement: Option A (recommended): delivery-outbox table keyed (alert record, channel) with status/attempts/last_error; scheduler job retries pending/failed with backoff until a max age; dispatch tasks tracked (fixes the GC-able fire-and-forget tasks too). Option B (minimal): on failure, mark the per-channel dedup row retryable with an attempts cap. Rationale for scheduling with v1.2.0: the formatter migration (#14) already touches every notifier; doing the dispatch-semantics change in the same wave avoids rewriting the notifier boundary twice. Pull earlier if the silent-drop rate observed in diagnostics warrants it. **Acceptance criteria:** - [ ] A transient per-channel failure results in a later successful delivery - [ ] Retries are bounded (attempts + max age) and observable per channel - [ ] No duplicate delivery for the same (alert, channel) on retry - [ ] Warning-class NWS alerts covered first --- _Filed from the 2026-07-17 codebase audit (`docs/.internal/report-2026-07-17.md`), finding F-11._

claude-bot added this to the v1.2.0 - Notification formatting & delivery reliability milestone

2026-07-18 05:31:16 +00:00

claude-bot added the

type:feature

area:notifications

type:decision

labels

2026-07-18 05:31:16 +00:00

claude-bot referenced this issue

2026-07-18 05:32:12 +00:00

Migrate richer channels onto the shared notification formatter #14

rbrooks referenced this issue from a commit

2026-07-18 20:57:52 +00:00

Add delivery outbox with bounded retries; harden pipeline lifecycle

claude-bot referenced this issue from a pull request that will close it,

2026-07-18 20:58:08 +00:00

v1.2.0 Phase B: delivery outbox + pipeline lifecycle #100

claude-bot closed this issue

2026-07-18 21:04:36 +00:00

claude-bot commented

2026-07-18 21:05:48 +00:00

Author

Contributor

Done in #100 (merged). Delivery is now at-least-once for the NWS warning path: a notification_deliveries outbox (migration 0020) records per-(alert, channel, kind) attempts; dispatch commits a durable pending row and sends inline, and a process_delivery_outbox job retries failed rows with exponential backoff (FOR UPDATE SKIP LOCKED) up to DELIVERY_MAX_ATTEMPTS/DELIVERY_MAX_AGE_HOURS before dead. Crucially, the notifiers now raise TransientDeliveryError/PermanentDeliveryError instead of swallowing HTTP errors, so the outbox actually observes and retries real transient failures (a sent row is never re-sent). Acceptance verified end-to-end: transient failure → later successful delivery; permanent → dead; no duplicates. SPC/MCD sends are recorded to the ledger for observability; SPC auto-retry is a documented follow-up (entangled with impact-page/AI side effects).

Done in #100 (merged). Delivery is now at-least-once for the NWS warning path: a `notification_deliveries` outbox (migration 0020) records per-(alert, channel, kind) attempts; dispatch commits a durable `pending` row and sends inline, and a `process_delivery_outbox` job retries `failed` rows with exponential backoff (`FOR UPDATE SKIP LOCKED`) up to `DELIVERY_MAX_ATTEMPTS`/`DELIVERY_MAX_AGE_HOURS` before `dead`. Crucially, the notifiers now raise `TransientDeliveryError`/`PermanentDeliveryError` instead of swallowing HTTP errors, so the outbox actually observes and retries real transient failures (a `sent` row is never re-sent). Acceptance verified end-to-end: transient failure → later successful delivery; permanent → dead; no duplicates. SPC/MCD sends are recorded to the ledger for observability; SPC auto-retry is a documented follow-up (entangled with impact-page/AI side effects).

claude-bot referenced this issue

2026-07-18 22:08:30 +00:00

Acknowledgement + re-notify (escalation) for warnings #81

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/WeatherBot#50

No description provided.

Rows
Columns