Skip to content

feat: expose narrow setters on EventSource for RETRY-spec regime switching - #109

Draft
tanderson-ld wants to merge 2 commits into
mainfrom
ta/SDK-2789/retry-conformance
Draft

feat: expose narrow setters on EventSource for RETRY-spec regime switching#109
tanderson-ld wants to merge 2 commits into
mainfrom
ta/SDK-2789/retry-conformance

Conversation

@tanderson-ld

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

Copy link
Copy Markdown
Contributor

Summary

Adds two narrow public setter methods on EventSourcesetInitialRetryDelayMillis(long) and setMaxRetryDelayMillis(long) — for SDK-driven regime switching. The motivating use case is RETRY-spec conformance in server SDKs: on classification of a failure as "unexpected" (per RETRY §1.6 / §1.7), the SDK's data source shifts retry timing into an extended regime (e.g., initial 5 min, max 1 hr), and shifts back after healthy operation.

  • Tracked as SDK-2789 under the RETRY-conformance epic SDK-2775.
  • Cross-referenced by server-sdk-guide.md — the cross-cutting implementation guide feeding forward from the Go reference implementation (SDK-2788).
  • Implementation plan at SDK-2789 plan.md (§4.1 covers the API-shape decision, §4.5 covers the cross-repo sequencing).

What ships

Two public methods on EventSource:

  • setInitialRetryDelayMillis(long millis) — updates the existing volatile baseRetryDelayMillis field (same field the wire-side SetRetryDelayEvent already updates); if the current strategy is a DefaultRetryDelayStrategy, the exponent counter is reset so the first subsequent apply() uses the new base directly.
  • setMaxRetryDelayMillis(long millis) — constructs a new immutable DefaultRetryDelayStrategy with the specified max delay and the exponent counter reset to 0, preserving the current backoff multiplier and jitter multiplier, and atomically swaps the reference. No-op for custom strategy impls (which don't expose a max-delay concept via the abstract interface).

Both realize the "reset n when delays change" invariant from the LaunchDarkly server-SDK implementation guide: the first attempt in a new regime uses the new initial delay directly rather than newBase × 2^oldN.

Two package-private helpers on DefaultRetryDelayStrategy: withResetCounter() and withMaxDelayMillisAndResetCounter(long). Existing public builder methods (maxDelay, backoffMultiplier, jitterMultiplier) are unchanged.

currentRetryDelayStrategy field made volatile to support the "caller can invoke setters from any thread" contract.

Design notes

Alternative considered: expose a single setRetryDelayStrategy(RetryDelayStrategy) method that lets the caller replace the whole strategy. Rejected because it gives consumers too much rope — an SDK doing regime switching only needs to move min/max between regimes; it shouldn't be able to accidentally change jitter or backoff-multiplier as part of the same knob. The narrow-setter API keeps the public surface minimal.

The EventSource.baseRetryDelayMillis handling of server-directed retry: hints is orthogonal to the new setters and remains unchanged. A later wire hint continues to overwrite the SDK-set base delay, matching WHATWG HTML Living Standard EventSource semantics.

Testing

  • 6 new unit tests in EventSourceRetryDelayStrategyUsageTest:
    • setInitialRetryDelayMillisUpdatesGetBaseRetryDelayMillis
    • setInitialRetryDelayMillisResetsExponentCounter
    • setMaxRetryDelayMillisClampsAndResetsExponentCounter
    • setInitialAndSetMaxComposeForExtendedRegimeSequence (asserts the RETRY spec's extended-regime doubling shape at ms-scale for test speed)
    • wireRetryHintStillTakesEffectAfterSdkSideSetters
    • settersOnCustomRetryDelayStrategyDoNotThrow
  • Full unit suite: BUILD SUCCESSFUL, 0 failures.
  • make contract-tests (sse-contract-tests harness): "All tests passed" end-to-end.

Test plan for reviewers

  • Confirm the "narrow setter" API shape is preferred over a whole-strategy setter.
  • Confirm currentRetryDelayStrategy being volatile is acceptable (the field was previously non-volatile with a comment noting it should only be accessed from the reading thread; the setters lift that invariant, and the volatile write is a defensive publication guarantee).
  • Confirm the two new package-private helpers on DefaultRetryDelayStrategy are appropriately scoped.
  • Sanity-check the wire-hint interaction test (wireRetryHintStillTakesEffectAfterSdkSideSetters) — the wire hint continues to override the SDK's initial delay after both setters have been used, which is intentional (SDK regime state ≠ wire-authoritative reconnect time).

Downstream

Consumed by java-server-sdk PR (SDK-2789) — Java SDK's RETRY-conformance work. That PR's CI will be red until this PR ships to Maven Central.

… EventSource

Adds two narrow public setter methods on EventSource for SDK-driven regime
switching. Motivating use case is RETRY-spec conformance in server SDKs:
on classification of a failure as "unexpected" (per RETRY §1.6 / §1.7), the
SDK's data source needs to shift the retry timing into an extended regime
(e.g. initial 5 min, max 1 hr), and shift back after healthy operation.
Cross-referenced in launchdarkly/sdk-scratchpad's server-sdk-guide.md, and
tracked as SDK-2789 (Java) under the RETRY-conformance epic SDK-2775.

API:
- setInitialRetryDelayMillis(long) updates the existing volatile
  baseRetryDelayMillis field (the same field wire-side SetRetryDelayEvent
  already updates). If the current strategy is a DefaultRetryDelayStrategy,
  the exponent counter is also reset so the first subsequent apply() uses
  the new base directly. Non-Default strategies just see the new base on
  the next apply() call.
- setMaxRetryDelayMillis(long) constructs a new DefaultRetryDelayStrategy
  with the specified max delay and the exponent counter reset to 0,
  preserving the current backoff multiplier and jitter multiplier, and
  atomically swaps the reference. No-op for custom strategy impls (which
  don't expose a max-delay concept via the abstract interface).

Both setters realize the "reset n when delays change" invariant from the
LaunchDarkly server-SDK implementation guide: the first attempt in a new
regime uses the new initial delay directly rather than newBase * 2^oldN.

Under the hood: DefaultRetryDelayStrategy gains two package-private
helpers (withResetCounter and withMaxDelayMillisAndResetCounter) that build
copies with a fresh counter. The public builder methods (maxDelay,
backoffMultiplier, jitterMultiplier) are unchanged. currentRetryDelayStrategy
is now volatile to support the "caller can invoke setters from any thread"
contract.

Tests: 6 new tests covering direct setter effects, counter-reset behavior,
composed extended-regime sequence, wire-hint interaction, and no-op
behavior on non-Default strategies. Full unit suite and sse-contract-tests
both green.
An SDK error handler running under ErrorStrategy.alwaysContinue calls
setInitialRetryDelayMillis / setMaxRetryDelayMillis inside its handleError
callback, in response to a fault that has just been classified as
UNEXPECTED per the RETRY specification. The immediately-prior
computeReconnectDelay() call had already stored nextReconnectDelayMillis
using the pre-transition strategy, so without a recompute the upcoming
reconnect would use the OLD regime's timing (e.g., 1 ms normal-regime
delay) and the extended-regime backoff would kick in only starting from
the NEXT fault.

Fix:
- Track a pendingReconnectWait flag: set by computeReconnectDelay after a
  fault, cleared by tryStart on successful reconnect.
- setInitialRetryDelayMillis and setMaxRetryDelayMillis, if
  pendingReconnectWait is true, recompute nextReconnectDelayMillis with
  the just-updated strategy. Do NOT advance the strategy (prior
  computeReconnectDelay already did that; advancing here would
  double-increment the counter for the next fault).

Verified via the sdk-test-harness RETRY-conformance streaming/retry test
"enters extended-regime backoff after unexpected HTTP error", which is
now green (was failing prior to this fix because the SDK reconnected at
normal-regime timing after the first 401).
@tanderson-ld tanderson-ld changed the title feat: expose narrow setters on EventSource for RETRY-spec regime switching (SDK-2789) feat: expose narrow setters on EventSource for RETRY-spec regime switching Aug 12, 2026
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.

1 participant