Skip to content

feat(server): RETRY-spec conformance in FDv1 streaming and polling - #200

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

feat(server): RETRY-spec conformance in FDv1 streaming and polling#200
tanderson-ld wants to merge 2 commits into
mainfrom
ta/SDK-2789/retry-conformance-work

Conversation

@tanderson-ld

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

Copy link
Copy Markdown
Contributor

Summary

Implements RETRY-spec conformance in the Java server SDK's FDv1 streaming and polling data sources (SDK-2789).

Behavioral change. HTTP responses that today cause an FDv1 data source to permanently stop (notably 401 / 403 / other 4xx) and TLS / certificate validation failures are no longer terminal. Streaming enters an extended-regime backoff (5 min initial → 1 hr max, doubling); polling continues at its configured cadence but engages the extended-regime wait after an UNEXPECTED failure. Either regime returns to normal after 60 s of continuous healthy operation (streaming) or two consecutive successful polls (polling).

Scope. FDv1 streaming and polling under lib/sdk/server/. FDv2, event delivery, and other network callers are unchanged.

What changed

  • FailureClass enum + classifier helpers — Live in launchdarkly-java-sdk-internal's HttpErrors. Classifies HTTP status (400/408/429/5xx = NORMAL, other 4xx = UNEXPECTED) and transport exceptions (TLS / cert = UNEXPECTED).
  • PollingStrategy — New state-machine encapsulation with onFailure(class) / onSuccess() / nextWait(). State: n (formula input), initialDelay, maxDelay, priorPollWasSuccessful. Wait floor is max(pollInterval, T − J); two consecutive successes reset from extended → normal.
  • PollingProcessor — Rewired to a self-driven loop that consults strategy.nextWait() between attempts. The State.OFF permanent-stop path is removed; the state stays INITIALIZING or INTERRUPTED with a lastError.
  • StreamProcessor — Consumes okhttp-eventsource's new multi-strategy retry API from launchdarkly/okhttp-eventsource#110. On UNEXPECTED classification it calls activateRetryDelayStrategy on the underlying EventSource to switch into the extended-regime RetryDelayStrategy. The library's built-in healthy-op reset returns the SDK to normal-regime timing after 60 s of continuous connectivity.
  • DataSourceStatusProvider docsState.INITIALIZING, State.OFF, State.INTERRUPTED, and getStateSince OFF-case Javadocs updated to reflect the new semantics (no HTTP-error → OFF transition). Aligned with the Go server SDK's parallel doc adjustments.
  • LDClient constructor Javadoc — Wording tightened so a "wrong SDK key" scenario is described as ongoing retry in the background, not as an "unsuccessful initialization" that reads as terminal.
  • Contract-test service — Declares retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling capabilities.

Testing

  • Unit tests — Full suite green. New coverage: HttpErrorsClassificationTest (classifier), PollingStrategyTest (strategy state machine), plus extended-regime timing-observation tests in StreamProcessorTest. Existing 401 / 403 tests were rewritten to assert extended-regime retry rather than permanent stop.
  • Contract tests via sdk-test-harness#404 — All 7 RETRY-conformance test cases pass end-to-end at production timing (5-minute extended-initial delay). Total wall clock ~12 min via parallel shards.

Test plan for reviewers

  • Sanity-check the FailureClass mapping — particularly TLS / cert = UNEXPECTED and any 4xx other than 400 / 408 / 429 = UNEXPECTED.
  • Verify PollingStrategy transition semantics: normal → extended fires exactly once per UNEXPECTED failure; two consecutive successful polls fully reset (n = 0, delay bounds back to normal, inExtended cleared so a subsequent UNEXPECTED re-triggers the transition).
  • Verify StreamProcessor.handleError ordering: classifier → regime switch → updateStatus(INTERRUPTED, …), unconditionally returning true so the eventsource keeps retrying.
  • Review the DataSourceStatusProvider Javadoc changes for accuracy vs. the current state machine.
  • Code comments deliberately describe current behavior only — no spec section refs, no historical framing ("previously", "no longer"), no cross-SDK references. Confirm you'd expect a reader to find that acceptable.

Dependency (why CI is red)

The multi-strategy retry API and the classifier helpers are only in unreleased branches:

  • okhttp-eventsource ≥ 5.0.0 (see launchdarkly/okhttp-eventsource#110) — must be released before this PR can build against Maven Central.
  • launchdarkly-java-sdk-internal ≥ 1.11.0 — must be released before this PR can build against Maven Central.

Once both are released, bump their versions in lib/sdk/server/build.gradle and CI will go green. Locally, the branch builds against mavenLocal() snapshots of both.


Note

Overview
FDv1 streaming and polling no longer treat HTTP 401/403 (and other unexpected 4xx) or TLS/cert failures as a permanent stop. Those failures now keep retrying with an extended backoff (5 min initial, doubling to 1 hr) instead of moving the data source to State.OFF.

Streaming uses EventSource’s multi-strategy retry API and switches to the extended regime on UNEXPECTED classification, then returns to normal after 60s of healthy connectivity. Polling is a self-scheduled loop driven by a new PollingStrategy state machine; two consecutive successful polls reset to the configured poll interval.

Shared FailureClass / HttpErrors classifiers distinguish NORMAL (400/408/429/5xx, generic I/O) vs UNEXPECTED (other 4xx, TLS). Status docs and tests now assert ongoing retry rather than shutdown. Contract-test capabilities retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling are advertised. FDv2 and events are unchanged.

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

…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.
@tanderson-ld
tanderson-ld force-pushed the ta/SDK-2789/retry-conformance-work branch from f87cd37 to a754251 Compare August 21, 2026 20:32
@tanderson-ld
tanderson-ld marked this pull request as ready for review August 21, 2026 20:36
@tanderson-ld
tanderson-ld requested a review from a team as a code owner August 21, 2026 20:36
@tanderson-ld tanderson-ld changed the title feat(server): RETRY-spec conformance in FDv1 streaming and polling (SDK-2789) feat(server): RETRY-spec conformance in FDv1 streaming and polling Aug 21, 2026

@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 1 potential issue.

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 9394a65. Configure here.

if (waitMs < floorMs) {
waitMs = floorMs;
}
return Duration.ofMillis(waitMs);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Extended wait after poll success

Medium Severity

After a successful poll while still in the extended regime, onSuccess leaves n unchanged, so nextWait keeps returning the large extended backoff. The confirming second success can be delayed by up to an hour, which stalls recovery long after the data source is healthy again and conflicts with returning to the configured poll cadence.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9394a65. Configure here.

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