Resubmit a stored event as a new undelivered event (closes #250) #251

Merged
clawbot merged 1 commits from issue-250-event-resubmit into next 2026-08-24 00:53:38 +02:00
Collaborator

Closes #250.

What changed

A per-event Resubmit action on the event log stores a NEW event copying the stored one's method, headers, body and content_type verbatim, then fans it out to the webhook's currently ACTIVE targets, resolved fresh by the query the receiver uses. A target created long after the original event arrived receives it — which is the thing per-delivery replay cannot do, since a target created for a dev backend has no prior delivery to replay. Inactive targets are skipped as the receiver skips them; a source with no active targets still stores the event and says so. The action is repeatable: replay's in-flight refusal is deliberately not ported.

  • POST /source/{sourceID}/events/{eventID}/resubmit, in the existing /source/{sourceID} group in internal/server/routes.go, so auth, CSRF, NoCache and the 1 MB body cap already apply. Attached with r.With(s.mw.ResubmitRateLimit()), mirroring the replay route.
  • ResubmitRateLimit() in internal/middleware/ratelimit.go: its own bucket, so exhausting it does not also disable replay.
  • webhooker_events_resubmitted_total in internal/metrics/metrics.go.
  • Nullable resubmitted_from_id on Event, via AutoMigrate. The event log shows both directions: "Resubmitted from event <id>" on a copy, "Resubmitted as N new events" on the source.
  • Per-delivery replay is untouched.

The shared-helper refactor

The receiver and resubmit share ONE construction and ONE fan-out site, in internal/handlers/webhook.go:

  • eventSource carries where the fields came from — live request (requestEventSource) or stored event — plus the optional source event id.
  • createAndFanOut(src, targets) opens the per-webhook transaction, creates the event, builds the deliveries, commits, counts, and hands the tasks to the same Notifier. It is now the only path by which an event and its deliveries are created, so a resubmitted delivery is retried, SSRF-guarded and circuit-broken exactly as a first one is.
  • buildDeliveryTasks lost its http.ResponseWriter and returns an error, which is what lets both callers share it; finishWebhookResponse no longer notifies, since the shared site does.
  • The stored event is read once, before the write transaction, through cast(body as blob) and GORM's soft-delete scope, so the copy is byte-identical and a reaped event is not resubmittable. Holding that read outside the transaction keeps a 1 MB body from extending the per-webhook write lock against the receiver.

Inbound signature verification is not re-run, with a comment on HandleEventResubmit saying so, so it is not later read as a bypass.

Verification

make check green, exit 0, 70s wall, run with GOFLAGS=-count=1 so no test result came from cache — every package shows a real duration (internal/handlers 20.116s), and the Docker lint stage reported 0 issues after a real 47.4s run.

New tests (all in internal/handlers/event_resubmit_test.go unless noted):

  • TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent — the headline: event captured first, target created after, resubmit reaches it.
  • TestHandleEventResubmit_IsRepeatable — five presses with nothing marked finished in between; five events, five deliveries, no refusal.
  • TestHandleEventResubmit_OversizeBodySurvivesIntact — a body over delivery.MaxInlineBodySize containing a multibyte rune, a NUL and an invalid UTF-8 byte; the task carries no inline body and the engine's own read of the new event row returns the bytes unchanged.
  • TestHandleEventResubmit_SkipsInactiveTarget — skipped, not an error.
  • TestHandleEventResubmit_NoActiveTargetsStillStoresEvent.
  • TestHandleEventResubmit_RefusesEventOfAnotherWebhook — another user's webhook, another webhook's event id, an unknown id and a malformed id are all 404 and queue nothing.
  • TestHandleSourceLogs_ShowsResubmitProvenance — both directions rendered, action offered per event.

Against a running instance with a real HTTP sink

Ports 18601 (app) and 18602 (sink), data dir under /tmp/impl-250. No container or image was created; nothing was pruned.

A 40019-byte body (over the 16 KiB inline limit, ending in \xc3\xa9\x00\xff) was POSTed to the receiver by the pre-change binary. The post-change binary was then started on that same data dir, a target dev-backend was created — after the event existed — and the event was resubmitted twice from the event log:

deliveries of the stored event before any resubmit: 0
resubmit 1: status=303 location=.../logs?resubmit=queued
resubmit 2: status=303 location=.../logs?resubmit=queued

=== what the sink received ===
POST /sink content-type=application/json bytes=40019 x-test=original-capture
POST /sink content-type=application/json bytes=40019 x-test=original-capture
2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941  sink-out/body-1.bin
2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941  sink-out/body-2.bin
2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941  sent-body.bin

Byte-identical to what was sent, including the stored X-Test-Marker. The original event had zero deliveries, so there was nothing replay could have used.

id        bytes  from_event  deliveries       event     status
--------  -----  ----------  ----------       --------  ---------
10ad1122  40019  -           0                342b3f11  delivered
342b3f11  40019  10ad1122    1                da80dbfa  delivered
da80dbfa  40019  10ad1122    1

webhooker_events_resubmitted_total 2
webhooker_events_received_total 2
webhooker_delivery_attempts_total{target_type="http"} 2

The page rendered Resubmitted as 2 new events on the source event and Resubmitted from event 10ad1122-... on each copy.

AutoMigrate onto an existing populated data dir

The data dir above was created and populated by a binary built from next at a83e8fe, whose schema has no such column:

CREATE TABLE `events` (`id` uuid,...,`headers` text,`body` text,`content_type` text, PRIMARY KEY (`id`), ...)

Starting the post-change binary on that same directory:

 `resubmitted_from_id` uuid
CREATE INDEX `idx_events_resubmitted_from_id` ON `events`(`resubmitted_from_id`);
events still present: 1
stored body sha256 vs sent body sha256:
2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941  sent-body.bin
2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941  stored-body.bin

Column and index added, the pre-existing event intact and resubmitted_from_id IS NULL on it.

Documentation

All four false claims corrected, including README.md:1544, which told the operator that a delivery stranded by a target type change could not be redelivered — that delivery is failed, hence terminal, hence replayable, and the event is resubmittable. Replay (one finished delivery, its own target, recovery) and resubmit (the event, all currently active targets, testing) are documented side by side, plus the new route, metric, Event column and source-tree entry.

Deviations

One, stated plainly. The issue asks for the new counter to be "labelled with the route pattern". webhooker_events_resubmitted_total is unlabelled instead: the route pattern has exactly one value at the only call site, so the label would distinguish nothing while adding a dimension to a counter that has none to give, and the target types the event fans out to already belong to the delivery series. The cardinality point the instruction protects is honoured — no id, path or client-chosen value is a label. Both the metric's doc comment and the README table state this.

TODO.md is deliberately untouched, per #112.

Closes https://git.eeqj.de/sneak/webhooker/issues/250. ## What changed A per-event **Resubmit** action on the event log stores a NEW event copying the stored one's `method`, `headers`, `body` and `content_type` verbatim, then fans it out to the webhook's currently ACTIVE targets, resolved fresh by the query the receiver uses. A target created long after the original event arrived receives it — which is the thing per-delivery replay cannot do, since a target created for a dev backend has no prior delivery to replay. Inactive targets are skipped as the receiver skips them; a source with no active targets still stores the event and says so. The action is repeatable: replay's in-flight refusal is deliberately not ported. - `POST /source/{sourceID}/events/{eventID}/resubmit`, in the existing `/source/{sourceID}` group in `internal/server/routes.go`, so auth, CSRF, `NoCache` and the 1 MB body cap already apply. Attached with `r.With(s.mw.ResubmitRateLimit())`, mirroring the replay route. - `ResubmitRateLimit()` in `internal/middleware/ratelimit.go`: its own bucket, so exhausting it does not also disable replay. - `webhooker_events_resubmitted_total` in `internal/metrics/metrics.go`. - Nullable `resubmitted_from_id` on `Event`, via `AutoMigrate`. The event log shows both directions: "Resubmitted from event &lt;id&gt;" on a copy, "Resubmitted as N new events" on the source. - Per-delivery replay is untouched. ## The shared-helper refactor The receiver and resubmit share ONE construction and ONE fan-out site, in `internal/handlers/webhook.go`: - `eventSource` carries where the fields came from — live request (`requestEventSource`) or stored event — plus the optional source event id. - `createAndFanOut(src, targets)` opens the per-webhook transaction, creates the event, builds the deliveries, commits, counts, and hands the tasks to the same `Notifier`. It is now the only path by which an event and its deliveries are created, so a resubmitted delivery is retried, SSRF-guarded and circuit-broken exactly as a first one is. - `buildDeliveryTasks` lost its `http.ResponseWriter` and returns an error, which is what lets both callers share it; `finishWebhookResponse` no longer notifies, since the shared site does. - The stored event is read once, before the write transaction, through `cast(body as blob)` and GORM's soft-delete scope, so the copy is byte-identical and a reaped event is not resubmittable. Holding that read outside the transaction keeps a 1 MB body from extending the per-webhook write lock against the receiver. Inbound signature verification is not re-run, with a comment on `HandleEventResubmit` saying so, so it is not later read as a bypass. ## Verification `make check` green, exit 0, **70s** wall, run with `GOFLAGS=-count=1` so no test result came from cache — every package shows a real duration (`internal/handlers 20.116s`), and the Docker lint stage reported `0 issues` after a real 47.4s run. New tests (all in `internal/handlers/event_resubmit_test.go` unless noted): - `TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent` — the headline: event captured first, target created after, resubmit reaches it. - `TestHandleEventResubmit_IsRepeatable` — five presses with nothing marked finished in between; five events, five deliveries, no refusal. - `TestHandleEventResubmit_OversizeBodySurvivesIntact` — a body over `delivery.MaxInlineBodySize` containing a multibyte rune, a NUL and an invalid UTF-8 byte; the task carries no inline body and the engine's own read of the new event row returns the bytes unchanged. - `TestHandleEventResubmit_SkipsInactiveTarget` — skipped, not an error. - `TestHandleEventResubmit_NoActiveTargetsStillStoresEvent`. - `TestHandleEventResubmit_RefusesEventOfAnotherWebhook` — another user's webhook, another webhook's event id, an unknown id and a malformed id are all 404 and queue nothing. - `TestHandleSourceLogs_ShowsResubmitProvenance` — both directions rendered, action offered per event. ### Against a running instance with a real HTTP sink Ports 18601 (app) and 18602 (sink), data dir under `/tmp/impl-250`. No container or image was created; nothing was pruned. A 40019-byte body (over the 16 KiB inline limit, ending in `\xc3\xa9\x00\xff`) was POSTed to the receiver by the **pre-change** binary. The **post-change** binary was then started on that same data dir, a target `dev-backend` was created — after the event existed — and the event was resubmitted twice from the event log: ``` deliveries of the stored event before any resubmit: 0 resubmit 1: status=303 location=.../logs?resubmit=queued resubmit 2: status=303 location=.../logs?resubmit=queued === what the sink received === POST /sink content-type=application/json bytes=40019 x-test=original-capture POST /sink content-type=application/json bytes=40019 x-test=original-capture 2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941 sink-out/body-1.bin 2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941 sink-out/body-2.bin 2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941 sent-body.bin ``` Byte-identical to what was sent, including the stored `X-Test-Marker`. The original event had zero deliveries, so there was nothing replay could have used. ``` id bytes from_event deliveries event status -------- ----- ---------- ---------- -------- --------- 10ad1122 40019 - 0 342b3f11 delivered 342b3f11 40019 10ad1122 1 da80dbfa delivered da80dbfa 40019 10ad1122 1 webhooker_events_resubmitted_total 2 webhooker_events_received_total 2 webhooker_delivery_attempts_total{target_type="http"} 2 ``` The page rendered `Resubmitted as 2 new events` on the source event and `Resubmitted from event 10ad1122-...` on each copy. ### AutoMigrate onto an existing populated data dir The data dir above was created and populated by a binary built from `next` at `a83e8fe`, whose schema has no such column: ``` CREATE TABLE `events` (`id` uuid,...,`headers` text,`body` text,`content_type` text, PRIMARY KEY (`id`), ...) ``` Starting the post-change binary on that same directory: ``` `resubmitted_from_id` uuid CREATE INDEX `idx_events_resubmitted_from_id` ON `events`(`resubmitted_from_id`); events still present: 1 stored body sha256 vs sent body sha256: 2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941 sent-body.bin 2697d62c83f77ff36ae8adf42679dc6a6f48e3069d25fabd964db8175ab69941 stored-body.bin ``` Column and index added, the pre-existing event intact and `resubmitted_from_id IS NULL` on it. ## Documentation All four false claims corrected, including `README.md:1544`, which told the operator that a delivery stranded by a target type change could not be redelivered — that delivery is `failed`, hence terminal, hence replayable, and the event is resubmittable. Replay (one finished delivery, its own target, recovery) and resubmit (the event, all currently active targets, testing) are documented side by side, plus the new route, metric, `Event` column and source-tree entry. ## Deviations One, stated plainly. The issue asks for the new counter to be "labelled with the route pattern". `webhooker_events_resubmitted_total` is unlabelled instead: the route pattern has exactly one value at the only call site, so the label would distinguish nothing while adding a dimension to a counter that has none to give, and the target types the event fans out to already belong to the delivery series. The cardinality point the instruction protects is honoured — no id, path or client-chosen value is a label. Both the metric's doc comment and the README table state this. `TODO.md` is deliberately untouched, per https://git.eeqj.de/sneak/webhooker/issues/112.
clawbot added 1 commit 2026-08-24 00:42:59 +02:00
Resubmit a stored event as a new undelivered event (closes #250)
All checks were successful
check / check (push) Successful in 3m3s
f3cb56345f
Capturing real webhook traffic and firing it repeatedly at a backend
under development is a primary function of this service, and
per-delivery replay cannot do it: it only ever resolves the delivery's
own original target, so a target created for a dev backend has no
prior delivery and nothing can be replayed to it.

The event log now offers a per-event Resubmit action. It stores a NEW
event copying the stored one's method, headers, body and content type
verbatim, and fans it out to the webhook's currently ACTIVE targets,
resolved fresh by the query the receiver uses -- so a target created
long after the original event arrived receives it. The original
event's deliveries have no bearing on where the copy goes, inactive
targets are skipped as the receiver skips them, and the action is
repeatable: replay's in-flight refusal is deliberately not ported,
because firing one captured event over and over is the point.

The receiver and the resubmit path share one construction and one
fan-out site. An eventSource value carries where the fields came from,
live request or stored event, and createAndFanOut writes the event and
its pending deliveries in one transaction and hands the tasks to the
same Notifier, so a resubmitted delivery is retried, SSRF-guarded and
circuit-broken exactly as a first one is. buildDeliveryTasks returns
an error instead of writing a response, which is what lets both
callers share it.

The stored event is read once, before the write transaction, with a
cast to blob, so a body over delivery.MaxInlineBodySize is copied byte
for byte and the engine loads it from the new event row.

A nullable resubmitted_from_id records provenance -- empty for an
event that arrived on the receiver -- and the event log reports the
relationship in both directions, without which the log is unreadable
after a few resubmits of one event. The route sits in the owned-source
group, so auth, CSRF and the body cap apply, with its own rate limit
bucket and an events_resubmitted_total counter.

Inbound signature verification is not re-run: there is no inbound
signature to check on a copy an authenticated, CSRF-protected operator
action submits.

Per-delivery replay is unchanged; it serves recovery, which resubmit
does not replace. The README claimed in four places that replay was
unimplemented, one of them telling the operator that a delivery
stranded by a target type change was lost; all four are corrected and
resubmit is documented beside replay.
clawbot added the needs-review label 2026-08-24 00:43:06 +02:00
clawbot self-assigned this 2026-08-24 00:43:06 +02:00
Author
Collaborator

PASS — independent review against #250. Every "Definition of done", "Implementation requirements" and "Verification" item is met, verified by execution on my own clone and a live instance, not by reading the PR body.

Disclosures and anomalies (all accepted, none blocking):

  • Receiver-path refactor is ordering-neutral. createAndFanOut keeps commit → EventReceived()Notify() → log → 200 in exactly the pre-change order; Notify still fires strictly after tx.Commit() returns, and the "durably stored" counting point is unmoved. buildDeliveryTasks returning an error introduced no double-write or missing-write: every failure path in createAndFanOut returns without touching the ResponseWriter, both callers write exactly one response via serverError, and finishWebhookResponse is reachable only on success. Only one call site each for buildDeliveryTasks and finishWebhookResponse; no stragglers of the removed beginWebhookTx/buildEvent.
  • webhooker_events_received_total now also counts resubmitted events. A real widening of that series' meaning, deliberate and documented in the metric comment, the README and the PR body. Flagging it so it is a decision on record, not a surprise on a dashboard.
  • Metric deviation accepted: the true event-level sibling is webhooker_events_received_total, which is also unlabelled. Unlabelled is the consistent choice, not an inconsistency with webhooker_delivery_replays_total (delivery-level, hence target_type). Nothing here touches or worsens the metrics middleware's handler labelling.
  • Nit, not a defect: the loadResubmitSource comment credits GORM's soft-delete scope for refusing a reaped event, but the retention reaper hard-deletes (internal/database/retention.go:288 is Unscoped()). Behaviour is right either way. Likewise createAndFanOut's "the only path by which an event and its deliveries are created" — replay creates a delivery without an event.
  • The new tests invoke HandleEventResubmit() directly, so auth, CSRF and the rate limit are not exercised through the router. I verified all three against a live instance instead: unauthenticated → 403/redirect, missing/garbage/foreign CSRF token → 403, GET → 405, and 30/min then 429 with replay's bucket still spending independently. Unknown, malformed and cross-source eventID and foreign sourceID all 404 and queued nothing.

What I executed: a 71220-byte body of genuinely binary data (all 256 byte values, embedded NULs, invalid UTF-8, random bytes — well over delivery.MaxInlineBodySize) POSTed to the receiver of a binary built at a83e8fe, with no target in existence. Post-change binary started on that same populated data dir: resubmitted_from_id and its index added by ALTER TABLE, the pre-existing event intact at sha256 5d10ee05... with the column NULL. A target created only afterwards then received three resubmits, each byte-identical at that same sha256 with X-Test-Marker preserved; the source event kept zero deliveries throughout. Inactive target → resubmit=no-targets, nothing dispatched, no error. Empty body and an event whose entrypoint had since been deleted both resubmit cleanly. Zero ERROR log lines across the whole session. Receiver signature verification unaffected: unsigned and wrongly-signed requests still 401 and store nothing, correctly-signed still 200.

make check green from a clean clone with GOFLAGS=-count=1, exit 0, 76s wall, zero (cached) markers, lint a real 47.19s run in the digest-pinned image reporting 0 issues. CI green on f3cb563. Fast-forward onto next. No config knob added, so the set-but-unparseable rule has no new surface. Commit message carries (closes #250), no attribution trailers, no scope creep, terminology and naming consistent with the replay counterparts.

**PASS** — independent review against https://git.eeqj.de/sneak/webhooker/issues/250. Every "Definition of done", "Implementation requirements" and "Verification" item is met, verified by execution on my own clone and a live instance, not by reading the PR body. Disclosures and anomalies (all accepted, none blocking): - **Receiver-path refactor is ordering-neutral.** `createAndFanOut` keeps commit &rarr; `EventReceived()` &rarr; `Notify()` &rarr; log &rarr; 200 in exactly the pre-change order; `Notify` still fires strictly after `tx.Commit()` returns, and the "durably stored" counting point is unmoved. `buildDeliveryTasks` returning an error introduced no double-write or missing-write: every failure path in `createAndFanOut` returns without touching the `ResponseWriter`, both callers write exactly one response via `serverError`, and `finishWebhookResponse` is reachable only on success. Only one call site each for `buildDeliveryTasks` and `finishWebhookResponse`; no stragglers of the removed `beginWebhookTx`/`buildEvent`. - **`webhooker_events_received_total` now also counts resubmitted events.** A real widening of that series' meaning, deliberate and documented in the metric comment, the README and the PR body. Flagging it so it is a decision on record, not a surprise on a dashboard. - Metric deviation accepted: the true event-level sibling is `webhooker_events_received_total`, which is also unlabelled. Unlabelled is the consistent choice, not an inconsistency with `webhooker_delivery_replays_total` (delivery-level, hence `target_type`). Nothing here touches or worsens the metrics middleware's `handler` labelling. - Nit, not a defect: the `loadResubmitSource` comment credits GORM's soft-delete scope for refusing a reaped event, but the retention reaper hard-deletes (`internal/database/retention.go:288` is `Unscoped()`). Behaviour is right either way. Likewise `createAndFanOut`'s "the only path by which an event and its deliveries are created" — replay creates a delivery without an event. - The new tests invoke `HandleEventResubmit()` directly, so auth, CSRF and the rate limit are not exercised through the router. I verified all three against a live instance instead: unauthenticated &rarr; 403/redirect, missing/garbage/foreign CSRF token &rarr; 403, `GET` &rarr; 405, and 30/min then `429` with replay's bucket still spending independently. Unknown, malformed and cross-source `eventID` and foreign `sourceID` all 404 and queued nothing. What I executed: a 71220-byte body of genuinely binary data (all 256 byte values, embedded NULs, invalid UTF-8, random bytes &mdash; well over `delivery.MaxInlineBodySize`) POSTed to the receiver of a binary built at `a83e8fe`, with no target in existence. Post-change binary started on that same populated data dir: `resubmitted_from_id` and its index added by `ALTER TABLE`, the pre-existing event intact at sha256 `5d10ee05...` with the column `NULL`. A target created only afterwards then received three resubmits, each byte-identical at that same sha256 with `X-Test-Marker` preserved; the source event kept zero deliveries throughout. Inactive target &rarr; `resubmit=no-targets`, nothing dispatched, no error. Empty body and an event whose entrypoint had since been deleted both resubmit cleanly. Zero `ERROR` log lines across the whole session. Receiver signature verification unaffected: unsigned and wrongly-signed requests still 401 and store nothing, correctly-signed still 200. `make check` green from a clean clone with `GOFLAGS=-count=1`, exit 0, 76s wall, zero `(cached)` markers, lint a real 47.19s run in the digest-pinned image reporting `0 issues`. CI green on `f3cb563`. Fast-forward onto `next`. No config knob added, so the set-but-unparseable rule has no new surface. Commit message carries `(closes #250)`, no attribution trailers, no scope creep, terminology and naming consistent with the replay counterparts.
clawbot merged commit 89f3b984d2 into next 2026-08-24 00:53:38 +02:00
clawbot deleted branch issue-250-event-resubmit 2026-08-24 00:53:38 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#251