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

packages/shared/common/__tests__/internal/fdv2/fallbackDirective.test.ts

@@ -1,6 +1,8 @@
 import {
+  DEFAULT_FDV1_FALLBACK_TTL_MS,
   readFallbackDirective,
   readGoodbyeFallbackDirective,
+  resolveFallbackTtlMs,
 } from '../../../src/internal/fdv2/fallbackDirective';
 
 function makeHeaders(map: Record<string, string>): { get(name: string): string | null } {
@@ -29,10 +31,11 @@ it('matches "true" case-insensitively', () => {
   expect(result.fdv1Fallback).toBe(true);
 });
 
-it('returns fdv1Fallback true with undefined TTL when x-ld-fd-fallback-ttl is absent', () => {
+it('applies the jittered default TTL when x-ld-fd-fallback-ttl is absent', () => {
   const result = readFallbackDirective(makeHeaders({ 'x-ld-fd-fallback': 'true' }));
   expect(result.fdv1Fallback).toBe(true);
-  expect(result.fdv1FallbackTtlMs).toBeUndefined();
+  expect(result.fdv1FallbackTtlMs).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+  expect(result.fdv1FallbackTtlMs).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
 });
 
 it('converts a TTL of "60" seconds to 60000 ms', () => {
@@ -43,28 +46,46 @@ it('converts a TTL of "60" seconds to 60000 ms', () => {
   expect(result.fdv1FallbackTtlMs).toBe(60000);
 });
 
-it('converts TTL "0" to 0 ms (indefinite fallback)', () => {
+it('applies the default TTL for a TTL of "0"', () => {
   const result = readFallbackDirective(
     makeHeaders({ 'x-ld-fd-fallback': 'true', 'x-ld-fd-fallback-ttl': '0' }),
   );
   expect(result.fdv1Fallback).toBe(true);
-  expect(result.fdv1FallbackTtlMs).toBe(0);
+  expect(result.fdv1FallbackTtlMs).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+  expect(result.fdv1FallbackTtlMs).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
 });
 
-it('returns undefined TTL for a non-numeric x-ld-fd-fallback-ttl value', () => {
+it('applies the default TTL for a non-numeric x-ld-fd-fallback-ttl value', () => {
   const result = readFallbackDirective(
     makeHeaders({ 'x-ld-fd-fallback': 'true', 'x-ld-fd-fallback-ttl': 'soon' }),
   );
   expect(result.fdv1Fallback).toBe(true);
-  expect(result.fdv1FallbackTtlMs).toBeUndefined();
+  expect(result.fdv1FallbackTtlMs).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+  expect(result.fdv1FallbackTtlMs).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
 });
 
-it('clamps negative TTL seconds to 0 ms (treated as indefinite)', () => {
+it('applies the default TTL for a negative TTL', () => {
   const result = readFallbackDirective(
     makeHeaders({ 'x-ld-fd-fallback': 'true', 'x-ld-fd-fallback-ttl': '-5' }),
   );
   expect(result.fdv1Fallback).toBe(true);
-  expect(result.fdv1FallbackTtlMs).toBe(0);
+  expect(result.fdv1FallbackTtlMs).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+  expect(result.fdv1FallbackTtlMs).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('accepts a TTL of exactly one hour', () => {
+  const result = readFallbackDirective(
+    makeHeaders({ 'x-ld-fd-fallback': 'true', 'x-ld-fd-fallback-ttl': '3600' }),
+  );
+  expect(result.fdv1FallbackTtlMs).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('applies the default TTL for a TTL longer than one hour', () => {
+  const result = readFallbackDirective(
+    makeHeaders({ 'x-ld-fd-fallback': 'true', 'x-ld-fd-fallback-ttl': '7200' }),
+  );
+  expect(result.fdv1FallbackTtlMs).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+  expect(result.fdv1FallbackTtlMs).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
 });
 
 it('header lookup is case-insensitive', () => {
@@ -101,16 +122,45 @@ it('readGoodbyeFallbackDirective: converts a protocolFallbackTTL of 60 seconds t
   expect(result.fdv1FallbackTtlMs).toBe(60000);
 });
 
-it('readGoodbyeFallbackDirective: converts protocolFallbackTTL 0 to 0 ms (indefinite fallback)', () => {
+it('readGoodbyeFallbackDirective: applies the default TTL for a protocolFallbackTTL of 0', () => {
   const result = readGoodbyeFallbackDirective({ reason: 'falling back', protocolFallbackTTL: 0 });
   expect(result.fdv1Fallback).toBe(true);
-  expect(result.fdv1FallbackTtlMs).toBe(0);
+  expect(result.fdv1FallbackTtlMs).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+  expect(result.fdv1FallbackTtlMs).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
 });
 
-it('readGoodbyeFallbackDirective: clamps negative protocolFallbackTTL to 0 ms', () => {
+it('readGoodbyeFallbackDirective: applies the default TTL for a negative protocolFallbackTTL', () => {
   const result = readGoodbyeFallbackDirective({ reason: 'falling back', protocolFallbackTTL: -5 });
   expect(result.fdv1Fallback).toBe(true);
-  expect(result.fdv1FallbackTtlMs).toBe(0);
+  expect(result.fdv1FallbackTtlMs).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+  expect(result.fdv1FallbackTtlMs).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('readGoodbyeFallbackDirective: accepts a protocolFallbackTTL of exactly one hour', () => {
+  const result = readGoodbyeFallbackDirective({
+    reason: 'falling back',
+    protocolFallbackTTL: 3600,
+  });
+  expect(result.fdv1FallbackTtlMs).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('readGoodbyeFallbackDirective: applies the default TTL for a protocolFallbackTTL over one hour', () => {
+  const result = readGoodbyeFallbackDirective({
+    reason: 'falling back',
+    protocolFallbackTTL: 7200,
+  });
+  expect(result.fdv1FallbackTtlMs).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+  expect(result.fdv1FallbackTtlMs).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('readGoodbyeFallbackDirective: truncates a fractional protocolFallbackTTL to whole seconds before the range check', () => {
+  const result = readGoodbyeFallbackDirective({
+    reason: 'falling back',
+    protocolFallbackTTL: 0.001,
+  });
+  expect(result.fdv1Fallback).toBe(true);
+  expect(result.fdv1FallbackTtlMs).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+  expect(result.fdv1FallbackTtlMs).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
 });
 
 it('readGoodbyeFallbackDirective: returns fdv1Fallback false for a non-numeric protocolFallbackTTL', () => {
@@ -130,3 +180,44 @@ it('readGoodbyeFallbackDirective: returns fdv1Fallback false for a non-finite pr
   expect(result.fdv1Fallback).toBe(false);
   expect(result.fdv1FallbackTtlMs).toBeUndefined();
 });
+
+it('resolveFallbackTtlMs: converts a whole number of seconds to milliseconds without jitter', () => {
+  expect(resolveFallbackTtlMs(60, () => 1)).toBe(60000);
+});
+
+it('resolveFallbackTtlMs: accepts a TTL of exactly one hour unchanged', () => {
+  expect(resolveFallbackTtlMs(3600, () => 1)).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('resolveFallbackTtlMs: uses the default for a TTL greater than one hour', () => {
+  expect(resolveFallbackTtlMs(3601, () => 0)).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('resolveFallbackTtlMs: uses the default for a TTL of zero', () => {
+  expect(resolveFallbackTtlMs(0, () => 0)).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('resolveFallbackTtlMs: uses the default for a negative TTL', () => {
+  expect(resolveFallbackTtlMs(-5, () => 0)).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('resolveFallbackTtlMs: uses the default for an absent TTL', () => {
+  expect(resolveFallbackTtlMs(undefined, () => 0)).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('resolveFallbackTtlMs: uses the default for a TTL that is not a number', () => {
+  expect(resolveFallbackTtlMs(NaN, () => 0)).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS);
+});
+
+it('resolveFallbackTtlMs: subtracts jitter of up to half the default TTL', () => {
+  expect(resolveFallbackTtlMs(undefined, () => 0.5)).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS * 0.75);
+  expect(resolveFallbackTtlMs(undefined, () => 1)).toBe(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+});
+
+it('resolveFallbackTtlMs: defaults to Math.random for jitter and stays within bounds', () => {
+  for (let i = 0; i < 50; i += 1) {
+    const ttl = resolveFallbackTtlMs(undefined);
+    expect(ttl).toBeGreaterThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS / 2);
+    expect(ttl).toBeLessThanOrEqual(DEFAULT_FDV1_FALLBACK_TTL_MS);
+  }
+});

packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts

@@ -595,9 +595,11 @@ it('falls back to FDv1 from initializer phase, skipping remaining initializers a
 });
 
 // When the FDv1 Fallback Directive arrives but the SDK has not been configured with an
-// FDv1 fallback synchronizer, the data source must transition to a terminal Closed state
-// instead of looping or trying other FDv2 sources.
-it('terminates when FDv1 fallback is requested but no FDv1 fallback synchronizer is configured', async () => {
+// FDv1 fallback synchronizer, the data source must wait out the fallback TTL and then
+// retry FDv2 rather than terminating.
+it('retries FDv2 after the fallback TTL when no FDv1 fallback synchronizer is configured', async () => {
+  const fdv2Data = { key: 'fdv2-data' };
+  let fdv2StartCount = 0;
   const mockSynchronizer1 = {
     start: jest
       .fn()
@@ -606,15 +608,22 @@ it('terminates when FDv1 fallback is requested but no FDv1 fallback synchronizer
           _dataCallback: (basis: boolean, data: any) => void,
           _statusCallback: (status: DataSourceState, err?: any) => void,
         ) => {
+          fdv2StartCount += 1;
           _statusCallback(DataSourceState.Initializing);
-          _statusCallback(
-            DataSourceState.Closed,
-            new LDFlagDeliveryFallbackError(
-              DataSourceErrorKind.ErrorResponse,
-              `Response header indicates to fallback to FDv1`,
-              500,
-            ),
-          );
+          if (fdv2StartCount === 1) {
+            _statusCallback(
+              DataSourceState.Closed,
+              new LDFlagDeliveryFallbackError(
+                DataSourceErrorKind.ErrorResponse,
+                `Response header indicates to fallback to FDv1`,
+                500,
+                1,
+              ),
+            );
+          } else {
+            _statusCallback(DataSourceState.Valid);
+            _dataCallback(true, fdv2Data);
+          }
         },
       ),
     stop: jest.fn(),
@@ -632,22 +641,84 @@ it('terminates when FDv1 fallback is requested but no FDv1 fallback synchronizer
   const statusCallback = jest.fn();
   const dataCallback = jest.fn();
   await new Promise<void>((resolve) => {
-    statusCallback.mockImplementation((state: DataSourceState) => {
-      if (state === DataSourceState.Closed) {
+    dataCallback.mockImplementation((_: boolean, data: any) => {
+      if (data === fdv2Data) {
         resolve();
       }
     });
     underTest.start(dataCallback, statusCallback);
   });
 
-  // The underlying synchronizer was started exactly once -- we did not loop trying other
-  // FDv2 sources after the directive arrived.
-  expect(mockSynchronizer1.start).toHaveBeenCalledTimes(1);
-  expect(dataCallback).not.toHaveBeenCalled();
+  expect(mockSynchronizer1.start).toHaveBeenCalledTimes(2);
+  expect(statusCallback).not.toHaveBeenCalledWith(DataSourceState.Closed, expect.anything());
 
-  // The terminal status is Closed (composite signals exhaustion when the FDv1 list is empty).
-  const lastCall = statusCallback.mock.calls[statusCallback.mock.calls.length - 1];
-  expect(lastCall[0]).toBe(DataSourceState.Closed);
+  await underTest.stop();
+});
+
+it('stops the FDv1 synchronizer before restoring FDv2 synchronizers after the fallback TTL', async () => {
+  const events: string[] = [];
+  const fdv2Data = { key: 'fdv2-data' };
+  let fdv2StartCount = 0;
+  const mockFDv2Sync = {
+    start: jest
+      .fn()
+      .mockImplementation(
+        (
+          _dataCallback: (basis: boolean, data: any) => void,
+          _statusCallback: (status: DataSourceState, err?: any) => void,
+        ) => {
+          fdv2StartCount += 1;
+          events.push('fdv2 start');
+          _statusCallback(DataSourceState.Initializing);
+          _statusCallback(DataSourceState.Valid);
+          if (fdv2StartCount === 1) {
+            _dataCallback(true, { fallbackToFDv1: true, fdv1FallbackTtlMs: 1 });
+          } else {
+            _dataCallback(true, fdv2Data);
+          }
+        },
+      ),
+    stop: jest.fn(() => events.push('fdv2 stop')),
+  };
+
+  const mockFDv1Synchronizer = {
+    start: jest
+      .fn()
+      .mockImplementation(
+        (
+          _dataCallback: (basis: boolean, data: any) => void,
+          _statusCallback: (status: DataSourceState, err?: any) => void,
+        ) => {
+          events.push('fdv1 start');
+          _statusCallback(DataSourceState.Initializing);
+          _statusCallback(DataSourceState.Valid);
+          _dataCallback(true, { key: 'fdv1-data' });
+        },
+      ),
+    stop: jest.fn(() => events.push('fdv1 stop')),
+  };
+
+  const underTest = new CompositeDataSource(
+    [],
+    [makeDataSourceFactory(mockFDv2Sync)],
+    [makeDataSourceFactory(mockFDv1Synchronizer)],
+    undefined,
+    makeTestTransitionConditions(),
+    makeZeroBackoff(),
+  );
+
+  await new Promise<void>((resolve) => {
+    const dataCallback = jest.fn((_: boolean, data: any) => {
+      if (data === fdv2Data) {
+        resolve();
+      }
+    });
+    underTest.start(dataCallback, jest.fn());
+  });
+
+  expect(events).toEqual(['fdv2 start', 'fdv2 stop', 'fdv1 start', 'fdv1 stop', 'fdv2 start']);
+
+  await underTest.stop();
 });
 
 // When an initializer delivers basis with `data.fallbackToFDv1`, the composite must apply
@@ -903,7 +974,10 @@ it('throttles FDv1 retries with backoff after the initial engagement', async ()
   // Two FDv1 factories so the composite cycles through 'fallback' transitions instead
   // of stopping after the first cull -- each cycle is a chance to (incorrectly) skip
   // backoff. The tracking Backoff below counts fail() calls as the observable proxy.
-  const makeFDv1Sync = () => ({
+  // The first FDv1 factory fails, so the composite retries with the second -- that retry
+  // is a chance to (incorrectly) skip backoff. The tracking Backoff below counts fail()
+  // calls as the observable proxy.
+  const mockFDv1Sync1 = {
     start: jest
       .fn()
       .mockImplementation(
@@ -923,9 +997,22 @@ it('throttles FDv1 retries with backoff after the initial engagement', async ()
         },
       ),
     stop: jest.fn(),
-  });
-  const mockFDv1Sync1 = makeFDv1Sync();
-  const mockFDv1Sync2 = makeFDv1Sync();
+  };
+
+  const mockFDv1Sync2 = {
+    start: jest
+      .fn()
+      .mockImplementation(
+        (
+          _dataCallback: (basis: boolean, data: any) => void,
+          _statusCallback: (status: DataSourceState, err?: any) => void,
+        ) => {
+          _statusCallback(DataSourceState.Initializing);
+          _statusCallback(DataSourceState.Valid);
+        },
+      ),
+    stop: jest.fn(),
+  };
 
   let failCalls = 0;
   let successCalls = 0;
@@ -952,7 +1039,7 @@ it('throttles FDv1 retries with backoff after the initial engagement', async ()
   const statusCallback = jest.fn();
   await new Promise<void>((resolve) => {
     statusCallback.mockImplementation((state: DataSourceState) => {
-      if (state === DataSourceState.Closed) {
+      if (state === DataSourceState.Valid) {
         resolve();
       }
     });
@@ -963,8 +1050,10 @@ it('throttles FDv1 retries with backoff after the initial engagement', async ()
   expect(mockFDv1Sync1.start).toHaveBeenCalledTimes(1);
   expect(mockFDv1Sync2.start).toHaveBeenCalledTimes(1);
   // One fail() per post-engagement retry; the initial engagement is not counted.
-  expect(failCalls).toBe(2);
+  expect(failCalls).toBe(1);
   expect(successCalls).toBe(0);
+
+  await underTest.stop();
 });
 
 it('reports error when all initializers fail', async () => {