feat: Add FDv1 polling synchronizer for FDv2 fallback (SDK-1923)#1159
feat: Add FDv1 polling synchronizer for FDv2 fallback (SDK-1923)#1159
Conversation
Add a standalone FDv1 polling synchronizer that natively implements the Synchronizer interface, enabling the FDv2 orchestrator to fall back to polling the FDv1 endpoint when the server signals x-ld-fd-fallback. Also fix a potential clock-drift issue in both PollingSynchronizer and FDv1PollingSynchronizer where a backwards system clock adjustment could cause sleep durations to exceed the configured poll interval.
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/js-client-sdk-common size report |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/shared/sdk-client/src/datasource/fdv2/FDv1PollingSynchronizer.ts
Show resolved
Hide resolved
| const updates: internal.Update[] = Object.entries(flags).map(([key, flag]) => ({ | ||
| kind: 'flag', | ||
| key, | ||
| version: flag.version ?? 1, |
There was a problem hiding this comment.
@kinyoklion can you confirm which version is used at this level?
There was a problem hiding this comment.
We need to basically disable the version handling entirely in the FDv2 path. But maybe we can just do that all at once? The typing may prove a problem.
| function scheduleNextPoll(startTime: number): void { | ||
| if (!stopped) { | ||
| const elapsed = Date.now() - startTime; | ||
| const sleepFor = Math.min(Math.max(pollIntervalMs - elapsed, 0), pollIntervalMs); |
There was a problem hiding this comment.
I had it add this little bit here since we were talking about time drift, to ensure the biggest interval for polling is [0, pollingIntervalMs].
| if (!stopped) { | ||
| const sleepFor = Math.max(pollIntervalMs - (Date.now() - startTime), 0); | ||
| const elapsed = Date.now() - startTime; | ||
| const sleepFor = Math.min(Math.max(pollIntervalMs - elapsed, 0), pollIntervalMs); |
There was a problem hiding this comment.
And then had it apply it here as well since presumably it's problematic in both.
| version: 1, | ||
| // The selector MUST be empty — a non-empty selector would cause FDv2 | ||
| // synchronizers to try to resume from a bogus state. | ||
| state: '', |
There was a problem hiding this comment.
This is optional, so we should not include it.
The state field on Payload is optional, so we should not include it rather than setting it to an empty string.
Move flagsToPayload inside the try block with JSON.parse so that non-object JSON (null, arrays, primitives) that fails in Object.entries is correctly reported as INVALID_DATA rather than falling through to the network error handler.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes 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.
| next(): Promise<FDv2SourceResult> { | ||
| if (!started) { | ||
| started = true; | ||
| doPoll(); |
There was a problem hiding this comment.
Unhandled promise rejection from fire-and-forget doPoll
Low Severity
doPoll() is called as fire-and-forget inside next() without a .catch() handler. While the body of doPoll has try/catch around the main logic, the logger?.debug(...) call on line 87 and Date.now() on line 88 execute before the try block. If the logger's debug method throws, the resulting promise rejection would be unhandled. Adding a no-op .catch() would guard against this.
2 participants
Add a standalone FDv1 polling synchronizer that natively implements the
Synchronizer interface, enabling the FDv2 orchestrator to fall back to
polling the FDv1 endpoint when the server signals x-ld-fd-fallback.
Also fix a potential clock-drift issue in both PollingSynchronizer and
FDv1PollingSynchronizer where a backwards system clock adjustment could
cause sleep durations to exceed the configured poll interval.
Note
Medium Risk
Adds a new synchronizer implementation and adjusts polling delay logic, which can affect how/when flag updates and error states are delivered. Risk is moderated by being largely additive and covered by new unit tests, but timing/state-machine regressions are still possible.
Overview
Adds a new
createFDv1PollingSynchronizerthat polls the FDv1 flags endpoint but emits FDv2Synchronizerresults, enabling FDv2 orchestration to fall back to FDv1 polling while still producingchangeSet/statusevents.The synchronizer starts polling lazily on first
next(), converts the full FDv1 flag JSON into a FDv2internal.Payload(type: 'full', idFDv1Fallback), and maps failures to interrupted vs terminal_error based on HTTP recoverability (plus invalid JSON/network errors).Also clamps the computed sleep delay in both
PollingSynchronizerand the new FDv1 poller to avoid oversized waits when the system clock moves backward, exports the new synchronizer fromfdv2/index.ts, and adds comprehensive Jest coverage for polling cadence, shutdown semantics, and error handling.Written by Cursor Bugbot for commit 37f0dd2. This will update automatically on new commits. Configure here.