From 433fdceee2f167eb19fc6df3e2505576ff962295 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Sep 2026 07:55:07 +0000 Subject: [PATCH] Say max_retries is the total attempt count, not a retry count (closes #316) The delivery core makes max_retries attempts in total: a fresh delivery starts at attempt one and gives up once the attempt number reaches max_retries, so 3 is three attempts, not four, and 0 is special-cased to a single fire-and-forget attempt with no retries and no circuit breaker. The create and edit target forms called it "Max retries" with no total, and the README data-model row called it "maximum retry attempts", so an operator wanting "try, then retry twice" would enter the wrong number. Both forms and the README rows now state the number is the total number of delivery attempts, with the 0 case spelled out. The delivery arithmetic is unchanged. A UI copy test renders both forms and pins the shared wording so it cannot drift back to a retry count. Model: opus-4-8 --- README.md | 14 +++--- internal/handlers/ui_copy_test.go | 77 +++++++++++++++++++++++++++++++ templates/source_detail.html | 9 ++-- templates/target_edit.html | 2 +- 4 files changed, 91 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 319989c..e1289af 100644 --- a/README.md +++ b/README.md @@ -1507,7 +1507,7 @@ events should be forwarded. | `type` | TargetType | One of: `http`, `slack`, `database`, `log` | | `active` | boolean | Whether deliveries are enabled (default: true) | | `config` | JSON text | Type-specific configuration | -| `max_retries` | integer | Maximum retry attempts for `http` and `slack` targets (0 = fire-and-forget, >0 = retries with backoff and a circuit breaker). Ignored by `database` and `log` targets | +| `max_retries` | integer | Total delivery attempts for `http` and `slack` targets, not retries on top of the first: 0 is a single fire-and-forget attempt with no retries and no circuit breaker, and a value of N makes N attempts in all, with exponential backoff and a per-target circuit breaker. Ignored by `database` and `log` targets | | `max_queue_size` | integer | Stored and shown on the target's detail view, but not enforced anywhere yet: nothing in the delivery engine consults it. Queue depth is set by the two fixed 10,000-entry channels | **Relations:** Belongs to Webhook. Has many Deliveries. @@ -1515,12 +1515,12 @@ events should be forwarded. **Target types:** - **`http`** — Forward the event as an HTTP POST to a configured URL. - Behavior depends on `max_retries`: when `max_retries` is 0 (the - default), the target operates in fire-and-forget mode — a single - attempt with no retries and no circuit breaker. When `max_retries` is - greater than 0, failed deliveries are retried with exponential backoff - up to `max_retries` attempts, protected by a per-target circuit - breaker. + `max_retries` is the total number of delivery attempts, not retries on + top of the first: when `max_retries` is 0 (the default), the target + operates in fire-and-forget mode, a single attempt with no retries and + no circuit breaker; a value of N makes up to N attempts in all, + retrying failed deliveries with exponential backoff and protecting them + with a per-target circuit breaker. - **`slack`** — Post the event as a formatted message to a Slack-compatible incoming webhook URL (`webhookUrl` in `config`). It is built on the same HTTP core as `http` and honours `max_retries` diff --git a/internal/handlers/ui_copy_test.go b/internal/handlers/ui_copy_test.go index da9b6d6..8598de9 100644 --- a/internal/handlers/ui_copy_test.go +++ b/internal/handlers/ui_copy_test.go @@ -300,3 +300,80 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) { "the page must render to completion, not abort partway", ) } + +// maxRetriesHelp is the wording both target forms must carry. The +// delivery core makes max_retries attempts in total, not that many +// retries on top of a first try (a fresh delivery starts at attempt 1 +// and target_http gives up once the attempt number reaches +// max_retries), and 0 is special-cased to a single fire-and-forget +// attempt with no circuit breaker. +const maxRetriesHelp = "This is the total number of delivery attempts, " + + "not retries on top of the first: a value of 3 makes three attempts " + + "in all. 0 means a single attempt with no retries and no circuit " + + "breaker." + +// TestTargetFormMaxRetriesCopyMatchesBehaviour pins the max_retries +// help text on both the create form (the add-target form on the webhook +// detail page) and the edit form, so the copy cannot drift back to +// calling the number a retry count. +func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) { + t.Parallel() + + var h *handlers.Handlers + + var sess *session.Session + + app := newTestApp(t, &h, &sess) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + webhook := &database.Webhook{Name: "wh", RetentionDays: 14} + webhook.ID = testWebhookID + + entrypoint := database.Entrypoint{Path: "abc123"} + entrypoint.ID = "ep-1" + + createBody := renderPage( + t, h, sess, "source_detail.html", map[string]any{ + dataKeyWebhook: webhook, + "Entrypoints": handlers.NewEntrypointViews( + []database.Entrypoint{entrypoint}, + ), + "Targets": delivery.NewTargetViews(nil), + "Events": []database.Event{}, + "BaseURL": "https://hooks.example.com", + }, + ) + + assert.Contains( + t, createBody, maxRetriesHelp, + "the add-target form must explain max_retries as total attempts", + ) + + // A slack target exercises the same max_retries field while needing + // only Config.URL from the edit template, so the test data stays + // minimal. The Target key mirrors the field names the template reads + // off the handler's view value. + editBody := renderPage( + t, h, sess, "target_edit.html", map[string]any{ + dataKeyWebhook: webhook, + "Target": map[string]any{ + "ID": "tg-1", + "Name": "t", + "Type": "slack", + "Active": true, + "MaxRetries": 3, + "Config": map[string]any{ + "URL": "https://hooks.slack.com/services/x", + }, + }, + dataKeyError: "", + }, + ) + + assert.Contains( + t, editBody, maxRetriesHelp, + "the target edit form must explain max_retries as total attempts", + ) +} diff --git a/templates/source_detail.html b/templates/source_detail.html index bde8cae..3b26967 100644 --- a/templates/source_detail.html +++ b/templates/source_detail.html @@ -120,9 +120,12 @@ -
- - +
+
+ + +
+

This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.

diff --git a/templates/target_edit.html b/templates/target_edit.html index 9194721..a2ff56c 100644 --- a/templates/target_edit.html +++ b/templates/target_edit.html @@ -69,7 +69,7 @@
-

0 is fire-and-forget: one attempt, no circuit breaker.

+

This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.

{{end}}