feat!: multi-strategy retry delay API for regime switching - #110
Merged
Conversation
Introduces a multi-strategy retry-delay API on EventSource so SDKs adopting the LaunchDarkly RETRY specification can register normal- and extended-regime strategies at build time and activate between them at runtime. Server-directed retry: hints from the SSE wire remain sticky across activations, matching WHATWG "reconnection time is set until updated" semantics. RetryDelayStrategy is now a snapshot-oriented immutable value: each instance exposes getDelayMillis() for the current retry's delay and getNext() for the successor instance. The single previous method apply(long) is removed along with the Result wrapper class; the base delay is no longer an out-of-band parameter but lives on the strategy itself and is updated via the new withBaseDelayMillis(long) method (default no-op for custom strategies without a base concept). DefaultRetryDelayStrategy gains initialDelay(long, TimeUnit) so each strategy can carry its own initial delay, letting normal- and extended-regime strategies coexist with different starting points. EventSource.activateRetryDelayStrategy(RetryDelayStrategy) swaps the active strategy at runtime; each registered strategy retains its own backoff progression state across activations. Passing null or an unregistered strategy is a silent no-op. Builder.retryDelayStrategy has additive semantics: the first call sets the default (initially active and reset target); subsequent calls register additional strategies. The reconnect-delay compute is deferred to sleep time so activation and wire-hint changes received during the fault window take effect on the impending reconnect, not the one after. BREAKING CHANGE: RetryDelayStrategy.apply(long) is replaced by getDelayMillis() + getNext() + withBaseDelayMillis(long). The Result class is removed. Custom RetryDelayStrategy implementations must migrate to the new abstract shape. EventSource no longer exposes getBaseRetryDelayMillis() or getNextRetryDelayMillis(); observability is via the "Waiting X milliseconds before reconnecting" log message.
4 tasks
tanderson-ld
added a commit
to launchdarkly/java-core
that referenced
this pull request
Aug 21, 2026
…polling data sources (SDK-2789) Guided by the server-sdk-guide.md in sdk-scratchpad; analogous to the Go server SDK's reference implementation. The behavioral change: HTTP responses that today cause a data source to permanently stop (notably 401, 403, other 4xx) and TLS/certificate validation failures are no longer terminal. Streaming enters an extended backoff regime (5 min -> 1 hour, doubling); polling continues at its configured cadence with extended-regime waits between failing polls. Recovery from either regime uses a healthy-operation reset (60 s of continuous connectivity for streaming; two consecutive successful polls for polling). Scope: FDv1 streaming and polling data sources under `lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/`. FDv2 is out of scope for this epic and is deferred to a future one; nothing in `datasourcev2/` or the DataSystem-related code paths is touched. Highlights: - FailureClass enum + classifier helpers in launchdarkly-java-sdk-internal's HttpErrors: NORMAL for HTTP 400/408/429 and 5xx and ordinary transport failures; UNEXPECTED for other 4xx (401/403/etc.) and TLS/certificate validation failures. - PollingStrategy: new state-machine encapsulation with onFailure(class) / onSuccess() / nextWait() methods. State: n (formula input), initialDelay, maxDelay, priorPollWasSuccessful. Wait floor: max(pollInterval, T - J). Two-consecutive-successes returns from extended to normal regime. - PollingProcessor: rewired to a self-driven loop using strategy.nextWait(). Removed the State.OFF permanent-stop path entirely; state stays INITIALIZING/INTERRUPTED with a lastError. - StreamProcessor: consumes okhttp-eventsource's new multi-strategy retry API (see launchdarkly/okhttp-eventsource#110). On UNEXPECTED classification, activates the extended-regime RetryDelayStrategy on the underlying EventSource; the library's built-in healthy-op reset returns to normal-regime timing after 60 s of continuous connectivity. - Constructor plumbing: PollingProcessor and StreamProcessor take extendedInitialReconnectDelay, extendedStreamMaxRetryDelay, retryResetInterval, and extendedInitialDelay as constructor parameters; package-private defaults threaded through ComponentsImpl. - Contract test service: declares retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling capabilities. Tests: - Unit tests: full test suite green. New coverage for classifier (HttpErrorsClassificationTest), strategy state machine (PollingStrategyTest), and extended-regime timing observation in StreamProcessorTest. Existing 401/403 tests rewritten to assert extended-regime retry rather than permanent stop. - Contract tests via sdk-test-harness PR #404 (RETRY-conformance tests): 7/7 parallel shards pass end-to-end at production timing (5-minute extended-initial-delay), ~12 min wall clock. CI: intentionally red on this PR until launchdarkly/okhttp-eventsource#110 and launchdarkly-java-sdk-internal 1.11.0 are released to Maven Central. The multi-strategy retry API this SDK relies on is only in that PR's branch, and the classifier helpers are only in the 1.11.0 branch. Once released, bump both versions in lib/sdk/server/build.gradle.
4 tasks
tanderson-ld
marked this pull request as ready for review
August 21, 2026 20:36
jsonbailey
reviewed
Aug 21, 2026
jsonbailey
left a comment
There was a problem hiding this comment.
Overall, looks good. I'll hold off on approval until the cursor comments are addressed.
tanderson-ld
added a commit
to launchdarkly/java-core
that referenced
this pull request
Aug 24, 2026
…polling data sources (SDK-2789) Guided by the server-sdk-guide.md in sdk-scratchpad; analogous to the Go server SDK's reference implementation. The behavioral change: HTTP responses that today cause a data source to permanently stop (notably 401, 403, other 4xx) and TLS/certificate validation failures are no longer terminal. Streaming enters an extended backoff regime (5 min -> 1 hour, doubling); polling continues at its configured cadence with extended-regime waits between failing polls. Recovery from either regime uses a healthy-operation reset (60 s of continuous connectivity for streaming; two consecutive successful polls for polling). Scope: FDv1 streaming and polling data sources under `lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/`. FDv2 is out of scope for this epic and is deferred to a future one; nothing in `datasourcev2/` or the DataSystem-related code paths is touched. The classifier this depends on (`FailureClass` + `HttpErrors.classify*`) lives in `launchdarkly-java-sdk-internal` and ships in its own PR. Highlights: - PollingStrategy: new state-machine encapsulation with onFailure(class) / onSuccess() / nextWait() methods. State: n (formula input), initialDelay, maxDelay, priorPollWasSuccessful. Wait floor: max(pollInterval, T - J). Two-consecutive-successes returns from extended to normal regime. - PollingProcessor: rewired to a self-driven loop using strategy.nextWait(). Removed the State.OFF permanent-stop path entirely; state stays INITIALIZING/INTERRUPTED with a lastError. - StreamProcessor: consumes okhttp-eventsource's new multi-strategy retry API (see launchdarkly/okhttp-eventsource#110). On UNEXPECTED classification, activates the extended-regime RetryDelayStrategy on the underlying EventSource; the library's built-in healthy-op reset returns to normal-regime timing after 60 s of continuous connectivity. - Constructor plumbing: PollingProcessor and StreamProcessor take extendedInitialReconnectDelay, extendedStreamMaxRetryDelay, retryResetInterval, and extendedInitialDelay as constructor parameters; package-private defaults threaded through ComponentsImpl. - DataSourceStatusProvider Javadocs: State.INITIALIZING, State.OFF, State.INTERRUPTED, and getStateSince OFF-case updated to reflect the new semantics (no HTTP-error -> OFF transition). - LDClient constructor Javadoc: describes an SDK-key rejection as ongoing background retry rather than an "unsuccessful initialization" that reads as terminal. - Contract test service: declares retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling capabilities. Tests: - Unit tests: full test suite green. New coverage for the strategy state machine (PollingStrategyTest) and extended-regime timing observation in StreamProcessorTest. Existing 401/403 tests rewritten to assert extended-regime retry rather than permanent stop. - Contract tests via sdk-test-harness PR #404 (RETRY-conformance tests): 7/7 parallel shards pass end-to-end at production timing (5-minute extended-initial-delay), ~12 min wall clock. CI: intentionally red on this PR until launchdarkly/okhttp-eventsource#110 releases okhttp-eventsource 5.0.0 and #204 releases launchdarkly-java-sdk-internal 1.11.0. The multi-strategy retry API this SDK relies on is only in that eventsource PR's branch, and the classifier helpers are only in that internal-artifact PR's branch. Once both are released, bump both versions in lib/sdk/server/build.gradle.
- remove Builder.retryDelay(long, TimeUnit) in favor of retryDelayStrategy(defaultStrategy().initialDelay(...)) - reject null in Builder.retryDelayStrategy with IllegalArgumentException - clamp server-directed retry hints at MAX_SERVER_DIRECTED_RETRY_DELAY_MILLIS (1 hour), matching the Go SDK's ApplyRetryTime - treat null return from RetryDelayStrategy.getNext() as "reuse this" instance so naive migration from v4's Result.next=null does not NPE - healthy-op reset now uses disconnectedTime - connectedTime instead of now - connectedTime, so consumer processing between fault delivery and the sleep-time compute does not spuriously reset backoff - clamp base against maxDelay in DefaultRetryDelayStrategy constructor so the first getDelayMillis() respects the configured max, matching the pre-PR apply() behavior - document race semantics on activateRetryDelayStrategy - migrate all tests and the contract-test service off the removed retryDelay method
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ 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 4999d92. Configure here.
CI jacocoTestCoverageVerification flagged the throw branch in Builder.retryDelayStrategy as uncovered.
Storing baseDelayMillis clamped against maxDelayMillis at construction destroyed the caller's intent whenever a builder chain called initialDelay before maxDelay (e.g. .initialDelay(5, MINUTES).maxDelay(1, HOURS) on defaultStrategy(), which carries DEFAULT_MAX_DELAY_MILLIS=30s until the maxDelay call replaces it). The first constructor invocation would clamp base to 30s, and no subsequent call could recover it. Preserve baseDelayMillis as-configured and apply the max clamp only when computing delayMillis. getNext() continues to clamp on the successor as before.
jsonbailey
approved these changes
Aug 24, 2026
Contributor
Author
|
Waiting to merge/release until java-server PR is done with review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Redesigns the retry-delay API on
EventSourceto support the LaunchDarkly RETRY-specification regime-switching pattern. Replaces the narrowsetInitialRetryDelayMillis/setMaxRetryDelayMillisshape from the previous PR (#109) with a multi-strategy activation model.Draft while the downstream consumer (java-server-sdk via java-core PR #200) is reworked to validate the new API end-to-end.
Tracks SDK-2789 under the RETRY-conformance epic SDK-2775.
Motivation
The previous PR's narrow setters had two fatal design smells:
EventSource.setMaxRetryDelayMillishad to reach through the abstractRetryDelayStrategyto a concreteDefaultRetryDelayStrategyviainstanceof. Custom strategies got a silent no-op.apply(long baseDelayMillis)argument conflated wire retry hints with backoff progression, forcing every caller to pass a base value the strategy usually ignored.The multi-strategy shape resolves both: no
instanceof, no argument coupling, and per-strategy initial delays become expressible so an extended-regime strategy can start at 5 min while normal starts at 1 s.What ships
RetryDelayStrategy(breaking)apply(long)andResultare removed.getDelayMillis()returns the delay for the current retry.getNext()returns the successor instance (immutable-progression pattern).withBaseDelayMillis(long)(default no-op) is the mutation channel for server-directedretry:hints. Custom strategies without a base concept opt out by not overriding.DefaultRetryDelayStrategyinitialDelay(long, TimeUnit)builder method for per-strategy initial delay.baseDelayMillisfield.ThreadLocalRandominstead ofSecureRandom(backoff jitter doesn't need cryptographic entropy).EventSourceactivateRetryDelayStrategy(RetryDelayStrategy)swaps the active registered strategy at runtime. Null / unregistered = silent no-op.Builder.retryDelayStrategy(RetryDelayStrategy)has additive semantics: first call sets the default (initially active AND the healthy-op reset target); subsequent calls register additional strategies for later activation.retry:hints are stored inserverDirectedInitialDelayMillisand applied to every registered strategy's reset instance — sticky across activations, matching WHATWG semantics.getBaseRetryDelayMillis()/getNextRetryDelayMillis(). The reconnect delay is observable via the"Waiting X milliseconds before reconnecting"log message.delayNow = nextDelay - (now - disconnectedTime)subtraction — aligned with Go, .NET, and Swift SSE clients which sleep for the full computed delay.Consumer example
```java
RetryDelayStrategy normal = RetryDelayStrategy.defaultStrategy()
.initialDelay(1, TimeUnit.SECONDS)
.maxDelay(30, TimeUnit.SECONDS);
RetryDelayStrategy extended = RetryDelayStrategy.defaultStrategy()
.initialDelay(5, TimeUnit.MINUTES)
.maxDelay(1, TimeUnit.HOURS);
EventSource es = new EventSource.Builder(...)
.retryDelayStrategy(normal) // first call = default
.retryDelayStrategy(extended) // second call = additional
.build();
// On extended-regime classification:
es.activateRetryDelayStrategy(extended);
// On healthy-op reset (revert to normal):
es.activateRetryDelayStrategy(normal);
```
Testing
EventSourceRetryDelayStrategyUsageTestcover: activation swap, per-strategy state preservation across activations, healthy-op reset reverting to default, null/unregistered no-op.es.nextReconnectDelayMillisfield reads to areadReconnectDelayFromLog()helper that consumes the info log.Downstream
Consumed by java-core PR #200 for the Java Server SDK's RETRY-conformance work. That PR's CI will be red until this ships to Maven Central.
Note
Overview
Breaking redesign of SSE reconnect timing: retry configuration moves from
Builder.retryDelay(...)andRetryDelayStrategy.apply(long)into immutable strategy instances (getDelayMillis()/getNext()), with optionalinitialDelay(...)onDefaultRetryDelayStrategyand wire overrides viawithBaseDelayMillis(long).EventSourcenow registers multiple strategies at build time (first call = default + healthy-op reset target; later calls = additional). Runtime switching usesactivateRetryDelayStrategy, with per-strategy backoff state preserved across swaps. Serverretry:hints are clamped (1h cap), applied to all registered strategies, and stay sticky through resets. Reconnect sleep is computed at sleep time (not at fault time), healthy-op duration uses connection length only, andgetBaseRetryDelayMillis/getNextRetryDelayMillisare removed.Contract tests and the full test suite migrate to
retryDelayStrategy(defaultStrategy().initialDelay(...))and log-based delay assertions.Reviewed by Cursor Bugbot for commit e6f2675. Bugbot is set up for automated code reviews on this repo. Configure here.