Connections: a failing webhook shows as working again after the next event is queued #574

Closed
opened 2026-10-01 10:47:45 +00:00 by tiagoagueda · 0 comments
Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, following where a webhook connection's last_error and last_ok_at are written, and confirmed with a probe on the snapshot.

Checked again on 2026-10-01 against main at 0a5cad587: still present.

The webhook notifier's send only queues a row, but notify treats any send that returns as a delivery and marks the connection as working. The real outcome is written later by the delivery pass, and the next queued event wipes it. So a receiver that answers 404, or has been down all day, shows on Connections as working whenever anything has happened since.

  • Where: notifications/service.py:71-92, _deliver: connection.record_test(True) at line 91 runs after plugin.send(...) returns; plugins/webhook/__init__.py:193-197, WebhookNotifier.send, only calls webhooks.enqueue_for; notifications/webhooks.py:134-152, deliver, records the real answer with record_test. Separately, notifications/webhooks.py:128-129 gives a row up on DestinationRefused without writing anything on the connection.
  • What happens: a delivery gets a 404; the pass writes "The receiver answered 404." on the connection. The person's next status change, reminder or capture queues another row, and record_test(True) clears last_error and sets last_ok_at to now. In the probe, last_error was "The receiver answered 404." after the pass and '' after one more notify. A receiver whose address now resolves to a private network is given up on at every delivery and the connection never says so at all.
  • Why it matters: the connection's status is the only place a person learns that their automation has stopped receiving events, and it reports success while every delivery fails.
  • Fix: let a notifier that delivers later say so, for example a class attribute delivers_later = True on WebhookNotifier, and have _deliver count it but skip record_test(True) for it, leaving the delivery pass as the only writer of the outcome. In deliver, call connection.record_test(False, str(refused)) before giving up on a refused destination.

Done when

  • A test in tests/test_webhooks.py delivers a row that gets a 404, then calls notify again, and expects the connection's last_error to still name the 404.
  • A test in the same file refuses the destination at delivery and expects the reason on the connection.
Found on 2026-09-29 in a read-only review of postulo at 95348908d, following where a webhook connection's `last_error` and `last_ok_at` are written, and confirmed with a probe on the snapshot. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The webhook notifier's `send` only queues a row, but `notify` treats any `send` that returns as a delivery and marks the connection as working. The real outcome is written later by the delivery pass, and the next queued event wipes it. So a receiver that answers 404, or has been down all day, shows on *Connections* as working whenever anything has happened since. - **Where:** `notifications/service.py:71-92`, `_deliver`: `connection.record_test(True)` at line 91 runs after `plugin.send(...)` returns; `plugins/webhook/__init__.py:193-197`, `WebhookNotifier.send`, only calls `webhooks.enqueue_for`; `notifications/webhooks.py:134-152`, `deliver`, records the real answer with `record_test`. Separately, `notifications/webhooks.py:128-129` gives a row up on `DestinationRefused` without writing anything on the connection. - **What happens:** a delivery gets a 404; the pass writes "The receiver answered 404." on the connection. The person's next status change, reminder or capture queues another row, and `record_test(True)` clears `last_error` and sets `last_ok_at` to now. In the probe, `last_error` was "The receiver answered 404." after the pass and `''` after one more `notify`. A receiver whose address now resolves to a private network is given up on at every delivery and the connection never says so at all. - **Why it matters:** the connection's status is the only place a person learns that their automation has stopped receiving events, and it reports success while every delivery fails. - **Fix:** let a notifier that delivers later say so, for example a class attribute `delivers_later = True` on `WebhookNotifier`, and have `_deliver` count it but skip `record_test(True)` for it, leaving the delivery pass as the only writer of the outcome. In `deliver`, call `connection.record_test(False, str(refused))` before giving up on a refused destination. ## Done when - A test in `tests/test_webhooks.py` delivers a row that gets a 404, then calls `notify` again, and expects the connection's `last_error` to still name the 404. - A test in the same file refuses the destination at delivery and expects the reason on the connection.
tiagoagueda added this to the 0.6.0 milestone 2026-10-01 10:47:45 +00:00
Sign in to join this conversation.
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
Postulo/postulo#574
No description provided.