Skip to content

feat: RetryProfile API for switching retry regimes at runtime - #68

Merged
tanderson-ld merged 8 commits into
mainfrom
ta/SDK-2788/retry-conformance
Aug 12, 2026
Merged

feat: RetryProfile API for switching retry regimes at runtime#68
tanderson-ld merged 8 commits into
mainfrom
ta/SDK-2788/retry-conformance

Conversation

@tanderson-ld

@tanderson-ld tanderson-ld commented Aug 5, 2026

Copy link
Copy Markdown

Summary

Adds a RetryProfile API so a caller can register multiple retry-timing profiles on a single stream and switch between them at runtime via Stream.ActivateProfile. Enables consumers to run a multi-regime retry policy (e.g., a normal regime + an extended regime for auth failures) while keeping the library's single-regime timing path intact for existing callers.

API additions

  • NewRetryProfile(options ...RetryProfileOption) *RetryProfile — construct an opaque handle.
  • RetryProfileBaseDelay(d), RetryProfileMaxDelay(d), RetryProfileJitter(r) — profile options.
  • StreamOptionDefaultRetryProfile(profile) — designate as the stream's effective default.
  • StreamOptionRegisterRetryProfile(profile) — register as an additional switchable profile.
  • Stream.ActivateProfile(profile *RetryProfile) — switch the currently-active profile at runtime.
  • StreamErrorHandlerResult.ActivateProfile — activate a profile from an error handler result (applied before the impending reconnect).
  • DefaultProfile — package-level sentinel meaning "revert to the effective default."
  • MaxServerDirectedRetryDelay = time.Hour — clamp ceiling for the SSE retry: field.

Legacy stream options (StreamOptionInitialRetry, StreamOptionUseBackoff, StreamOptionUseJitter, StreamOptionRetryResetInterval) continue to work unchanged; when no explicit RetryProfile is provided they synthesize the effective default.

Semantics

  • Lazy overlay resolution. At delay-computation time, each property is resolved by walking active-profile.speceffective-default.spec → hard-coded fallback. Profile specs are immutable.
  • Per-profile formula counter n. Each registered profile tracks its own backoff-formula counter. Progression is retained across activations, so rapid oscillation between regimes preserves each regime's state.
  • Healthy-op reset. When elapsed >= resetInterval, zeros every profile's n and reverts active to the effective default. Does NOT clear server-directed base-delay overrides (matches HTML5 SSE spec's "reconnection time is set until updated").
  • Server retry: field. Updates every registered profile's base-delay override (stream-wide per HTML5). Never touches any profile's declared maxDelay ceiling.
  • Wire clamp. SSE retry: values above MaxServerDirectedRetryDelay are clamped, in milliseconds before the time.Millisecond multiplication, so extreme int64 wire values cannot overflow the Duration.

Internal notes for reviewers

  • backoffStrategy.applyBackoff and jitterStrategy.applyJitter interfaces were widened to accept per-call maxDelay / ratio. Math bodies are unchanged from the pre-existing library; only the parameter source moved from receiver fields to method args, so one strategy instance can serve multiple profiles.
  • Internal SetBaseDelay renamed to ApplyRetryTime; it now iterates all registered profiles.

Test plan

  • Full test suite passes (go test ./...).
  • SSE contract-test harness passes (make contract-tests — "All tests passed").
  • Pre-existing backoff/jitter math bodies preserved byte-for-byte in effect (only parameter sourcing changed).
  • Library-maintainer review.

Refs SDK-2788.


Note

Overview
Introduces a RetryCurve model so one SSE stream can register multiple retry-timing profiles and switch the active profile at runtime via Stream.ActivateCurve, StreamErrorHandlerResult.ActivateCurve, and new options StreamOptionDefaultRetryCurve / StreamOptionRegisterRetryCurve. Each curve keeps its own backoff counter; healthy-period reset still reverts to the effective default (and overrides a same-tick activation), while server retry: hints apply stream-wide with a 1h clamp via MaxServerDirectedRetryDelay.

retryDelayStrategy is refactored from a single delay config to a map of curves with lazy property overlay (curve → default → fallbacks). Legacy StreamOptionInitialRetry / backoff / jitter options still synthesize the default when no explicit curve is set; retry option fields are now pointers so unset vs explicit zero (e.g. immediate retry) is preserved. SetBaseDelay becomes ApplyRetryTime; jitter gets a guard so zero-delay retries do not panic.

Reviewed by Cursor Bugbot for commit c78cbd3. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread stream.go
pub := ev.(*publication)
if pub.Retry() > 0 {
stream.retryDelay.SetBaseDelay(time.Duration(pub.Retry()) * time.Millisecond)
stream.retryDelay.ApplyRetryTime(clampServerDirectedRetry(pub.Retry()))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: renamed, existing name was misleading, even on main this did more than set base delay.

Comment thread server.go

var delayedEvent eventOrComment
jitterStrategy := newDefaultJitter(0.5, 0)
jitterStrategy := newDefaultJitter(0)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: Jitter is now passed as a param to the strategy at jitter application time.

Comment thread retry_delay.go
type backoffStrategy interface {
applyBackoff(baseDelay time.Duration, retryCount int) time.Duration
applyBackoff(baseDelay time.Duration, retryCount int, maxDelay time.Duration) time.Duration
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: backoffStrategy and jitterStrategy are internal only interfaces. The max and jitter are now properties of the retry curve and not fixed in the strategy.

Comment thread retry_delay.go
}

type defaultJitterStrategy struct {
ratio float64

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: jitter ratio moved to the retry curve to support cases of different jitters in different sitatuions.

Comment thread retry_delay.go Outdated
// streamOptions.
func newRetryDelayStrategyFromOptions(opts *streamOptions, randSeed int64) *retryDelayStrategy {
// Resolve the effective default curve.
effectiveDefault := opts.defaultRetryCurve

@tanderson-ld tanderson-ld Aug 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: if a default curve was not provided, we will make a default curve from the old stream options in order to not be a breaking change.

Comment thread retry_delay.go
func (r *retryDelayStrategy) SetBaseDelay(baseDelay time.Duration) {
// Does NOT reset the newly-activated curve's retryCount — each curve's counter
// retains its progression across activations. Does NOT touch any curve's
// baseDelayOverride.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: the baseDelayOverride is set via the server directed retry: event.

@tanderson-ld
tanderson-ld marked this pull request as ready for review August 6, 2026 13:58
@tanderson-ld
tanderson-ld requested a review from a team as a code owner August 6, 2026 13:58
Introduces a RetryCurve opaque handle. Callers construct curves via
NewRetryCurve(options...), designate them at subscribe time via
StreamOptionDefaultRetryCurve / StreamOptionRegisterRetryCurve, and switch
between them at runtime via Stream.ActivateCurve. Enables SDKs to run a
multi-regime retry policy (e.g., a normal regime + an extended regime for
auth failures) while keeping the library's single-regime timing path intact
for legacy callers.

Overlay resolution walks (active-curve spec -> effective-default spec ->
hard-coded fallbacks), evaluated lazily at delay-computation time. Per-curve
formula counter n is retained across activations. Healthy-operation reset
zeros all curves' formula counters and reverts to the effective default; it
does not clear base-delay overrides (matches SSE spec's "reconnection time
is set until updated").

SSE `retry:` field is honored per HTML5 semantics: the stream read loop
updates every registered curve's base-delay override. Values above 1 hour
are clamped per RETRY spec section 1.11.4 (new MaxServerDirectedRetryDelay
constant). Clamping happens in milliseconds before the multiplication by
time.Millisecond so extreme wire values cannot overflow the Duration.

Internal changes:
- Widened backoffStrategy.applyBackoff and jitterStrategy.applyJitter to
  accept per-call maxDelay / ratio so a single strategy instance can serve
  multiple curves. Math bodies unchanged from the pre-existing library.
- Renamed internal SetBaseDelay to ApplyRetryTime; it now iterates all
  registered curves.

Legacy stream options (StreamOptionInitialRetry / UseBackoff / UseJitter /
RetryResetInterval) continue to work unchanged; when no explicit
RetryCurve is provided they synthesize the effective default.

Refs SDK-2788.
- Rename `max` parameter on RetryCurveMaxDelay to `maxDelay` (revive
  redefines-builtin-id: `max` shadows the Go 1.21 built-in).
- Add `//nolint:unused // used only in tests` to activeCurve, matching
  the existing convention on hasJitter.
- Wrap the applyBackoff signature and the three firstNonNil calls in
  resolveCurveProperties across multiple lines (lll: 120-char limit).

No logic changes. `make lint` and `go test ./...` both clean locally.
Adds an ActivateCurve field to StreamErrorHandlerResult. When an
errorHandler returns a non-nil ActivateCurve, the Stream activates
that curve before computing the next retry delay.

This is the ergonomic path for callers that want to switch retry
regimes on the same failure that triggers the switch (e.g., swap to
an extended-regime curve when a 401 is seen). Without it, callers
would have to hold a reference to the Stream and call
Stream.ActivateCurve out of band -- awkward during initial-connect
retries where the Stream is not yet returned to the caller.

If a healthy-operation reset fires on the same NextRetryDelay call,
the reset trumps the activation, matching the semantics of a manual
Stream.ActivateCurve call in the same window.
@tanderson-ld
tanderson-ld force-pushed the ta/SDK-2788/retry-conformance branch from ac1294c to 295ff5f Compare August 7, 2026 20:52

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 295ff5f. Configure here.

Comment thread retry_delay.go
Comment thread retry_delay.go
Comment thread retry_delay.go
@tanderson-ld tanderson-ld changed the title feat: named RetryCurve API for switching retry regimes at runtime feat: RetryCurve API for switching retry regimes at runtime Aug 10, 2026
Refactors the four legacy retry-timing fields (initialRetry, backoffMaxDelay,
jitterRatio, retryResetInterval) on streamOptions from plain values to pointer
types. The synth path in newRetryDelayStrategyFromOptions now propagates any
non-nil pointer including zero values; nil means the caller never touched the
option, and delay resolution falls through to the hard-coded fallback.

Restores pre-refactor parity for two documented but broken behaviors:
- StreamOptionInitialRetry(0) once again yields immediate retry (was silently
  falling back to DefaultInitialRetry after the RetryCurve refactor).
- StreamOptionRetryResetInterval(0) once again disables the healthy-op reset
  (was silently promoted to DefaultRetryResetInterval).

backoffMaxDelay and jitterRatio are pointerized for uniformity across the four
legacy timing knobs even though their zero semantics are unchanged (0 still
means "feature disabled"). Downstream consumers in NextRetryDelay continue to
gate on effectiveMax > 0 and effectiveJitter > 0, so nil and &0 collapse to
the same behavior for those two -- the pointerization only buys structural
consistency, not new expressiveness.

Test helpers updated: mkRetryDelay and mkRetryDelayWithCurves address the
timing arguments so callers of the helper still express the same intent via
value arguments. Tests wanting the "never set / library default" fallback now
construct streamOptions directly and leave the field nil.
rand.Int63n panics when its argument is <= 0. applyJitter reaches that path
when computedDelay is zero (regardless of ratio) or when a nonzero delay
multiplied by a small ratio truncates the int64 product to zero.

Pre-existing bug, but the preceding parity fix on legacy timing options makes
StreamOptionInitialRetry(0) actually reachable, which in turn makes the
zero-delay path (and this panic) reachable via a documented combination.
Guard on the computed span: a zero window means there is no jitter to
subtract, so return the delay unchanged.
Adds tests requested during multi-agent review of this PR. No behavior
change; docstring corrections and new tests only.

Docstring corrections:
- retryDelayStrategy.activateCurve: replace imprecise "silent no-op if
  already active" -- the write is unconditional but idempotent.
- mkRetryDelayWithCurves helper: stale comment claimed resetInterval=0
  falls back to DefaultRetryResetInterval, but after the pointer refactor
  it explicitly disables the healthy-op reset. Correct the comment.

New tests (retry_curve_test.go):
- TestRetryCurveMaxDelayZeroOverridesPositiveDefault pins that an explicit
  RetryCurveMaxDelay(0) on an ext curve disables backoff even when the
  effective default has a positive ceiling. Analogous to the existing
  TestRetryCurveBaseDelayZeroIsExplicitNotUnset.
- TestRetryCurveJitterZeroOverridesPositiveDefault pins the same shape for
  jitter.
- TestApplyRetryTimeUpdatesAllCurves extended with second- and third-
  attempt assertions on both the default and extended curves, pinning
  that a server-directed retry hint replaces the base delay but backoff
  continues against the hinted base with the counter advancing.

New tests (stream_reconnect_test.go):
- TestStreamErrorHandlerActivateCurveWiredOnInitialConnect covers the
  advertised StreamErrorHandlerResult.ActivateCurve field via the initial-
  connect retry loop in SubscribeWithRequestAndOptions.
- TestStreamErrorHandlerActivateCurveWiredOnExistingConnection covers the
  same field via the worker-goroutine error path in Stream.stream.
- TestStreamActivateCurvePublicMethodReachesStrategy covers the public
  Stream.ActivateCurve method and the DefaultCurve sentinel revert path.

Multi-agent review consensus: no correctness blockers; these tests close
coverage gaps around the advertised runtime-activation API surface.
Comment thread retry_curve.go Outdated
Comment thread interface.go Outdated
… wording

Per @kinyoklion's review on #68.

Rename (RetryProfile matches "stepped, jittered, shifted-and-windowed
function" more precisely than "curve"):

- Public API: RetryCurve, NewRetryCurve, RetryCurveOption,
  RetryCurveBaseDelay / MaxDelay / Jitter, DefaultCurve,
  Stream.ActivateCurve, StreamOptionDefaultRetryCurve /
  RegisterRetryCurve, and StreamErrorHandlerResult.ActivateCurve are all
  renamed to their Profile counterparts.
- Files retry_curve.go / retry_curve_test.go moved to retry_profile.go /
  retry_profile_test.go via git mv (rename detection needs -M20% given the
  content churn).
- Internal identifiers, comments, and test names updated in parallel;
  "curve" no longer appears anywhere in the codebase.

Docstring wording:
- "HTML5 semantics" replaced with "WHATWG HTML Living Standard's
  EventSource reconnection-time semantics".
- "stream-level" / "stream-wide" replaced with "applies per eventsource
  instance" in the two docstrings covering reset-interval scope and the
  scope of a server-directed retry: hint.

Unit tests green; sse-contract-tests harness passes end-to-end.
tanderson-ld added a commit to launchdarkly/go-server-sdk that referenced this pull request Aug 11, 2026
The eventsource library renamed RetryCurve to RetryProfile in response
to review feedback on launchdarkly/eventsource#68. Update the streaming
data-source wire-up to consume the new API.

- es.NewRetryCurve / RetryCurveBaseDelay / MaxDelay / Jitter → es.NewRetryProfile / RetryProfileBaseDelay / …
- result.ActivateCurve → result.ActivateProfile
- es.StreamOptionDefaultRetryCurve / RegisterRetryCurve → es.StreamOptionDefaultRetryProfile / RegisterRetryProfile
- Local vars defaultCurve / extendedCurve → defaultProfile / extendedProfile
- Comment references to "retry curve" / "extended-regime curve" → "profile"

go.mod is intentionally left pinned at eventsource v1.10.0. CI will be
red on this PR until eventsource releases the renamed API and go.mod is
bumped, matching the sequencing the epic assumes.
@kinyoklion kinyoklion changed the title feat: RetryCurve API for switching retry regimes at runtime feat: RetryProfile API for switching retry regimes at runtime Aug 11, 2026
@tanderson-ld
tanderson-ld merged commit c430aec into main Aug 12, 2026
9 checks passed
@tanderson-ld
tanderson-ld deleted the ta/SDK-2788/retry-conformance branch August 12, 2026 17:23
tanderson-ld added a commit that referenced this pull request Aug 12, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.13.0](v1.12.0...v1.13.0)
(2026-08-12)


### Features

* activate retry curves from error handler result
([295ff5f](295ff5f))
* add named RetryCurve API for switching retry regimes at runtime
([0ef5312](0ef5312))
* RetryProfile API for switching retry regimes at runtime
([#68](#68))
([c430aec](c430aec))


### Bug Fixes

* distinguish unset from explicit-zero on legacy timing options
([ce80a7b](ce80a7b))
* guard defaultJitterStrategy.applyJitter against zero span
([12f82f7](12f82f7))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> **Release Please** bumps the package version from **1.12.0** to
**1.13.0** in `.release-please-manifest.json` and adds the **1.13.0**
section to `CHANGELOG.md`.
> 
> That release entry documents already-merged work: **RetryProfile** /
named **RetryCurve** APIs to switch retry regimes at runtime, applying
retry curves from **StreamErrorHandler** results, fixes for unset vs
explicit-zero legacy timing options, and a guard in
**defaultJitterStrategy** when jitter span is zero.
> 
> This PR does not change library source—only release metadata and
changelog.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
1dabcc1. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
tanderson-ld added a commit to launchdarkly/go-server-sdk that referenced this pull request Aug 12, 2026
Consumes the RetryProfile API introduced in launchdarkly/eventsource#68 and
released as v1.13.0. This is the final piece of the SDK-2788 chain; CI on
this PR should now go green.

- go.mod: launchdarkly/eventsource v1.10.0 -> v1.13.0
- go.sum updated accordingly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants