From 9305af4f85c072e5c6c779dd97f71d470ca49389 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 2 Oct 2026 23:47:16 +0200 Subject: [PATCH] Show each target's delivered and failed deliveries in the target list (closes #372) The target list showed nothing about how a target's deliveries were going. Each target now shows Delivered and Failed, each in total and in the last 24 hours. The totals are the per-target running totals kept for the statistics pane, so retention does not reduce them; the 24-hour figures are one count over the deliveries' final-status index, for all of the webhook's targets at once. Pending and retrying deliveries count in neither. If the event database cannot be read, each row says so instead of showing zeros. The archive details and Download button on database targets are kept. Model: opus-5-5 --- README.md | 8 +- internal/handlers/target_list.go | 90 +++++++++++++++++++++ internal/handlers/target_list_test.go | 103 +++++++++++++++++++++++++ internal/server/alpine_browser_test.go | 28 ++++++- internal/server/routes_test.go | 15 +++- templates/source_detail.html | 12 +++ 6 files changed, 250 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 66359f2..8069395 100644 --- a/README.md +++ b/README.md @@ -1913,6 +1913,12 @@ counted with the pane's query. It opens each webhook's event database once with the number of webhooks and, for each, with the deliveries that finished in the last 24 hours, never with the events stored. +The target list on the webhook page shows, for each target, its +`delivered` and `failed` totals, which retention does not reduce, and its +deliveries that became `delivered` and `failed` in the last 24 hours, +counted with the pane's query. Deliveries still `pending` or `retrying` +count in neither. + #### Event-tier indexes These indexes on the per-webhook event databases are declared in the model @@ -1920,7 +1926,7 @@ tags, so `AutoMigrate` creates them on a fresh database: | Table | Columns | Serves | | ------------------ | --------------------------- | ------ | -| `deliveries` | `status`, `deleted_at`, `finished_at`, `target_id` | Startup recovery, the retry and pending sweeps every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status, and the webhook page's statistics and the webhook list, which count each target's deliveries by status and when they finished | +| `deliveries` | `status`, `deleted_at`, `finished_at`, `target_id` | Startup recovery, the retry and pending sweeps every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status, and the webhook page's statistics and target list and the webhook list, which count each target's deliveries by status and when they finished | | `deliveries` | `event_id`, `deleted_at` | The event log, which loads each event's deliveries, and retention, which counts and deletes the deliveries of expired events | | `delivery_results` | `delivery_id`, `deleted_at` | The event log, which loads the attempts of a page's deliveries, and retention, which deletes the attempts of expired events | | `events` | `deleted_at`, `created_at` | The webhook page's statistics, which count recent events | diff --git a/internal/handlers/target_list.go b/internal/handlers/target_list.go index 579e079..035a575 100644 --- a/internal/handlers/target_list.go +++ b/internal/handlers/target_list.go @@ -2,11 +2,13 @@ package handlers import ( "errors" + "fmt" "io/fs" "path/filepath" "time" "github.com/dustin/go-humanize" + "gorm.io/gorm" "sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/delivery" ) @@ -15,11 +17,27 @@ import ( type TargetRowView struct { delivery.TargetView + // Deliveries counts the target's delivered and failed deliveries, + // and is nil when the webhook's event database could not be read. + Deliveries *TargetDeliveries + // Archive is a database target's archive file, and nil for a target // of any other type. Archive *ArchiveFileView } +// TargetDeliveries is how many of a target's deliveries became +// delivered and how many failed: in total, which retention does not +// reduce, and in the last 24 hours. Deliveries still pending or +// retrying count in neither. +type TargetDeliveries struct { + Delivered int64 + Failed int64 + + DeliveredLast24Hours int64 + FailedLast24Hours int64 +} + // ArchiveFileView is what a database target's row shows about its // archive file. type ArchiveFileView struct { @@ -45,10 +63,24 @@ func (h *Handlers) targetRows( views := delivery.NewTargetViews(targets) rows := make([]TargetRowView, len(views)) + deliveries, err := h.loadTargetDeliveries(webhook.ID) + if err != nil { + h.log.Error( + "failed to read target delivery counts", + "webhook_id", webhook.ID, + "error", err, + ) + } + // NewTargetViews returns one view per target, in order. for i := range views { rows[i].TargetView = views[i] + if err == nil { + counts := deliveries[targets[i].ID] + rows[i].Deliveries = &counts + } + if targets[i].Type == database.TargetTypeDatabase { rows[i].Archive = h.archiveFileView(webhook, &targets[i]) } @@ -57,6 +89,64 @@ func (h *Handlers) targetRows( return rows } +// loadTargetDeliveries reads the delivery counts of a webhook's targets +// from its event database, keyed by target. A target with no deliveries +// is left out, and so is every target when the event database does not +// exist yet, since opening it would create it. +func (h *Handlers) loadTargetDeliveries( + webhookID string, +) (map[string]TargetDeliveries, error) { + if !h.dbMgr.DBExists(webhookID) { + return map[string]TargetDeliveries{}, nil + } + + webhookDB, err := h.dbMgr.GetDB(webhookID) + if err != nil { + return nil, err + } + + return readTargetDeliveries(webhookDB, time.Now()) +} + +// readTargetDeliveries counts each target's deliveries that became +// delivered and those that failed: in total from the targets' running +// totals, and in the 24 hours before now from the deliveries' status +// index. Each is one query for all the targets, and neither reads every +// stored delivery. +func readTargetDeliveries( + db *gorm.DB, now time.Time, +) (map[string]TargetDeliveries, error) { + var totals []database.TargetTotals + + err := db.Find(&totals).Error + if err != nil { + return nil, fmt.Errorf("reading target totals: %w", err) + } + + lastDay, err := finishedByTarget(db, now.Add(-longWindow)) + if err != nil { + return nil, err + } + + byTarget := make(map[string]TargetDeliveries, len(totals)) + + for _, total := range totals { + byTarget[total.TargetID] = TargetDeliveries{ + Delivered: total.Delivered, + Failed: total.Failed, + } + } + + for _, finished := range lastDay { + counts := byTarget[finished.TargetID] + counts.DeliveredLast24Hours = finished.Delivered + counts.FailedLast24Hours = finished.Failed + byTarget[finished.TargetID] = counts + } + + return byTarget, nil +} + // archiveFileView describes a database target's archive file from the // file's metadata alone; the archive is never opened. The file is found // by the name the archive writer uses, so it follows a rename of the diff --git a/internal/handlers/target_list_test.go b/internal/handlers/target_list_test.go index 75e46d6..89c60fe 100644 --- a/internal/handlers/target_list_test.go +++ b/internal/handlers/target_list_test.go @@ -3,6 +3,7 @@ package handlers_test import ( "os" "path/filepath" + "regexp" "strings" "testing" "time" @@ -12,6 +13,7 @@ import ( "sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/delivery" "sneak.berlin/go/webhooker/internal/handlers" + "sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/session" ) @@ -69,3 +71,104 @@ func TestHandleSourceDetail_ShowsArchiveFile(t *testing.T) { assert.Contains(t, body, "not created yet") assert.NotContains(t, body, "Archive Size:") } + +// targetList returns the text of the targets section in a rendered +// webhook page, from its heading to the next heading, with the markup +// taken out and each run of space made one space. Each target's row +// then reads as its name, type, state and buttons, followed by the +// lines below them. +func targetList(t *testing.T, page string) string { + t.Helper() + + _, list, found := strings.Cut(page, ">Targets") + require.True(t, found, "the page has no targets section") + + list, _, _ = strings.Cut(list, "]*>`).ReplaceAllString(list, " ") + + return strings.Join(strings.Fields(list), " ") +} + +// TestHandleSourceDetail_ShowsTargetDeliveries checks each target row's +// delivered and failed deliveries, in total and in the last 24 hours, +// for the history seedStatsHistory builds, before and after the real +// retention reaper removes the oldest event. The http target has one +// delivered, one of them in the last 24 hours, and three failed, one of +// them in the last 24 hours and one of them the oldest event's, which +// retention removes without changing the total. The active log target +// has two failed, both in the last 24 hours, and its pending and +// retrying deliveries count in neither. The four inactive log targets +// have none. +func TestHandleSourceDetail_ShowsTargetDeliveries(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + log *logger.Logger + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr, &log) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + hist := seedStatsHistory(t, h, sess, db, dbMgr) + + const ( + httpRow = "Delivered: 1 in total, 1 in the last 24 hours " + + "Failed: 3 in total, 1 in the last 24 hours" + activeLogRow = "t-log log Active Edit Deactivate Delete " + + "Delivered: 0 in total, 0 in the last 24 hours " + + "Failed: 2 in total, 2 in the last 24 hours" + inactiveLogRow = "t-log log Inactive Edit Activate Delete " + + "Delivered: 0 in total, 0 in the last 24 hours " + + "Failed: 0 in total, 0 in the last 24 hours" + ) + + list := targetList(t, renderSourceDetailPage(t, h, sess, hist.webhook.ID)) + assert.Equal(t, 1, strings.Count(list, httpRow)) + assert.Equal(t, 1, strings.Count(list, activeLogRow)) + assert.Equal(t, 4, strings.Count(list, inactiveLogRow)) + + statsPrune(t, db, dbMgr, log, hist.webhookDB) + + list = targetList(t, renderSourceDetailPage(t, h, sess, hist.webhook.ID)) + assert.Equal(t, 1, strings.Count(list, httpRow)) + assert.Equal(t, 1, strings.Count(list, activeLogRow)) + assert.Equal(t, 4, strings.Count(list, inactiveLogRow)) +} + +// TestHandleSourceDetail_TargetDeliveriesUnreadable checks that when the +// webhook's event database cannot be read, each target's row says so +// instead of showing zeros. +func TestHandleSourceDetail_TargetDeliveriesUnreadable(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + wh := seedWebhook(t, db) + seedTarget(t, db, wh.ID, database.TargetTypeLog) + + webhookDB, err := dbMgr.GetDB(wh.ID) + require.NoError(t, err) + require.NoError(t, + webhookDB.Migrator().DropTable(&database.TargetTotals{})) + + list := targetList(t, renderSourceDetailPage(t, h, sess, wh.ID)) + assert.Contains(t, list, "t-log log Active Edit Deactivate Delete "+ + "The delivery counts could not be read.") + assert.NotContains(t, list, "Delivered:") +} diff --git a/internal/server/alpine_browser_test.go b/internal/server/alpine_browser_test.go index f7974f5..0282113 100644 --- a/internal/server/alpine_browser_test.go +++ b/internal/server/alpine_browser_test.go @@ -47,7 +47,8 @@ const ( // TestAlpineRunsUnderTheSecurityPolicy loads the webhook page and the // event log in a headless browser, served by the real router and so // under the real Content-Security-Policy, and checks that the pages' -// Alpine.js directives and the copy control work. +// Alpine.js directives and the copy control work, and that a target's +// row shows its delivery counts. func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) { t.Parallel() @@ -118,6 +119,9 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) { } checkRefusedTarget(ctx, t, page) + checkTargetDeliveries(ctx, t, page, target.Name, + "0 in total, 0 in the last 24 hours", + "1 in total, 1 in the last 24 hours") checkCopy(ctx, t, page) checkEntrypointEdit(ctx, t, page, page+"/events") checkRecentEvents(ctx, t, page) @@ -452,6 +456,28 @@ func checkRefusedTarget(ctx context.Context, t *testing.T, url string) { assert.Empty(t, typed, "after Cancel, the next Add keeps the url entered") } +// checkTargetDeliveries loads a webhook page and checks that the row of +// the target named name shows delivered and failed beside its +// "Delivered:" and "Failed:" labels. +func checkTargetDeliveries( + ctx context.Context, t *testing.T, url, name, delivered, failed string, +) { + t.Helper() + + row := `//span[text()="` + name + `"]/ancestor::div[@class="p-4"][1]` + figure := func(label, value string) string { + return row + `//span[text()="` + label + + `"]/following-sibling::span[text()="` + value + `"]` + } + + require.NoError(t, chromedp.Run(ctx, loadPage(url))) + + assert.Truef(t, shown(ctx, figure("Delivered:", delivered)), + "the row of %s does not show %q delivered", name, delivered) + assert.Truef(t, shown(ctx, figure("Failed:", failed)), + "the row of %s does not show %q failed", name, failed) +} + // checkCopy loads a webhook page and checks that the Copy control beside // its entrypoint's URL is a button, and that clicking it copies the URL // and says so: the button reads "Copied" only once the copy succeeded. diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index a1077f0..da41aa2 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -11,6 +11,7 @@ import ( "strconv" "strings" "testing" + "time" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -439,7 +440,8 @@ func (e *testEnv) storedEntrypoint( } // seedFailedDelivery records a terminally failed delivery of an event -// to a target in the webhook's own database. +// to a target in the webhook's own database, as the delivery engine +// leaves one: finished now, and counted in its target's totals. func (e *testEnv) seedFailedDelivery( t *testing.T, webhookID, eventID, targetID string, @@ -449,16 +451,21 @@ func (e *testEnv) seedFailedDelivery( webhookDB, err := e.dbMgr.GetDB(webhookID) require.NoError(t, err) + finishedAt := time.Now() dlv := &database.Delivery{ - EventID: eventID, - TargetID: targetID, - Status: database.DeliveryStatusFailed, + EventID: eventID, + TargetID: targetID, + Status: database.DeliveryStatusFailed, + FinishedAt: &finishedAt, } require.NoError( t, webhookDB.Omit(clause.Associations).Create(dlv).Error, ) + require.NoError(t, database.AddTargetTotals(webhookDB, + database.TargetTotals{TargetID: targetID, Deliveries: 1, Failed: 1}, + )) return dlv } diff --git a/templates/source_detail.html b/templates/source_detail.html index 0531795..2aae5d7 100644 --- a/templates/source_detail.html +++ b/templates/source_detail.html @@ -271,6 +271,18 @@ {{end}} {{end}} + {{with .Deliveries}} +
+ Delivered: + {{.Delivered}} in total, {{.DeliveredLast24Hours}} in the last 24 hours +
+
+ Failed: + {{.Failed}} in total, {{.FailedLast24Hours}} in the last 24 hours +
+ {{else}} +
The delivery counts could not be read.
+ {{end}} {{else}}
No targets configured.