Skip to content

fix: Make server FDv1 fallback TTL-based and recover FDv2 after the TTL - #2110

Draft
beekld wants to merge 1 commit into
skz/SDK-2912/fdv2-recovery-orchestration-recovery-timerfrom
devin/1792430159-server-fdv1-fallback-ttl
Draft

beekld wants to merge 1 commit into
skz/SDK-2912/fdv2-recovery-orchestration-recovery-timerfrom
devin/1792430159-server-fdv1-fallback-ttl

Conversation

@beekld

@beekld beekld commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

This PR makes the server SDK's FDv1 fallback TTL-based, per sdk-specs#251. Fallback is never permanent: FDv2 is retried once the TTL elapses.

Closes SDK-2983 for the server side (part of SDK-2935). Client side is covered by #1982 / #1875. Stacked on #1982 to reuse its resolveFallbackTtlMs, so retarget to main once that merges.

  • X-LD-FD-Fallback-TTL is now honored. A TTL in (0, 3600]s is used as-is. Missing, malformed, zero, negative, or > 3600 becomes a jittered default in [30min, 1h].
  • CompositeDataSource keeps its FDv2 synchronizers while on FDv1 and restores them after the TTL. This also applies with no FDv1 synchronizer (fdv1Fallback: null): the data source waits out the TTL and retries FDv2 instead of terminating with Closed.
  • The FDv2 source that sent the directive is no longer culled, so recovery has something to return to.
  • Before the next source starts, the current source's callbacks are detached and it is stopped. FDv1 and FDv2 never write concurrently.
  • Deliberately unchanged: the server still doesn't treat in-band goodbye.protocolFallbackTTL as a fallback signal (it never did, and it reads headers). Contract tests are unchanged because the harness doesn't cover fallback TTL/recovery.
Implementation details

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Mechanism

  • Requestor and StreamingProcessorFDv2 parse the directive with the shared internal.readFallbackDirective instead of checking only x-ld-fd-fallback.
  • The resolved TTL travels on LDFlagDeliveryFallbackError.fdv1FallbackTtlMs, an optional fourth constructor argument. It also travels on the payload marker as fdv1FallbackTtlMs next to fallbackToFDv1. LDFeatureRequestor's callback gets an optional fifth argument for it.
  • CompositeDataSource schedules a cancellable recovery timer when fallback engages. The main loop races that timer with the current source's transitions and external stop. When the timer fires, the composite restores the FDv2 list, clears the engaged flag, and does a switchToSync. When no FDv1 source is available, the loop waits on the timer instead of resolving stop.
  • LDFlagDeliveryFallbackError is recoverable: false. The composite previously culled the reporting FDv2 factory before engaging fallback. The fallback check now runs first. Later fallback errors from FDv1 sources are still culled and throttled as before.

Testing

  • Requestor: valid TTL headers (1, 500, 3600) pass through unchanged. Missing, 0, -5, 3601, and abc give a value in [30min, 1h].
  • StreamingProcessorFDv2: the open-header TTL is attached to the payload marker.
  • CompositeDataSource: FDv2 is retried after the TTL with no FDv1 synchronizer (replaces the old "terminates" test). FDv1 is stopped before FDv2 restarts (event-order assertion). The FDv1 retry-throttling test now ends on a valid FDv1 source, because exhausting FDv1 sources no longer terminates the data source.
  • Ran locally: js-sdk-common and js-server-sdk-common unit tests, lint, format check, and build all pass.

Reference implementation: cpp-sdks#631.

Link to Devin session: https://app.devin.ai/sessions/70626b2e151142fd9c4fbbedd01d08df
Open in Devin Desktop: https://app.devin.ai/desktop/session/70626b2e151142fd9c4fbbedd01d08df?variant=devin
Requested by: @beekld

Co-Authored-By: Bee Klimt <bklimt@launchdarkly.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

This branch has not been deployed

No deployments
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