Exponential backoff: interval_seconds=0 disables backoff entirely, and jitter pushes delays past MaxRetrySeconds

[krishna3554]

Summary

Two defects in ExponentialBackoffRetryStrategy.NextDuration (retrystrategies/exponentialBackoff.go):

1. intervalSeconds == 0 is never guarded, so a subscription configured with interval_seconds: 0 silently degenerates the "exponential backoff" into near-immediate retries (0–5 s) for every attempt, ignoring both the exponential curve and maxRetrySeconds. The constructor defensively defaults maxRetrySeconds == 0 to 7200 but does nothing for the interval — an asymmetry that suggests the guard was simply missed.
2. Jitter is added after the max cap, so actual delays can exceed the configured MaxRetrySeconds by up to ~5 seconds.

Location

• File: retrystrategies/exponentialBackoff.go
• Function: ExponentialBackoffRetryStrategy.NextDuration / NewExponential
• Relevant code:

func (r *ExponentialBackoffRetryStrategy) NextDuration(attempts uint64) time.Duration {
	retrySeconds := float64(r.intervalSeconds) * math.Pow(2, float64(attempts))
	if uint64(retrySeconds) > r.maxRetrySeconds {
		retrySeconds = float64(r.maxRetrySeconds)
	}

	d := time.Duration(retrySeconds) * time.Second

	jitter := time.Duration(rand.Uint64() % 10e9)

	d += jitter / 2

	return d
}

func NewExponential(intervalSeconds, maxRetrySeconds uint64) *ExponentialBackoffRetryStrategy {
	if maxRetrySeconds == 0 {
		maxRetrySeconds = 7200
	}
	...
}

Problem

Defect 1: With intervalSeconds = 0, retrySeconds computes to 0 for every attempt, never exceeds maxRetrySeconds, and the returned duration collapses to jitter/2 — a random value in [0s, 5s). A strategy explicitly named "exponential backoff" performs no backoff at all.

This input is reachable from user configuration: the dashboard/API subscription model accepts interval_seconds as a plain uint64 (api/models/subscription.go::RetryConfiguration, validated only as an integer via valid:"int~..."), and Transform() forwards it verbatim; NewRetryStrategyFromMetadata then passes it straight into NewExponential. The value 0 therefore flows through with no rejection and no defaulting, unlike maxRetrySeconds.

Defect 2: The cap comparison runs on retrySeconds before jitter is added:

if uint64(retrySeconds) > r.maxRetrySeconds {
    retrySeconds = float64(r.maxRetrySeconds)
}
d := time.Duration(retrySeconds) * time.Second
jitter := time.Duration(rand.Uint64() % 10e9)
d += jitter / 2

A delay pinned to maxRetrySeconds still receives up to half of 10 s of extra jitter, so the effective maximum is maxRetrySeconds + ~5s. If maxRetrySeconds is meant as an upper bound on retry delay (its name and the capping branch say so), the bound is violated by construction.

Trigger / Reproduction

Static-analysis finding (not executed).

• Defect 1: create/update a subscription with "interval_seconds": 0 and exponential strategy; observe retries fire at sub-5-second intervals regardless of attempt number.
• Defect 2: configure any large interval_seconds; once the computed delay reaches maxRetrySeconds, sampled delays still exceed it by up to 5 seconds.

Expected Behavior

• intervalSeconds == 0 should be rejected or defaulted (mirroring the existing maxRetrySeconds handling), so the strategy always produces a growing backoff.
• The final duration should respect maxRetrySeconds as a hard upper bound — e.g., apply jitter within the remaining headroom (min(max - base, jitter)) or clamp after adding it.

Actual Behavior

Zero intervals disable backoff entirely; capped delays overshoot the configured maximum by up to 5 seconds.

Impact

Webhook deliveries retry against customer endpoints far more aggressively than the operator's configuration promises (defect 1), or slightly beyond the declared ceiling (defect 2). For a delivery system whose endpoints rate-limit or charge per request, a single mistyped subscription turns "exponential backoff up to 2 hours" into effectively immediate hammering.

Suggested Direction

In NewExponential, default intervalSeconds == 0 to a sane base (or return an error so config validation can reject it earlier), and clamp the final duration: compute base (capped), then add min(jitter, maxRetry - base) when a ceiling exists. Unit tests covering intervalSeconds = 0, attempts past the cap boundary, and the delay ≤ maxRetrySeconds invariant would pin both fixes.

Evidence

• retrystrategies/exponentialBackoff.go: no zero-interval guard in NewExponential while maxRetrySeconds == 0 is defaulted; jitter added after the cap check.
• api/models/subscription.go::RetryConfiguration: IntervalSeconds uint64 accepted with only integer-format validation; Transform() copies it through unchanged.
• retrystrategies/retry.go::NewRetryStrategyFromMetadata: metadata values feed NewExponential directly.

[linear]

PDE-1027