From f5b4aec0bd804dcde3904d2d475d182b88fa9101 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 7 Aug 2026 17:27:00 +0000 Subject: [PATCH] Align docs with archive semantics; fail loud on non-positive expiry (#43) - README: rewrite the database-target documentation (target-types bullet and the per-webhook databases section) to describe the shipped archiving semantics -- separate archive-{webhookID}.db, debounced close/reopen for offline archiving, auto-recreate, creation-validated optional expiry with prune-on-open, and fail-loud delivery on archive write errors -- replacing the stale always-successful stub description. - parseArchiveExpiry now returns an error for set-but-non-positive durations ("0s", "-5h") instead of silently defaulting to keep-forever, matching ValidateArchiveExpiry at creation time; the delivery then fails loudly like any other archive error. TestParseArchiveExpiry extended with zero and negative cases. - databaseTarget type comment: "fire-and-forget" -> "no-retry", matching the fail-loud behaviour. --- README.md | 31 ++++++++++++++------ internal/delivery/target_database.go | 2 +- internal/delivery/target_database_archive.go | 9 ++++-- internal/delivery/target_database_test.go | 30 +++++++++++-------- 4 files changed, 47 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 4621dd8..1514b61 100644 --- a/README.md +++ b/README.md @@ -363,10 +363,12 @@ events should be forwarded. greater than 0, failed deliveries are retried with exponential backoff up to `max_retries` attempts, protected by a per-target circuit breaker. -- **`database`** — Confirm the event is stored in the webhook's - per-webhook database (no external delivery). Since events are always - written to the per-webhook DB on ingestion, this target marks delivery - as immediately successful. Useful for ensuring durable event archival. +- **`database`** — Archive the full event as a row into a separate + per-webhook archive database (`archive-{webhookID}.db`) for long-term + retention, with an optional creation-validated expiry (default: keep + forever). No external delivery and no retries; an archive write + failure fails the delivery. See the database target section under + "Per-Webhook Event Databases" for the full semantics. - **`log`** — Write the event to the application log (stdout). Useful for debugging. @@ -512,11 +514,22 @@ This separation provides: page cache, and its own lock, so concurrent event ingestion across webhooks won't contend. -The **database target type** leverages this architecture: since events -are already stored in the per-webhook database by design, the database -target simply marks the delivery as immediately successful. The -per-webhook DB IS the dedicated event database — that's the whole point -of the database target type. +The **database target type** builds on this architecture to provide +long-term archiving, separate from the per-webhook event database (which +may prune events under its own retention). Delivering to a database +target writes the full event — body, headers, method, content type, and +webhook/entrypoint/event identifiers — as a row into a dedicated archive +database, `archive-{webhookID}.db`, stored under the data directory +beside the event database. After each write the archive handle is closed +and reopened, debounced to at most once per second, so an operator can +move the archive file away for offline archiving without stopping the +service; a moved or removed archive file is recreated automatically on +the next write. An optional `expiry` in the target's config JSON (e.g. +`{"expiry":"720h"}`) is validated when the target is created — the +default (unset or the literal `never`) keeps rows forever — and rows +older than the expiry are pruned each time the archive is (re)opened. An +archive write failure is never silent success: the delivery records a +failed attempt with the error and is marked failed. The **Slack target type** sends webhook events as formatted messages to any Slack-compatible incoming webhook URL (works with Slack, Mattermost, diff --git a/internal/delivery/target_database.go b/internal/delivery/target_database.go index 8258b5e..0443aaa 100644 --- a/internal/delivery/target_database.go +++ b/internal/delivery/target_database.go @@ -10,7 +10,7 @@ import ( "sneak.berlin/go/webhooker/internal/database" ) -// databaseTarget is a fire-and-forget target that archives the +// databaseTarget is a no-retry target that archives the // full inbound event into a per-webhook archive SQLite file, // separate from the per-webhook event database. The event is // already persisted in the per-webhook event DB by the time diff --git a/internal/delivery/target_database_archive.go b/internal/delivery/target_database_archive.go index 77f1589..547a0af 100644 --- a/internal/delivery/target_database_archive.go +++ b/internal/delivery/target_database_archive.go @@ -77,7 +77,10 @@ type archivedEvent struct { // parseArchiveExpiry reads the optional expiry from a database // target's config JSON. An empty config, an empty expiry, or // the literal "never" all mean keep forever, returned as a zero -// duration. Any other value is parsed as a Go duration. +// duration. Any other value must parse as a positive Go +// duration; a set-but-invalid value (unparseable, zero, or +// negative) is an error rather than a silent default, matching +// ValidateArchiveExpiry at target creation. func parseArchiveExpiry( configJSON string, ) (time.Duration, error) { @@ -106,7 +109,9 @@ func parseArchiveExpiry( } if dur <= 0 { - return 0, nil + return 0, fmt.Errorf( + "%w: %q", errArchiveExpiryNotPositive, cfg.Expiry, + ) } return dur, nil diff --git a/internal/delivery/target_database_test.go b/internal/delivery/target_database_test.go index 420bfaf..3ee38ac 100644 --- a/internal/delivery/target_database_test.go +++ b/internal/delivery/target_database_test.go @@ -250,15 +250,18 @@ func TestParseArchiveExpiry(t *testing.T) { t.Parallel() cases := []struct { - name string - in string - want time.Duration + name string + in string + want time.Duration + wantErr bool }{ - {"empty config", "", 0}, - {"explicit never", `{"expiry":"never"}`, 0}, - {"empty expiry", `{"expiry":""}`, 0}, - {"duration", `{"expiry":"1h"}`, time.Hour}, - {"zero duration", `{"expiry":"0s"}`, 0}, + {"empty config", "", 0, false}, + {"explicit never", `{"expiry":"never"}`, 0, false}, + {"empty expiry", `{"expiry":""}`, 0, false}, + {"duration", `{"expiry":"1h"}`, time.Hour, false}, + {"unparseable", `{"expiry":"nonsense"}`, 0, true}, + {"zero duration", `{"expiry":"0s"}`, 0, true}, + {"negative duration", `{"expiry":"-5h"}`, 0, true}, } for _, tc := range cases { @@ -266,15 +269,16 @@ func TestParseArchiveExpiry(t *testing.T) { t.Parallel() got, err := delivery.ExportParseArchiveExpiry(tc.in) + if tc.wantErr { + require.Error(t, err) + + return + } + require.NoError(t, err) assert.Equal(t, tc.want, got) }) } - - _, err := delivery.ExportParseArchiveExpiry( - `{"expiry":"nonsense"}`, - ) - require.Error(t, err) } // seedDatabaseTargetDelivery seeds a pending delivery for a