Skip to content

chore: schedule FDv2 recovery orchestration after fallback TTL expires - #1857

Closed
joker23 wants to merge 2 commits into
skz/SDK-2911/fdv1-fallback-fix-recovery-primitivesfrom
skz/SDK-2912/fdv1-fallback-fix-recovery-orchestration
Closed

chore: schedule FDv2 recovery orchestration after fallback TTL expires#1857
joker23 wants to merge 2 commits into
skz/SDK-2911/fdv1-fallback-fix-recovery-primitivesfrom
skz/SDK-2912/fdv1-fallback-fix-recovery-orchestration

Conversation

@joker23

@joker23 joker23 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Wires the FDv2-recovery primitives from the previous PR in this stack into FDv2DataSource's synchronizer run loop: after falling back to FDv1, a recovery condition races alongside the existing fallback/recovery conditions, using the server-supplied TTL (or a caller-configurable override, or a 1-hour default if absent -- a TTL of 0 means indefinite, no recovery scheduled).

On firing, SourceManager.fdv2Recovery() re-enables FDv2 and the loop naturally attempts a fresh FDv2 connection -- if the server still signals fallback on that attempt, fallback re-engages from the new result. Also guards against the FDv1 synchronizer's own results re-triggering the fallback machinery. Ships with regression tests covering TTL present/absent/zero and the no-re-trigger guard.


Note

Overview
Adds a guard in handleFdv1Fallback so when the active synchronizer is already the FDv1 fallback slot, results that still carry fdv1Fallback no longer call sourceManager.fdv1Fallback() again. That stops redundant fallback churn (e.g. spinning FDv2 back up or re-blocking slots) while the SDK is already on FDv1.

A regression test (SDK-2617) covers FDv2 signaling fallback, then the FDv1 synchronizer emitting both fallback-flagged and normal changeSets, and asserts FDv2 is only created once and both FDv1 payloads are delivered.

Also clarifies the synchronizer loop comment: condition timers must be closed on throw or early break, not only on normal paths.

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

@joker23
joker23 marked this pull request as ready for review August 13, 2026 17:55
@joker23
joker23 requested a review from a team as a code owner August 13, 2026 17:55
cursor[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@joker23
joker23 force-pushed the skz/SDK-2911/fdv1-fallback-fix-recovery-primitives branch from b3def90 to bb71b36 Compare August 18, 2026 20:12
@joker23
joker23 force-pushed the skz/SDK-2912/fdv1-fallback-fix-recovery-orchestration branch from 50a8250 to 4209e07 Compare August 18, 2026 20:12
@github-actions

Copy link
Copy Markdown
Contributor

@launchdarkly/js-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 26360 bytes
Compressed size limit: 29000
Uncompressed size: 129188 bytes

devin-ai-integration[bot]

This comment was marked as resolved.

@joker23
joker23 force-pushed the skz/SDK-2911/fdv1-fallback-fix-recovery-primitives branch from bb71b36 to b3def90 Compare August 18, 2026 20:19
@joker23
joker23 force-pushed the skz/SDK-2912/fdv1-fallback-fix-recovery-orchestration branch from 4209e07 to 50a8250 Compare August 18, 2026 20:19
@joker23
joker23 force-pushed the skz/SDK-2911/fdv1-fallback-fix-recovery-primitives branch from b3def90 to ad07e81 Compare August 18, 2026 20:29
@joker23
joker23 force-pushed the skz/SDK-2912/fdv1-fallback-fix-recovery-orchestration branch from 50a8250 to 6ef2c4f Compare August 18, 2026 20:29
createFDv2RecoveryCondition was removed from Conditions.ts in PR #1856; this drops the now-broken caller and its config/tracking from FDv2DataSource.ts.
@github-actions

Copy link
Copy Markdown
Contributor

@launchdarkly/js-client-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 41678 bytes
Compressed size limit: 44000
Uncompressed size: 225516 bytes

@github-actions

Copy link
Copy Markdown
Contributor

@launchdarkly/browser size report
This is the brotli compressed size of the ESM build.
Compressed size: 179960 bytes
Compressed size limit: 200000
Uncompressed size: 833634 bytes

@github-actions

Copy link
Copy Markdown
Contributor

@launchdarkly/js-client-sdk size report
This is the brotli compressed size of the ESM build.
Compressed size: 32473 bytes
Compressed size limit: 34000
Uncompressed size: 116454 bytes

@joker23

joker23 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

@cursor review

@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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 63d2f3d. Configure here.

@joker23

joker23 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Folding this into #1856. After removing the Condition-based FDv2 recovery primitive per review (#1856 (comment)), this layer's only remaining content was the FDv1-fallback re-trigger guard in handleFdv1Fallback -- which depends directly on #1856's isCurrentSynchronizerFDv1Fallback primitive. Folded that guard + its regression test into #1856 rather than keeping this as a near-empty separate PR. #1858 now rebases directly on top of #1856.

@joker23 joker23 closed this Aug 18, 2026
@joker23
joker23 deleted the skz/SDK-2912/fdv1-fallback-fix-recovery-orchestration branch August 18, 2026 21:03
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