Stop all-clear delivery retrying forever (#148) #151

Merged

claude-bot merged 1 commit from fix/webex-permanent-failure into main

2026-07-28 02:07:59 +00:00

claude-bot commented

2026-07-28 01:58:20 +00:00

Contributor

Closes #148.

Problem

A Webex all-clear returning HTTP 400 was retried once a minute, indefinitely in production.

The classification was already correct — classify_http_error maps a 4xx other than 408/429 to PermanentDeliveryError, and webex.send_alert_lifted raises it. The loss happened one layer up: _dispatch_alert_lifted caught every exception and returned a bare False, throwing the distinction away. The caller only stamped lifted_notified_at on success, so the row stayed pending and send_pending_lifted_notifications re-selected it every cycle — for a request that could never succeed.

Not Webex-specific. _dispatch_alert_lifted is the shared dispatch for every channel type, so the same unbounded loop applied to all of them. That covers the issue's "check the other notifiers for the same pattern" task: there was one pattern, in one place.

Change

_dispatch_alert_lifted returns (sent, permanent), preserving what the notifier already worked out.
record_delivery_result accepts permanent=True and marks the ledger row dead immediately, rather than burning through delivery_max_attempts on a request that cannot succeed.
The all-clear loop stamps lifted_notified_at once the delivery row reaches dead — whether from a permanent rejection or from transient failures that exhausted delivery_max_attempts / delivery_max_age_hours.

Transient failures still retry. They just stop eventually. No new configuration: this reuses the bounds the outbox already had, which the all-clear path was simply not consulting.

Diagnosability

The Webex response body is now logged on rejection, with the trackingId header. Webex explains a rejection in the body's message field; the status code alone is not actionable, which is exactly why the production 400 could not be diagnosed from logs. Only the response is logged — the bot token lives in the request headers and is never echoed back.

That is the piece that will tell you why channel e70842aa is 400ing, which I could not determine from the code alone. Likely candidates are a deleted room or a revoked token, but the body will say.

Tests

tests/test_all_clear_retry_bounds.py:

permanent failure → ledger row dead after exactly 1 attempt, record stamped
the reported symptom: four cycles of a permanent failure → exactly one send attempt, not four
transient failure → retries, but stops at delivery_max_attempts (3 sends across 6 cycles) and ends dead
control: the happy path still sends exactly once across repeated cycles and marks the delivery sent

Verification

Step	Result
`ruff check .`	passed
`python -m compileall app`	OK
SQLite bulk tier	809 passed
`alembic upgrade head` on fresh Postgres 16	clean
Postgres integration tier	4 passed

Note

This stops the loop; it does not retroactively clear the stuck row in production. The existing sa-* record for channel e70842aa still has lifted_notified_at NULL, so on deploy it will make one final attempt, fail, and be marked dead — after which the log goes quiet and the delivery row carries the reason.

🤖 Generated with Claude Code

Closes #148. ## Problem A Webex all-clear returning HTTP 400 was retried **once a minute, indefinitely** in production. The classification was already correct — `classify_http_error` maps a 4xx other than 408/429 to `PermanentDeliveryError`, and `webex.send_alert_lifted` raises it. The loss happened one layer up: `_dispatch_alert_lifted` caught every exception and returned a bare `False`, throwing the distinction away. The caller only stamped `lifted_notified_at` on success, so the row stayed pending and `send_pending_lifted_notifications` re-selected it every cycle — for a request that could never succeed. **Not Webex-specific.** `_dispatch_alert_lifted` is the shared dispatch for every channel type, so the same unbounded loop applied to all of them. That covers the issue's "check the other notifiers for the same pattern" task: there was one pattern, in one place. ## Change - `_dispatch_alert_lifted` returns `(sent, permanent)`, preserving what the notifier already worked out. - `record_delivery_result` accepts `permanent=True` and marks the ledger row **dead immediately**, rather than burning through `delivery_max_attempts` on a request that cannot succeed. - The all-clear loop stamps `lifted_notified_at` once the delivery row reaches `dead` — whether from a permanent rejection or from transient failures that exhausted `delivery_max_attempts` / `delivery_max_age_hours`. Transient failures still retry. They just stop eventually. No new configuration: this reuses the bounds the outbox already had, which the all-clear path was simply not consulting. ## Diagnosability The Webex response body is now logged on rejection, with the `trackingId` header. Webex explains a rejection in the body's `message` field; the status code alone is not actionable, which is exactly why the production 400 could not be diagnosed from logs. **Only the response is logged** — the bot token lives in the request headers and is never echoed back. That is the piece that will tell you *why* channel `e70842aa` is 400ing, which I could not determine from the code alone. Likely candidates are a deleted room or a revoked token, but the body will say. ## Tests `tests/test_all_clear_retry_bounds.py`: - permanent failure → ledger row `dead` after exactly 1 attempt, record stamped - **the reported symptom:** four cycles of a permanent failure → exactly **one** send attempt, not four - transient failure → retries, but stops at `delivery_max_attempts` (3 sends across 6 cycles) and ends `dead` - **control:** the happy path still sends exactly once across repeated cycles and marks the delivery `sent` ## Verification | Step | Result | |---|---| | `ruff check .` | passed | | `python -m compileall app` | OK | | SQLite bulk tier | 809 passed | | `alembic upgrade head` on fresh Postgres 16 | clean | | Postgres integration tier | 4 passed | ## Note This stops the loop; it does not retroactively clear the stuck row in production. The existing `sa-*` record for channel `e70842aa` still has `lifted_notified_at` NULL, so on deploy it will make one final attempt, fail, and be marked dead — after which the log goes quiet and the delivery row carries the reason. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit

2026-07-28 01:58:20 +00:00

Stop all-clear delivery retrying forever (#148 )

CI / test (pull_request) Successful in 4m47s

Details

d441e5f0f1

A Webex all-clear returning HTTP 400 was retried once a minute, indefinitely,
in production. The notifiers already classify their failures correctly --
classify_http_error maps a 4xx other than 408/429 to PermanentDeliveryError --
but _dispatch_alert_lifted caught every exception and returned a bare False,
discarding the distinction. The caller only stamped lifted_notified_at on
success, so the row stayed pending and was re-selected on every cycle, for a
request that could never succeed.

Not Webex-specific: _dispatch_alert_lifted is the shared dispatch for every
channel type, so the same unbounded loop applied to all of them.

- _dispatch_alert_lifted now returns (sent, permanent), preserving what the
  notifier already determined.
- record_delivery_result takes permanent=True and marks the ledger row dead
  immediately rather than burning through delivery_max_attempts on a request
  that cannot succeed.
- The all-clear loop stamps lifted_notified_at when the delivery row reaches
  `dead`, whether from a permanent rejection or from transient failures that
  exhausted delivery_max_attempts / delivery_max_age_hours. Transient failures
  still retry; they just stop eventually. The delivery row keeps the diagnosis.

Also log the Webex response body and trackingId on rejection. Webex explains a
rejection in the body (`message`) and ties it to a trackingId header; the
status code alone is not actionable, which is why the production 400 could not
be diagnosed from logs. Only the response is logged -- the bot token lives in
the request headers and is never echoed back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot merged commit b6b22a26b5 into main

2026-07-28 02:07:59 +00:00

claude-bot referenced this pull request from a commit

2026-07-28 02:08:00 +00:00

Merge pull request 'Stop all-clear delivery retrying forever (#148)' (#151) from fix/webex-permanent-failure into main

claude-bot referenced this pull request

2026-07-28 02:08:27 +00:00

Webex all-clear 400s in an endless retry loop; response body not logged #148