feat(internal): add FailureClass enum and classifier helpers to HttpErrors (SDK-2789) - #204
Merged
Merged
Conversation
…rrors (SDK-2789) Introduces a shared classifier that categorizes network failures into NORMAL or UNEXPECTED regimes. Enables downstream server-SDK code to select between normal and extended-regime backoff without duplicating the classification rules. - FailureClass enum (NORMAL, UNEXPECTED) with a package-private cause-chain scan for TLS / certificate exceptions. - HttpErrors.classifyHTTPFailure(int): 400 / 408 / 429 and 5xx are NORMAL; other 4xx (401 / 403 / etc.) are UNEXPECTED; non-4xx / non-5xx failure statuses are NORMAL. - HttpErrors.classifyTransportFailure(Throwable): TLS or certificate validation anywhere in the exception chain is UNEXPECTED; all other transport failures are NORMAL. - HttpErrors.classifyAndLogHTTPFailure / classifyAndLogTransportFailure: classify, log at the appropriate level (Error for UNEXPECTED, Warn for NORMAL), and return the classification. - Deprecates HttpErrors.isHttpErrorRecoverable and checkIfErrorIsRecoverableAndLog in favor of the classify* helpers. The boolean "give up permanently" contract does not fit callers that keep retrying regardless of classification; existing callers can migrate incrementally. Unit coverage: HttpErrorsClassificationTest exercises the classifier against the full 4xx / 5xx / transport / TLS-cause matrix. Enables SDK-2789 (server SDK's RETRY-spec conformance work), which consumes these helpers in its FDv1 streaming and polling data sources.
tanderson-ld
added a commit
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.
4 tasks
joker23
reviewed
Aug 24, 2026
- FailureClass: drop redundant CertificateException check (already covered by GeneralSecurityException, which is its parent) - HttpErrors: rename classifyHTTPFailure -> classifyHttpFailure and classifyAndLogHTTPFailure -> classifyAndLogHttpFailure to match the existing Http camelCase convention (httpErrorDescription, HttpErrorException) and Java standard-library naming - HttpErrors.classifyAndLogTransportFailure: log exceptions via LogValues.exceptionSummary instead of e.toString(), matching the existing pattern in DefaultEventProcessor
|
Looks good to me, I can defer the review to @jsonbailey per review request |
jsonbailey
approved these changes
Aug 24, 2026
tanderson-ld
pushed a commit
that referenced
this pull request
Aug 25, 2026
🤖 I have created a release *beep* *boop* --- ## [1.11.0](launchdarkly-java-sdk-internal-1.10.0...launchdarkly-java-sdk-internal-1.11.0) (2026-08-25) ### Features * **internal:** add FailureClass enum and classifier helpers to HttpErrors (SDK-2789) ([#204](#204)) ([79766a1](79766a1)) --- 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 PR** for `lib/shared/internal`: bumps **launchdarkly-java-sdk-internal** from **1.10.0** to **1.11.0** in `gradle.properties`, `.release-please-manifest.json`, and the package changelog. > > The **1.11.0** release notes document the shipped feature from [#204](#204): a **`FailureClass`** enum and classifier helpers on **`HttpErrors`** for categorizing HTTP failures (SDK-2789). This diff does not include application code—only versioning and release metadata. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3b9fcad. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
tanderson-ld
added a commit
that referenced
this pull request
Aug 31, 2026
…#200) ## Summary Implements RETRY-spec conformance in the Java server SDK's FDv1 streaming and polling data sources ([SDK-2789](https://launchdarkly.atlassian.net/browse/SDK-2789)). This PR is scoped to the server SDK only; the classifier helpers it consumes ship separately in #204. **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 - **`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](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` docs** — `State.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. The classifier this SDK depends on — `FailureClass` and `HttpErrors.classify*` helpers — ships in #204. ## Testing - **Unit tests** — Full suite green. New coverage: `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](launchdarkly/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 - [x] 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. - [x] Review the `DataSourceStatusProvider` Javadoc changes for accuracy vs. the current state machine. - [x] 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. ## Dependencies (why CI is red) Two unreleased upstream artifacts: - **`launchdarkly-java-sdk-internal ≥ 1.11.0`** — Must be released before this PR can build against Maven Central. Ships via #204 + its release-please chore. - **`okhttp-eventsource ≥ 5.0.0`** — Must be released before this PR can build against Maven Central. Ships via [launchdarkly/okhttp-eventsource#110](launchdarkly/okhttp-eventsource#110). 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. [SDK-2789]: https://launchdarkly.atlassian.net/browse/SDK-2789?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > **FDv1 data sources no longer treat HTTP auth and other “unexpected” failures as terminal.** Streaming and polling keep retrying in the background with classified backoff instead of moving to `State.OFF` (e.g. 401/403). > > **Polling** switches from fixed-rate scheduling to a self-driven loop backed by new **`PollingStrategy`**: normal cadence at `pollInterval`, extended exponential backoff (default 5 min → 1 hr cap, jitter) after `FailureClass.UNEXPECTED`, reset after two consecutive successes. > > **Streaming** adopts **okhttp-eventsource 5.x** multi-regime `RetryDelayStrategy`: normal reconnect caps at 30s; on unexpected failures the SDK activates an extended strategy (5 min → 1 hr) and relies on a 60s healthy-connection threshold to return to normal timing. > > **Public docs and tests** align with the new model: `DataSourceStatusProvider` Javadoc and `LDClient` init wording no longer describe HTTP errors as permanent shutdown; contract-test service advertises retry-conformance capabilities; unit/e2e tests assert continued retry and extended-regime behavior. Dependencies bump **`launchdarkly-java-sdk-internal`** and **`okhttp-eventsource`** for `HttpErrors` classification and strategy APIs. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 50513bc. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> BEGIN_COMMIT_OVERRIDE feat: conform FDv1 streaming and polling data sources to the RETRY specification HTTP responses that previously caused a data source to permanently stop (notably 401, 403, and other 4xx), and TLS certificate validation failures no longer terminate it. Streaming enters an extended backoff regime starting at 5 minutes and doubling to a 1 hour ceiling; polling continues at its configured interval but engages the extended regime after an unexpected failure. Streaming returns to normal backoff after 60 seconds of continuous healthy operation; polling returns to its normal cadence after two consecutive successful polls. Two consequences are visible to applications. DataSourceStatusProvider.State.OFF is now reached only by explicit shutdown, not by an HTTP error, so applications monitoring for OFF to detect an invalid SDK key will no longer see it. And because an invalid SDK key no longer short circuits initialization, the LDClient constructor waits out the full startWait timeout rather than returning as soon as the 401 arrives. END_COMMIT_OVERRIDE
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
3 participants
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
Introduces a shared network-failure classifier in
launchdarkly-java-sdk-internalthat categorizes failures as either NORMAL (typically transient) or UNEXPECTED (indicative of a longer-lived condition — e.g., invalid SDK key, TLS misconfiguration). Downstream SDKs can use the classification to select between normal-regime and extended-regime backoff.Enables the server SDK's RETRY-spec conformance work in SDK-2789 (see PR #200), which needs these helpers to select its retry regime.
What changed
FailureClassenum —NORMALandUNEXPECTED, with a package-private cause-chain scan for TLS / certificate exceptions.HttpErrors.classifyHTTPFailure(int)— ReturnsNORMALfor 400 / 408 / 429, 5xx, and any other status the SDK treats as a failure; returnsUNEXPECTEDfor other 4xx (401 / 403 / etc.).HttpErrors.classifyTransportFailure(Throwable)— ReturnsUNEXPECTEDif TLS or certificate validation appears anywhere in the exception chain;NORMALotherwise.HttpErrors.classifyAndLogHTTPFailure/classifyAndLogTransportFailure— Classify, log at the appropriate level (Error forUNEXPECTED, Warn forNORMAL), and return the classification.HttpErrors.isHttpErrorRecoverableandcheckIfErrorIsRecoverableAndLogare now@Deprecated. The boolean "give up permanently" contract doesn't fit callers that keep retrying regardless of classification; the Javadoc points migrations at the new helpers. Existing callers keep working — deprecation is source-compatible.Testing
HttpErrorsClassificationTestcovers the full HTTP status matrix (400/408/429/5xx = NORMAL, 401/403/404/418/451 = UNEXPECTED, non-4xx/non-5xx = NORMAL) and the transport-exception matrix (ordinary I/O = NORMAL, SSL/certificate = UNEXPECTED, TLS as a cause of a wrapper exception = UNEXPECTED).lib/shared/internaltests continue to pass; the deprecated methods still exercise their original behavior.Test plan for reviewers
UNEXPECTED.Downstream
Once this PR merges and release-please publishes
launchdarkly-java-sdk-internal 1.11.0, launchdarky/java-core#200 will bump itsbuild.gradledep to 1.11.0 and consume the classifier from the server SDK's data sources.Note
Overview
Adds shared internal network failure classification so data sources can pick normal vs extended backoff instead of treating some errors as “stop retrying forever.”
Introduces a
FailureClassenum (NORMALvsUNEXPECTED) andHttpErrorshelpers to classify HTTP status codes (e.g. 400/408/429 and 5xx →NORMAL; most other 4xx including 401/403 →UNEXPECTED) and transport exceptions (TLS/certificate causes anywhere in the chain →UNEXPECTED). NewclassifyAndLogHttpFailure/classifyAndLogTransportFailureclassify, log at Error vs Warn, and return the class for the caller.isHttpErrorRecoverableandcheckIfErrorIsRecoverableAndLogare deprecated (behavior unchanged); Javadoc points callers at the new APIs. Unit tests cover the HTTP and transport classification matrices.Reviewed by Cursor Bugbot for commit f67fd5e. Bugbot is set up for automated code reviews on this repo. Configure here.