mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Merge candidate 20260908T050749Z-core-runtime
Change-source: pulse-maintainer
This commit is contained in:
@@ -154,6 +154,26 @@ verifies three requests and the two-second exponential wait before the final
|
||||
request. This is transport timing proof, not durable queue retry, installed
|
||||
provider acceptance or recipient receipt.
|
||||
|
||||
### Webhook transport retry exhaustion
|
||||
|
||||
The enhanced retry sender makes one initial HTTP attempt plus `RetryCount`
|
||||
retries; non-positive counts select `WebhookDefaultRetries`. Repeated valid
|
||||
zero-delay 429 hints do not reset that budget. When all attempts fail, the
|
||||
sender returns the total attempt count and records one failed webhook history
|
||||
entry with the final status and the number of retries, not one entry per HTTP
|
||||
attempt. Every attempt retains the event ID and payload. A queue delivery that
|
||||
uses this sender has a transport ceiling of effective retry count plus one;
|
||||
layering up to `MaxAttempts` queue deliveries multiplies that ceiling, but does
|
||||
not guarantee destination receipt. Permanent errors may stop retries earlier.
|
||||
|
||||
`TestSendWebhookWithRetry_RateLimitExhaustion` in
|
||||
`internal/notifications/webhook_enhanced_test.go` verifies configured counts
|
||||
1 and 2 and default selection for 0 and -1 against an owned HTTP fixture that
|
||||
always returns 429 with `Retry-After: 0`. It checks request identity and body,
|
||||
attempt exhaustion, and the single failed history record's status, retry
|
||||
count, payload size and error text. This is transport-budget and local history
|
||||
proof only, not queue persistence, provider receipt or installed qualification.
|
||||
|
||||
Notification-management HTTP production and its unit/contract proof now live
|
||||
together under `internal/api/alerting/`. Router-level scope and integration
|
||||
tests remain in `internal/api`, while the compatibility aliases there keep the
|
||||
|
||||
@@ -302,10 +302,11 @@ func (n *NotificationManager) shouldSendWebhook(webhook EnhancedWebhookConfig, a
|
||||
|
||||
// sendWebhookWithRetry implements exponential backoff retry with enhanced error tracking
|
||||
// Note: When used with the persistent queue, retry behavior is layered:
|
||||
// - Transport retries (this function): up to RetryCount attempts with exponential backoff
|
||||
// - Queue retries: up to MaxAttempts (default 3) with exponential backoff
|
||||
// Total attempts = RetryCount * MaxAttempts (e.g., 3 * 3 = 9 HTTP calls for a single notification)
|
||||
// This ensures delivery even during transient failures at either layer.
|
||||
// - Transport: one initial attempt plus RetryCount retries (non-positive counts use WebhookDefaultRetries)
|
||||
// - Queue: up to MaxAttempts delivery attempts with exponential backoff
|
||||
// The HTTP attempt ceiling is (effective RetryCount + 1) * MaxAttempts when
|
||||
// each queue delivery uses this transport. Permanent errors can stop earlier;
|
||||
// exhausting either retry budget does not guarantee delivery.
|
||||
func (n *NotificationManager) sendWebhookWithRetry(webhook EnhancedWebhookConfig, payload []byte, eventID string) error {
|
||||
maxRetries := webhook.RetryCount
|
||||
if maxRetries <= 0 {
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -292,6 +293,49 @@ func TestParseRetryAfterBackoff(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Even repeated valid zero-delay hints must not reset the transport retry budget.
|
||||
func TestSendWebhookWithRetry_RateLimitExhaustion(t *testing.T) {
|
||||
for _, retries := range []int{1, 2, 0, -1} {
|
||||
t.Run(fmt.Sprint(retries), func(t *testing.T) {
|
||||
nm := NewNotificationManager("http://pulse.local")
|
||||
t.Cleanup(nm.Stop)
|
||||
require.NoError(t, nm.UpdateAllowedPrivateCIDRs("127.0.0.1"))
|
||||
var attempts atomic.Int32
|
||||
payload := []byte(`{"test":true}`)
|
||||
server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
attempts.Add(1)
|
||||
assert.Equal(t, "limited:alert", r.Header.Get("X-Pulse-Event-ID"))
|
||||
body, err := io.ReadAll(r.Body)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, payload, body)
|
||||
w.Header().Set("Retry-After", "0")
|
||||
w.WriteHeader(http.StatusTooManyRequests)
|
||||
}))
|
||||
defer server.Close()
|
||||
webhook := EnhancedWebhookConfig{
|
||||
WebhookConfig: WebhookConfig{Name: "Limited webhook", URL: server.URL},
|
||||
RetryEnabled: true,
|
||||
RetryCount: retries,
|
||||
}
|
||||
wantRetries := retries
|
||||
if wantRetries <= 0 {
|
||||
wantRetries = WebhookDefaultRetries
|
||||
}
|
||||
err := nm.sendWebhookWithRetry(webhook, payload, "limited:alert")
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), fmt.Sprintf("after %d attempts", wantRetries+1))
|
||||
assert.Equal(t, int32(wantRetries+1), attempts.Load())
|
||||
history := nm.GetWebhookHistory()
|
||||
require.Len(t, history, 1, "one failed delivery, not one history entry per HTTP attempt")
|
||||
assert.False(t, history[0].Success)
|
||||
assert.Equal(t, http.StatusTooManyRequests, history[0].StatusCode)
|
||||
assert.Equal(t, wantRetries, history[0].RetryAttempts)
|
||||
assert.Equal(t, len(payload), history[0].PayloadSize)
|
||||
assert.Contains(t, history[0].ErrorMessage, "HTTP 429")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSendWebhookWithRetry_429RetryAfter(t *testing.T) {
|
||||
nm := NewNotificationManager("http://pulse.local")
|
||||
_ = nm.UpdateAllowedPrivateCIDRs("127.0.0.1")
|
||||
|
||||
Reference in New Issue
Block a user