Say max_retries is the total attempt count, not a retry count #320

Open
clawbot wants to merge 1 commits from issue-316-max-retries-total-attempts-copy into next
4 changed files with 91 additions and 11 deletions
Showing only changes of commit 433fdceee2 - Show all commits
+7 -7
View File
@@ -1507,7 +1507,7 @@ events should be forwarded.
| `type` | TargetType | One of: `http`, `slack`, `database`, `log` | | `type` | TargetType | One of: `http`, `slack`, `database`, `log` |
| `active` | boolean | Whether deliveries are enabled (default: true) | | `active` | boolean | Whether deliveries are enabled (default: true) |
| `config` | JSON text | Type-specific configuration | | `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 | | `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. **Relations:** Belongs to Webhook. Has many Deliveries.
@@ -1515,12 +1515,12 @@ events should be forwarded.
**Target types:** **Target types:**
- **`http`** — Forward the event as an HTTP POST to a configured URL. - **`http`** — Forward the event as an HTTP POST to a configured URL.
Behavior depends on `max_retries`: when `max_retries` is 0 (the `max_retries` is the total number of delivery attempts, not retries on
default), the target operates in fire-and-forget mode — a single top of the first: when `max_retries` is 0 (the default), the target
attempt with no retries and no circuit breaker. When `max_retries` is operates in fire-and-forget mode, a single attempt with no retries and
greater than 0, failed deliveries are retried with exponential backoff no circuit breaker; a value of N makes up to N attempts in all,
up to `max_retries` attempts, protected by a per-target circuit retrying failed deliveries with exponential backoff and protecting them
breaker. with a per-target circuit breaker.
- **`slack`** — Post the event as a formatted message to a - **`slack`** — Post the event as a formatted message to a
Slack-compatible incoming webhook URL (`webhookUrl` in `config`). It Slack-compatible incoming webhook URL (`webhookUrl` in `config`). It
is built on the same HTTP core as `http` and honours `max_retries` is built on the same HTTP core as `http` and honours `max_retries`
+77
View File
@@ -300,3 +300,80 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
"the page must render to completion, not abort partway", "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",
)
}
+6 -3
View File
@@ -120,9 +120,12 @@
<label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label> <label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
<input type="number" name="timeout" min="0" max="300" :disabled="targetType !== 'http'" class="input text-sm w-24"> <input type="number" name="timeout" min="0" max="300" :disabled="targetType !== 'http'" class="input text-sm w-24">
</div> </div>
<div x-show="targetType === 'http'" class="flex gap-2 items-center"> <div x-show="targetType === 'http'">
<label class="text-sm text-gray-700">Max retries (0 = fire-and-forget):</label> <div class="flex gap-2 items-center">
<input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24"> <label class="text-sm text-gray-700">Max retries:</label>
<input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
</div>
<p class="text-xs text-gray-500 mt-1">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.</p>
</div> </div>
<div x-show="targetType === 'slack'"> <div x-show="targetType === 'slack'">
<input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="targetType !== 'slack'" class="input text-sm"> <input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="targetType !== 'slack'" class="input text-sm">
+1 -1
View File
@@ -69,7 +69,7 @@
<div class="form-group"> <div class="form-group">
<label for="max_retries" class="label">Max retries</label> <label for="max_retries" class="label">Max retries</label>
<input type="number" id="max_retries" name="max_retries" value="{{.Target.MaxRetries}}" min="0" max="20" class="input"> <input type="number" id="max_retries" name="max_retries" value="{{.Target.MaxRetries}}" min="0" max="20" class="input">
<p class="text-xs text-gray-500 mt-1">0 is fire-and-forget: one attempt, no circuit breaker.</p> <p class="text-xs text-gray-500 mt-1">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.</p>
</div> </div>
{{end}} {{end}}