feat: conform FDv1 streaming and polling to RETRY spec by tanderson-ld · Pull Request #429 · launchdarkly/go-server-sdk

tanderson-ld · 2026-08-07T20:26:43Z

Summary

Brings the FDv1 streaming and polling data sources into conformance with the RETRY specification. Previously-terminal HTTP responses (401, 403, other 4xx that aren't 400/408/429) and TLS/certificate validation failures no longer cause a permanent stop — the data sources engage an extended-regime backoff and continue retrying indefinitely.

Streaming: unexpected failures activate an extended retry curve on the underlying eventsource stream (base 5 min → cap 1 h, doubling with jitter). Sixty seconds of continuous healthy operation reverts to the normal-regime curve.
Polling: new `pollingStrategy` state machine encoding RETRY §1.4 with a wait floor at `PollInterval` per §1.4.4. Unexpected failures engage the extended regime with `initialDelay = max(configured, PollInterval)`; two consecutive successful polls reset back to the normal regime per §1.8.

FDv2 adoption of RETRY is out of scope for this ticket (deferred to a future epic).

Public API

Unchanged. New test-only knobs are exposed via `.Internal()` escape hatches on the streaming and polling builders, with matching free-function wrappers in a new `testhelpers/datasourcetest` package. Production code must not import that package — it exists so the sdk-test-harness can compress the extended regime into an observable window during contract tests.

Testservice / contract tests

Adds `ExtendedInitialDelayMS` / `ResetThresholdMS` on the streaming servicedef, `ExtendedInitialDelayMS` on polling.
Declares the `retry-conformance-fdv1-streaming` and `retry-conformance-fdv1-polling` capabilities so the sdk-test-harness can drive the new conformance suite.
Contract tests: feat: add RETRY-conformance contract tests for FDv1 streaming and polling sdk-test-harness#404 (targeting `v2`).

Test plan

Unit tests in `internal/datasource` pass (streaming + polling + `pollingStrategy`)
End-to-end tests in `ldclient_end_to_end_test.go` pass
Contract tests (feat: add RETRY-conformance contract tests for FDv1 streaming and polling sdk-test-harness#404) pass against this branch's testservice (124 total, 108 ran, 16 skipped, 0 failed)
Depends on eventsource release before this can merge

Note

Overview
FDv1 streaming and polling now retry indefinitely instead of treating 401/403/other 4xx (except 400/408/429) and TLS/cert errors as terminal. Those failures are classified as unexpected and use a slower backoff (default 5m, cap 1h, jitter); other errors stay on the normal curve. Status stays Interrupted (or Initializing if never Valid); Off is reserved for explicit shutdown or unparseable config. Init on a bad key now times out (ErrInitializationTimeout) rather than ErrInitializationFailed.

Streaming uses eventsource retry profiles (eventsource 1.14) and cancels via request context. Polling adds a pollingStrategy state machine with a PollInterval wait floor and reset after two consecutive successes.

Docs for DataSourceStateOff and client init errors are updated to match. Testservice advertises retry-conformance-fdv1-streaming / retry-conformance-fdv1-polling.

^{Reviewed by Cursor Bugbot for commit 576f4d3. Bugbot is set up for automated code reviews on this repo. Configure here.}

Bring the FDv1 streaming and polling data sources into conformance with the RETRY specification: no HTTP response and no transport-level failure causes the data source to permanently stop. Previously-terminal 4xx statuses (401, 403, 405, other 4xx that aren't 400/408/429) and TLS/cert validation failures now trigger an extended-regime backoff and continue retrying indefinitely. Streaming: classifies each failure via classifyHTTPFailure / classifyTransportFailure into Normal or Unexpected. Unexpected failures activate an extended retry curve on the underlying eventsource stream (base 5 min, max 1 hour, doubling with jitter) instead of stopping. Sixty seconds of continuous healthy operation returns the source to the normal-regime curve. Polling: introduces pollingStrategy, an encapsulated RETRY §1.4 state machine (attempts / regime / rng) with a wait floor at PollInterval per RETRY §1.4.4. Unexpected classifications engage the extended regime with initialDelay = max(configured, PollInterval); two consecutive successful polls reset back to the normal regime per RETRY §1.8. The poll loop uses a dynamically-resettable timer to serve the state machine. Configuration surface (public builders): unchanged. New test-only knobs are exposed via <Builder>.Internal() escape hatches on both builders, with matching free-function wrappers in the new testhelpers/datasourcetest package. Production code must not import that package; it exists so that contract tests can compress the extended regime into an observable window. Testservice: adds ExtendedInitialDelayMS / ResetThresholdMS knobs on the streaming servicedef, ExtendedInitialDelayMS on polling, and declares the retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling capabilities so the sdk-test-harness can drive the new conformance suite. go.mod / testservice/go.mod carry a temporary local replace directive pointing to ../eventsource so this branch can build against the unreleased RetryCurve API in eventsource PR #68. The TODO comment on the replace notes it must be removed once the eventsource release tagging that API ships.

The branch still depends on the unreleased RetryCurve API in eventsource PR #68 to compile, but committing the local replace directive to the PR would mask the fact that this PR cannot merge until that API is released. Reviewers should treat the pending CI failure ("undefined: eventsource. NewRetryCurve" etc.) as the intended signal. To iterate locally in the meantime, add a personal (uncommitted) replace github.com/launchdarkly/eventsource => ../eventsource to go.mod / testservice/go.mod. Once an eventsource release with the RetryCurve API is tagged, bump the eventsource require line here.

pollingStrategy.OnFailure previously incremented the counter unconditionally before applying the regime swap. A sequence of "normal, normal, unexpected" would leave the counter at 3 at the moment of the regime swap, producing a first-extended-regime wait of initialDelay * 2^2 = 20min rather than the intended 5min (bounded by extendedPollMaxDelay = 1h in the worst case). Fix: on the transition from normal into extended regime (detected via initialDelay == normalInterval at the moment of the unexpected failure), reset n to 1 so the formula yields initialDelay * 2^0 = initialDelay. RETRY §1.5.3 explicitly delegates the increment behavior on unexpected failure to the component's own specification, so this is spec-conformant. Also rename the field from `attempts` to `n` to (a) name the field for its role as the formula input (RETRY §1.4.1) and (b) match the naming convention used in the RETRY spec and streaming Confluence spec. Adds two regression tests: - TestPollingStrategy_UnexpectedAfterNormalFailuresStartsAtInitialDelay covers the specific case that was broken (normal, normal, normal, unexpected → first extended wait ∈ [2.5min, 5min]). - TestPollingStrategy_UnexpectedWhileAlreadyExtendedContinuesDoubling covers the counterpart (second unexpected in extended does NOT re-reset n; normal failure in extended increments n without exiting the regime). Contract-test scenarios in sdk-test-harness all begin with an unexpected failure as the first failure of the SDK's lifetime, so the bug was not triggered by the harness. Discovered during retro discussion.

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.

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

Resolves conflict in testservice/servicedef/service_params.go: keeps CapabilityHookEnvironmentID from v7 (#430) alongside CapabilityRetryConformanceFDv1Streaming and CapabilityRetryConformanceFDv1Polling from this branch.

Threads on PR #429: 1. Close cannot stop stream retries Replace halt chan with streamReqCtx / streamReqCancel context. Build the streaming request via http.NewRequestWithContext so Close() interrupts an in-flight Do; retry-sleep interruption arrives with eventsource #71 once it releases. 2. Contradictory polling retry logs Fold both classifyAndLog branches to Warnf per CLM 1.1.4 (under RETRY no failure is permanent, so per-attempt logs are always temporary conditions). Drop the hardcoded "will continue retrying with extended backoff" suffix that contradicted polling's "will retry at next scheduled poll interval". Add a one-time Info log on the transition into extended regime for both streaming and polling: "Classified failure as UNEXPECTED; engaging extended backoff." 3, 5. Unicode dashes in Go comments -- replaced with ASCII across all PR-touched files. 4. Broken extended-regime transition detection Replace equality-based detection (initialDelay == normalInterval) with an explicit inExtended flag on pollingStrategy. The prior check re-fired the transition path on every unexpected failure whenever PollInterval >= extendedInitialPollInterval (via the clamp), which held n at 1 and defeated RETRY 1.4.1's doubling. OnFailure now returns a bool signalling the transition once, so the caller emits the extended-backoff Info log exactly once per transition. OnSuccess's two-consecutive-success reset also clears inExtended so subsequent unexpected failures re-transition properly. Regression tests: doubling under PollInterval == extendedInitial, OnFailure transition-return semantics, post-reset re-transition, PollInterval = 10min (moderate), PollInterval = 2h (collapse case where extended regime is indistinguishable from normal cadence). SDK-2788

Two lint findings from the previous commit's changes to NewStreamProcessor / subscribe: - gosec G118: streamReqCancel is stored on the returned StreamProcessor and invoked from Close(). gosec's scope-local heuristic doesn't see the cross-scope call. Suppress explicitly with a nolint directive and a justification comment. - lll (line too long): wrap the http.NewRequestWithContext call across four lines to get under the 120-char limit. Local make lint clean across all four modules.

…rmance-work # Conflicts: # go.mod # go.sum

Reviewers rejected exposing extended-regime timing knobs on the SDK's public builders -- those would put a test-harness bypass in the customer-facing surface. Delete every public addition while preserving the RETRY-spec behavior. Removed: - Builder.Internal() accessors and *Internal types - DefaultExtendedInitialReconnectDelay, DefaultRetryResetInterval, DefaultExtendedInitialPollInterval public constants - testhelpers/datasourcetest/ package - Servicedef ExtendedInitialDelayMS / ResetThresholdMS fields (matches sdk-test-harness PR #404 refactor) Kept: full RETRY behavior (401/403/other-4xx no longer terminal; extended regime engages), all defaults unchanged (5-min extended-initial, 60s activeSince reset). Contract test capabilities retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling remain declared. Added: defaultExtendedInitialPollDelay fallback in internal/datasource so removing the public constant doesn't silently drop the 5-min default. Streaming already had the equivalent fallback pre-refactor. E2E tests exercising 401 retry now use a test-local ComponentConfigurer (streaming) or default polling config (polling) instead of the deleted datasourcetest helpers. Also tidied polling_strategy_test.go to reference the new fallback constant instead of scattered 5*time.Minute literals where semantics matched. Validated: full RETRY-conformance suite (13 leaf tests, 9 streaming + 4 polling) passed against the sdk-test-harness PR #404 branch with -enable-long-running-tests at real production 5-minute extended-regime timing.

eventsource v1.14.0 (via launchdarkly/eventsource#71, released as #72) teaches Subscribe to observe the HTTP request's context as a stream- lifetime cancel signal. StreamProcessor.Close already cancels its streamReqCtx, so this consumption makes Close interrupt any in-flight retry timer instead of blocking until the timer fires -- addressing the "Shutdown skips explicit stream close" Cursor Bugbot finding on this PR. No SDK code change is needed: the existing streamReqCtx cancellation now propagates through eventsource's new context observer.

cursor

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 76dcb98. Configure here.}

…cted failures The RETRY refactor changed SDK behavior (keep retrying instead of stopping) but not the semantic meaning of unexpected errors -- 401/403 and TLS/cert failures still almost always indicate a real customer-side misconfiguration requiring operator attention. Restore pre-RETRY loudness for those: - classifyAndLogHTTPFailure and classifyAndLogTransportFailure now log at Error for Unexpected classification, Warn for Normal (was Warn for all). - httpErrorDescription: restore "(invalid SDK key)" for 401/403 (was "(authentication failed)" post-refactor). Documentation updates: refresh docstrings on ErrInitializationFailed, MakeClient, MakeCustomClient, dataSystem.Start, DataSourceStateInitializing, DataSourceStateOff, DataSourceStatus.StateSince (Off case), UpdateStatus on both DataSourceUpdateSink and DataSourceStatusReporter, and the FDv1 streaming file-header comment. All reflect that HTTP-level failures no longer permanently stop the data source, and DataSourceStateOff is now reached only via explicit shutdown or unrecoverable startup configuration errors.

…rk' into ta/SDK-2788/retry-conformance-work

beekld

the code seems fine to me. the tests have good coverage. but i think we need a pass on the comments to bring them in line with our standards

Strip spec-section citations and stale ticket references from comments in the polling/streaming data source and its tests. Rewrite test comments that framed behavior in terms of prior implementation to describe current behavior only. Simplify the hard-to-follow comment on the polling-strategy n-reset test.

jsonbailey

Minor nits that I won't block but should be easily fixed

jsonbailey · 2026-08-21T16:29:23Z

+	putEvent    = "put"
+	patchEvent  = "patch"
+	deleteEvent = "delete"
+	// The LaunchDarkly stream should send a heartbeat comment every 3 minutes.


This comment was here previously but should we adjust this to say 5 minutes or set the timeout to be 3 minutes?

I think this is "the FD server SHOULD send a heartbeat every 3 minutes, so 5 minutes as the timeout is fine.

Co-authored-by: Jason Bailey <jbailey@launchdarkly.com>

tanderson-ld added 2 commits August 7, 2026 16:26

tanderson-ld marked this pull request as ready for review August 10, 2026 19:32

tanderson-ld requested a review from a team as a code owner August 10, 2026 19:32

cursor Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread internal/datasource/streaming_data_source.go

Comment thread internal/datasource/helpers.go Outdated

Comment thread internal/datasource/polling_strategy.go Outdated

cursor Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread internal/datasource/polling_strategy.go Outdated

Comment thread internal/datasource/polling_strategy_test.go Outdated

tanderson-ld changed the title ~~feat: conform FDv1 streaming and polling to RETRY spec (SDK-2788)~~ feat: conform FDv1 streaming and polling to RETRY spec Aug 11, 2026

tanderson-ld force-pushed the ta/SDK-2788/retry-conformance-work branch from 6d2b96e to a2972ad Compare August 11, 2026 17:29

tanderson-ld added 4 commits August 11, 2026 14:43

Merge branch 'v7' into ta/SDK-2788/retry-conformance-work

1835d23

Resolves conflict in testservice/servicedef/service_params.go: keeps CapabilityHookEnvironmentID from v7 (#430) alongside CapabilityRetryConformanceFDv1Streaming and CapabilityRetryConformanceFDv1Polling from this branch.

cursor Bot reviewed Aug 17, 2026

View reviewed changes

Comment thread internal/datasource/streaming_data_source.go

tanderson-ld added 2 commits August 17, 2026 16:25

Merge remote-tracking branch 'origin/v7' into ta/SDK-2788/retry-confo…

6d91fa8

…rmance-work # Conflicts: # go.mod # go.sum

cursor Bot reviewed Aug 17, 2026

View reviewed changes

Comment thread internal/datasource/streaming_data_source.go

tanderson-ld and others added 3 commits August 18, 2026 14:23

Merge branch 'v7' into ta/SDK-2788/retry-conformance-work

76dcb98

cursor Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread internal/datasource/polling_data_source.go

tanderson-ld added 2 commits August 19, 2026 15:05

Merge remote-tracking branch 'origin/ta/SDK-2788/retry-conformance-wo…

1084159

…rk' into ta/SDK-2788/retry-conformance-work

beekld requested changes Aug 19, 2026

View reviewed changes

tanderson-ld requested a review from beekld August 21, 2026 14:00

jsonbailey approved these changes Aug 21, 2026

View reviewed changes

Update internal/datasource/polling_strategy.go

576f4d3

Co-authored-by: Jason Bailey <jbailey@launchdarkly.com>

beekld approved these changes Aug 21, 2026

View reviewed changes

Uh oh!

Conversation

tanderson-ld commented Aug 7, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Public API

Testservice / contract tests

Test plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

beekld left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsonbailey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jsonbailey Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

tanderson-ld Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tanderson-ld commented Aug 7, 2026 •

edited by cursor Bot

Loading