diff --git a/packages/shared/sdk-client/__tests__/datasource/fdv2/FDv2DataSource.test.ts b/packages/shared/sdk-client/__tests__/datasource/fdv2/FDv2DataSource.test.ts index 815ec9261f..285ee0833f 100644 --- a/packages/shared/sdk-client/__tests__/datasource/fdv2/FDv2DataSource.test.ts +++ b/packages/shared/sdk-client/__tests__/datasource/fdv2/FDv2DataSource.test.ts @@ -1180,3 +1180,49 @@ it('stops initializer chain when a transfer-none changeSet triggers fdv1 fallbac expect(dataCallback).toHaveBeenCalledWith(fdv1Payload); ds.close(); }); + +// -- FDv1 fallback re-trigger guard (regression: SDK-2617) -- + +it('does not re-trigger fallback when the fdv1 synchronizer itself yields a fallback-flagged result', async () => { + const dataCallback = jest.fn(); + const statusManager = makeStatusManager(); + const logger = makeLogger(); + + const fdv2Payload = makePayload({ state: 'fdv2-selector' }); + const fdv1PayloadA = makePayload({ state: 'fdv1-a' }); + const fdv1PayloadB = makePayload({ state: 'fdv1-b' }); + + let fdv2Created = 0; + const fdv2Factory = jest.fn(() => { + fdv2Created += 1; + return makeMockSynchronizer([changeSet(fdv2Payload, { fdv1Fallback: true, fdv1FallbackTtlMs: 0 })]); + }); + const fdv1Sync = makeMockSynchronizer([ + changeSet(fdv1PayloadA, { fdv1Fallback: true }), + changeSet(fdv1PayloadB, { fdv1Fallback: false }), + ]); + + const slots: SynchronizerSlot[] = [ + createSynchronizerSlot({ create: fdv2Factory }), + createSynchronizerSlot({ create: () => fdv1Sync }, { isFDv1Fallback: true }), + ]; + + const ds = createFDv2DataSource({ + initializerFactories: [], + synchronizerSlots: slots, + dataCallback, + statusManager, + selectorGetter: noSelector, + logger, + }); + + await ds.start(); + + await statusManager.waitForState('VALID', 3); + expect(dataCallback).toHaveBeenCalledWith(fdv1PayloadA); + expect(dataCallback).toHaveBeenCalledWith(fdv1PayloadB); + expect(fdv2Created).toBe(1); + + ds.close(); +}); + diff --git a/packages/shared/sdk-client/__tests__/datasource/fdv2/SourceManager.test.ts b/packages/shared/sdk-client/__tests__/datasource/fdv2/SourceManager.test.ts index dc68443467..bb5bfb3247 100644 --- a/packages/shared/sdk-client/__tests__/datasource/fdv2/SourceManager.test.ts +++ b/packages/shared/sdk-client/__tests__/datasource/fdv2/SourceManager.test.ts @@ -452,3 +452,59 @@ it('close prevents further gets', () => { expect(manager.getNextInitializerAndSetActive()).toBeUndefined(); expect(manager.getNextAvailableSynchronizerAndSetActive()).toBeUndefined(); }); + +// -- fdv2Recovery and isCurrentSynchronizerFDv1Fallback -- + +it('fdv2Recovery blocks FDv1 slots and unblocks non-FDv1 slots', () => { + const fdv2Factory = { create: jest.fn(() => ({ next: jest.fn(), close: jest.fn() })) }; + const fdv1Factory = { create: jest.fn(() => ({ next: jest.fn(), close: jest.fn() })) }; + + const slots: SynchronizerSlot[] = [ + createSynchronizerSlot(fdv2Factory), + createSynchronizerSlot(fdv1Factory, { isFDv1Fallback: true }), + ]; + + const sm = createSourceManager([], slots, () => undefined); + + // Engage FDv1 fallback first + sm.fdv1Fallback(); + expect(slots[0].state).toBe('blocked'); + expect(slots[1].state).toBe('available'); + + // Recover: FDv2 unblocked, FDv1 blocked + sm.fdv2Recovery(); + expect(slots[0].state).toBe('available'); + expect(slots[1].state).toBe('blocked'); +}); + +it('fdv2Recovery resets the synchronizer index so the next selection starts from FDv2', () => { + const fdv2Factory = { create: jest.fn(() => ({ next: jest.fn(), close: jest.fn() })) }; + const fdv1Factory = { create: jest.fn(() => ({ next: jest.fn(), close: jest.fn() })) }; + + const slots: SynchronizerSlot[] = [ + createSynchronizerSlot(fdv2Factory), + createSynchronizerSlot(fdv1Factory, { isFDv1Fallback: true }), + ]; + + const sm = createSourceManager([], slots, () => undefined); + sm.fdv1Fallback(); + + // Advance into the FDv1 slot + sm.getNextAvailableSynchronizerAndSetActive(); + expect(sm.isCurrentSynchronizerFDv1Fallback).toBe(true); + + // After recovery the FDv2 slot is first + sm.fdv2Recovery(); + const next = sm.getNextAvailableSynchronizerAndSetActive(); + expect(next).toBeDefined(); + expect(sm.isCurrentSynchronizerFDv1Fallback).toBe(false); +}); + +it('isCurrentSynchronizerFDv1Fallback returns false when synchronizer index is -1 (before first selection)', () => { + const slots: SynchronizerSlot[] = [ + createSynchronizerSlot({ create: jest.fn() }, { isFDv1Fallback: true }), + ]; + const sm = createSourceManager([], slots, () => undefined); + // No selection made yet (index = -1) + expect(sm.isCurrentSynchronizerFDv1Fallback).toBe(false); +}); diff --git a/packages/shared/sdk-client/src/datasource/fdv2/Conditions.ts b/packages/shared/sdk-client/src/datasource/fdv2/Conditions.ts index 8891ddee6f..b7bb9f53d5 100644 --- a/packages/shared/sdk-client/src/datasource/fdv2/Conditions.ts +++ b/packages/shared/sdk-client/src/datasource/fdv2/Conditions.ts @@ -23,7 +23,7 @@ function conditionTimer(timeoutMs: number, type: ConditionType, taskName: string const timed = cancelableTimedPromise(timeoutMs / 1000, taskName); return { promise: timed.promise.then( - () => new Promise(() => {}), // cancelled — never settle + () => new Promise(() => {}), // cancelled - never settle () => type, // timeout fired ), cancel: timed.cancel, @@ -109,6 +109,8 @@ function createCondition( }); function startTimer() { + // idempotent: a fallback condition calls start() on every interrupted status, + // so a timer already running must not be restarted by a later one if (!timer && !closed) { timer = conditionTimer(timeoutMs, type, `${type} condition`); timer.promise.then((t) => { @@ -123,7 +125,7 @@ function createCondition( timer = undefined; } - // No inform handler — start immediately (recovery behavior). + // No inform handler - start immediately (recovery behavior) if (!informHandler) { startTimer(); } @@ -139,6 +141,8 @@ function createCondition( }, close() { + // left unresolved on purpose: getConditions() always builds a + // fresh group for the next iteration, so no caller is ever left waiting on this closed = true; cancelTimer(); }, diff --git a/packages/shared/sdk-client/src/datasource/fdv2/FDv2DataSource.ts b/packages/shared/sdk-client/src/datasource/fdv2/FDv2DataSource.ts index fba90989a7..76fa2fbac0 100644 --- a/packages/shared/sdk-client/src/datasource/fdv2/FDv2DataSource.ts +++ b/packages/shared/sdk-client/src/datasource/fdv2/FDv2DataSource.ts @@ -132,6 +132,11 @@ export function createFDv2DataSource(config: FDv2DataSourceConfig): FDv2DataSour } function handleFdv1Fallback(result: FDv2SourceResult): boolean { + // Guard: if the FDv1 fallback synchronizer itself produces a result flagged + // fdv1Fallback, do not re-run the fallback machinery - we are already on FDv1. + if (sourceManager.isCurrentSynchronizerFDv1Fallback) { + return false; + } if (result.fdv1Fallback && sourceManager.hasFDv1Fallback()) { sourceManager.fdv1Fallback(); return true; @@ -255,7 +260,7 @@ export function createFDv2DataSource(config: FDv2DataSourceConfig): FDv2DataSour logger?.debug('Fallback condition active for current synchronizer.'); } - // try/finally ensures conditions are closed on all code paths. + // Conditions hold timers; close them even if the inner loop throws or breaks early. let synchronizerRunning = true; try { while (!closed && synchronizerRunning) { diff --git a/packages/shared/sdk-client/src/datasource/fdv2/SourceManager.ts b/packages/shared/sdk-client/src/datasource/fdv2/SourceManager.ts index 30019800d8..14dfd65b75 100644 --- a/packages/shared/sdk-client/src/datasource/fdv2/SourceManager.ts +++ b/packages/shared/sdk-client/src/datasource/fdv2/SourceManager.ts @@ -102,6 +102,16 @@ export interface SourceManager { /** Block all non-FDv1 synchronizers and unblock FDv1 synchronizers. */ fdv1Fallback(): void; + /** + * Reverses {@link fdv1Fallback}: blocks all FDv1 fallback synchronizers and + * unblocks non-FDv1 synchronizers, then resets the synchronizer index so the + * next selection starts from the primary FDv2 synchronizer. + */ + fdv2Recovery(): void; + + /** True if the currently active synchronizer slot is an FDv1 fallback. */ + readonly isCurrentSynchronizerFDv1Fallback: boolean; + /** True if the current synchronizer is the first available (primary). */ isPrimeSynchronizer(): boolean; @@ -213,6 +223,17 @@ export function createSourceManager( synchronizerIndex = -1; }, + fdv2Recovery() { + synchronizerSlots.forEach((slot) => { + slot.state = slot.isFDv1Fallback ? 'blocked' : 'available'; + }); + synchronizerIndex = -1; + }, + + get isCurrentSynchronizerFDv1Fallback(): boolean { + return synchronizerSlots[synchronizerIndex]?.isFDv1Fallback === true; + }, + isPrimeSynchronizer(): boolean { return synchronizerIndex === findFirstAvailableIndex(); },