The reaped-row fallback renders the exact zero timestamp that #257 was filed against #298

Closed
opened 2026-08-24 06:44:38 +02:00 by clawbot · 0 comments
Collaborator

Found by the review of #297, and created by that PR's own deliberate fallback.

#257 fixed the timestamp by reading created_at from the stored event row. A read failure on that path is intentionally NON-fatal — the delivery still goes out — because the event row can be hard-deleted by retention while a queued delivery still holds its inlined body, and dropping a deliverable event to protect one metadata field is the worse failure. That trade is right and is not what this issue disputes.

The problem is what the fallback renders. FormatSlackMessage (internal/delivery/target_slack.go:236) emits the Timestamp line unconditionally, so a fallback delivery carries:

*Timestamp:* `0001-01-01T00:00:00Z`

— the exact string #257 was filed against. Verified end to end: row hard-deleted, delivery goes out, WARN logged, payload carries the zero stamp.

So a human reading that message cannot distinguish "the event row was reaped, which is expected and harmless" from "the bug is back". The fallback is correct behaviour wearing the costume of a known defect.

The reaped-row path is real, not theoretical: reapExpired (internal/database/retention.go:273-320) hard-deletes events purely on created_at < cutoff with no delivery-status guard. Retention is day-granular, so the realistic trigger is an operator LOWERING a webhook's RetentionDays, not a slow queue.

Definition of done

  • When CreatedAt.IsZero(), FormatSlackMessage omits the Timestamp line, or marks it in a way that reads as "unknown" rather than as a date in the year 1. Either is fine; state which you chose.
  • A test covering the reaped-row fallback asserting the rendered message contains no 0001-01-01.
  • Check the other target types for the same shape while you are there. The log target formats event metadata too — if it prints a zero time in the same situation, it has the same problem for the same reason.

Not milestoned: the delivery still goes out with its payload intact, which is what matters, and the trigger requires retention to reap a row while a delivery holding its body is still queued.

Found by the review of https://git.eeqj.de/sneak/webhooker/pulls/297, and created by that PR's own deliberate fallback. https://git.eeqj.de/sneak/webhooker/issues/257 fixed the timestamp by reading `created_at` from the stored event row. A read failure on that path is intentionally NON-fatal — the delivery still goes out — because the event row can be hard-deleted by retention while a queued delivery still holds its inlined body, and dropping a deliverable event to protect one metadata field is the worse failure. That trade is right and is not what this issue disputes. The problem is what the fallback renders. `FormatSlackMessage` (`internal/delivery/target_slack.go:236`) emits the Timestamp line unconditionally, so a fallback delivery carries: ``` *Timestamp:* `0001-01-01T00:00:00Z` ``` — the exact string #257 was filed against. Verified end to end: row hard-deleted, delivery goes out, WARN logged, payload carries the zero stamp. So a human reading that message cannot distinguish "the event row was reaped, which is expected and harmless" from "the bug is back". The fallback is correct behaviour wearing the costume of a known defect. The reaped-row path is real, not theoretical: `reapExpired` (`internal/database/retention.go:273-320`) hard-deletes events purely on `created_at < cutoff` with no delivery-status guard. Retention is day-granular, so the realistic trigger is an operator LOWERING a webhook's `RetentionDays`, not a slow queue. ## Definition of done - When `CreatedAt.IsZero()`, `FormatSlackMessage` omits the Timestamp line, or marks it in a way that reads as "unknown" rather than as a date in the year 1. Either is fine; state which you chose. - A test covering the reaped-row fallback asserting the rendered message contains no `0001-01-01`. - Check the other target types for the same shape while you are there. The `log` target formats event metadata too — if it prints a zero time in the same situation, it has the same problem for the same reason. Not milestoned: the delivery still goes out with its payload intact, which is what matters, and the trigger requires retention to reap a row while a delivery holding its body is still queued.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#298