Resubmit a stored event as a new undelivered event (closes #250) #251
Reference in New Issue
Block a user
Delete Branch "issue-250-event-resubmit"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #250.
What changed
A per-event Resubmit action on the event log stores a NEW event copying the stored one's
method,headers,bodyandcontent_typeverbatim, 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 ininternal/server/routes.go, so auth, CSRF,NoCacheand the 1 MB body cap already apply. Attached withr.With(s.mw.ResubmitRateLimit()), mirroring the replay route.ResubmitRateLimit()ininternal/middleware/ratelimit.go: its own bucket, so exhausting it does not also disable replay.webhooker_events_resubmitted_totalininternal/metrics/metrics.go.resubmitted_from_idonEvent, viaAutoMigrate. The event log shows both directions: "Resubmitted from event <id>" on a copy, "Resubmitted as N new events" on the source.The shared-helper refactor
The receiver and resubmit share ONE construction and ONE fan-out site, in
internal/handlers/webhook.go:eventSourcecarries 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 sameNotifier. 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.buildDeliveryTaskslost itshttp.ResponseWriterand returns an error, which is what lets both callers share it;finishWebhookResponseno longer notifies, since the shared site does.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
HandleEventResubmitsaying so, so it is not later read as a bypass.Verification
make checkgreen, exit 0, 70s wall, run withGOFLAGS=-count=1so no test result came from cache — every package shows a real duration (internal/handlers 20.116s), and the Docker lint stage reported0 issuesafter a real 47.4s run.New tests (all in
internal/handlers/event_resubmit_test.gounless 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 overdelivery.MaxInlineBodySizecontaining 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 targetdev-backendwas created — after the event existed — and the event was resubmitted twice from the event log: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.The page rendered
Resubmitted as 2 new eventson the source event andResubmitted 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
nextata83e8fe, whose schema has no such column:Starting the post-change binary on that same directory:
Column and index added, the pre-existing event intact and
resubmitted_from_id IS NULLon 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 isfailed, 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,Eventcolumn 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_totalis 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.mdis deliberately untouched, per #112.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):
createAndFanOutkeeps commit →EventReceived()→Notify()→ log → 200 in exactly the pre-change order;Notifystill fires strictly aftertx.Commit()returns, and the "durably stored" counting point is unmoved.buildDeliveryTasksreturning an error introduced no double-write or missing-write: every failure path increateAndFanOutreturns without touching theResponseWriter, both callers write exactly one response viaserverError, andfinishWebhookResponseis reachable only on success. Only one call site each forbuildDeliveryTasksandfinishWebhookResponse; no stragglers of the removedbeginWebhookTx/buildEvent.webhooker_events_received_totalnow 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.webhooker_events_received_total, which is also unlabelled. Unlabelled is the consistent choice, not an inconsistency withwebhooker_delivery_replays_total(delivery-level, hencetarget_type). Nothing here touches or worsens the metrics middleware'shandlerlabelling.loadResubmitSourcecomment credits GORM's soft-delete scope for refusing a reaped event, but the retention reaper hard-deletes (internal/database/retention.go:288isUnscoped()). Behaviour is right either way. LikewisecreateAndFanOut's "the only path by which an event and its deliveries are created" — replay creates a delivery without an event.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 then429with replay's bucket still spending independently. Unknown, malformed and cross-sourceeventIDand foreignsourceIDall 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 ata83e8fe, with no target in existence. Post-change binary started on that same populated data dir:resubmitted_from_idand its index added byALTER TABLE, the pre-existing event intact at sha2565d10ee05...with the columnNULL. A target created only afterwards then received three resubmits, each byte-identical at that same sha256 withX-Test-Markerpreserved; 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. ZeroERRORlog lines across the whole session. Receiver signature verification unaffected: unsigned and wrongly-signed requests still 401 and store nothing, correctly-signed still 200.make checkgreen from a clean clone withGOFLAGS=-count=1, exit 0, 76s wall, zero(cached)markers, lint a real 47.19s run in the digest-pinned image reporting0 issues. CI green onf3cb563. Fast-forward ontonext. 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.