From 285fc9fd225a6bf8e007df95ee08135032b2537c Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 7 Feb 2025 15:56:33 -0600 Subject: [PATCH 01/14] feat: adds CompositeDataSource for FDv2 --- .../DataSystem/CompositeDataSource.test.ts | 208 ++++++++++++++++++ .../subsystem/DataSystem/CallbackHandler.ts | 93 ++++++++ .../DataSystem/CompositeDataSource.ts | 153 +++++++++++++ .../api/subsystem/DataSystem/DataSource.ts | 35 +++ .../DataSystem/DataSystemInitializer.ts | 24 ++ .../DataSystem/DataSystemSynchronizer.ts | 24 ++ .../src/api/subsystem/DataSystem/index.ts | 3 + .../sdk-server/src/options/Configuration.ts | 9 +- 8 files changed, 545 insertions(+), 4 deletions(-) create mode 100644 packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts create mode 100644 packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts create mode 100644 packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts create mode 100644 packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts create mode 100644 packages/shared/common/src/api/subsystem/DataSystem/DataSystemInitializer.ts create mode 100644 packages/shared/common/src/api/subsystem/DataSystem/DataSystemSynchronizer.ts create mode 100644 packages/shared/common/src/api/subsystem/DataSystem/index.ts diff --git a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts new file mode 100644 index 0000000000..bb4f9ef49e --- /dev/null +++ b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts @@ -0,0 +1,208 @@ +import { CompositeDataSource } from '../../../src/api/subsystem/DataSystem/CompositeDataSource'; +import { Data, HealthStatus } from '../../../src/api/subsystem/DataSystem/DataSource'; +import { + DataSystemInitializer, + InitializerFactory, +} from '../../../src/api/subsystem/DataSystem/DataSystemInitializer'; +import { + DataSystemSynchronizer, + SynchronizerFactory, +} from '../../../src/api/subsystem/DataSystem/DataSystemSynchronizer'; + +function makeInitializerFactory(internal: DataSystemInitializer): InitializerFactory { + return { + create: () => internal, + }; +} + +function makeSynchronizerFactory(internal: DataSystemSynchronizer): SynchronizerFactory { + return { + create: () => internal, + }; +} + +it('initializer gets basis, switch to syncrhonizer', async () => { + const mockInitializer1 = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: HealthStatus, durationMS: number) => void, + _errorHander: (err: Error) => void, + ) => { + _dataCallback(true, { key: 'init1' }); + }, + ), + stop: jest.fn(), + }; + + const mockSynchronizer1 = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: HealthStatus, durationMS: number) => void, + _errorHander: (err: Error) => void, + ) => { + _dataCallback(false, { key: 'sync1' }); + }, + ), + stop: jest.fn(), + }; + + const underTest = new CompositeDataSource( + [makeInitializerFactory(mockInitializer1)], + [makeSynchronizerFactory(mockSynchronizer1)], + ); + const callback = jest.fn(); + underTest.run(callback, jest.fn()); + + // pause so scheduler can resolve awaits + await new Promise((f) => { + setTimeout(f, 1); + }); + + expect(mockInitializer1.run).toHaveBeenCalledTimes(1); + expect(mockSynchronizer1.run).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledTimes(2); + expect(callback).toHaveBeenNthCalledWith(1, true, { key: 'init1' }); + expect(callback).toHaveBeenNthCalledWith(2, false, { key: 'sync1' }); +}); + +it('initializer gets basis, switch to synchronizer 1, fallback to synchronizer 2, recover to synchronizer 1', async () => { + const mockInitializer1: DataSystemInitializer = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: HealthStatus, durationMS: number) => void, + _errorHander: (err: Error) => void, + ) => { + _dataCallback(true, { key: 'init1' }); + }, + ), + stop: jest.fn(), + }; + + let sync1RunCount = 0; + const mockSynchronizer1 = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: HealthStatus, durationMS: number) => void, + _errorHander: (err: Error) => void, + ) => { + if (sync1RunCount === 0) { + _errorHander({ name: 'Error', message: 'I am error...man!' }); // error that will lead to fallback + } else { + _dataCallback(false, { key: 'sync1' }); // second run will lead to data + } + sync1RunCount += 1; + }, + ), + stop: jest.fn(), + }; + + const mockSynchronizer2 = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: HealthStatus, durationMS: number) => void, + _errorHander: (err: Error) => void, + ) => { + _dataCallback(false, { key: 'sync2' }); + _statusCallback(HealthStatus.Online, Number.MAX_VALUE); // this should lead to recovery + }, + ), + stop: jest.fn(), + }; + + const underTest = new CompositeDataSource( + [makeInitializerFactory(mockInitializer1)], + [makeSynchronizerFactory(mockSynchronizer1), makeSynchronizerFactory(mockSynchronizer2)], + ); + const callback = jest.fn(); + underTest.run(callback, jest.fn()); + + // pause so scheduler can resolve awaits + await new Promise((f) => { + setTimeout(f, 1); + }); + expect(mockInitializer1.run).toHaveBeenCalledTimes(1); + expect(mockSynchronizer1.run).toHaveBeenCalledTimes(2); + expect(mockSynchronizer2.run).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledTimes(3); + expect(callback).toHaveBeenNthCalledWith(1, true, { key: 'init1' }); + expect(callback).toHaveBeenNthCalledWith(2, false, { key: 'sync2' }); // sync1 errors and fallsback + expect(callback).toHaveBeenNthCalledWith(3, false, { key: 'sync1' }); // sync2 recovers back to sync1 +}); + +it('it reports error when all initializers fail', async () => { + const mockInitializer1: DataSystemInitializer = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: HealthStatus, durationMS: number) => void, + _errorHander: (err: Error) => void, + ) => { + _errorHander({ name: 'Error', message: 'I am initializer1 error!' }); + }, + ), + stop: jest.fn(), + }; + + const mockInitializer2: DataSystemInitializer = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: HealthStatus, durationMS: number) => void, + _errorHander: (err: Error) => void, + ) => { + _errorHander({ name: 'Error', message: 'I am initializer2 error!' }); + }, + ), + stop: jest.fn(), + }; + + const underTest = new CompositeDataSource( + [makeInitializerFactory(mockInitializer1), makeInitializerFactory(mockInitializer2)], + [], // no synchronizers for this test + ); + + const dataCallback = jest.fn(); + const errorCallback = jest.fn(); + underTest.run(dataCallback, errorCallback); + + // pause so scheduler can resolve awaits + await new Promise((f) => { + setTimeout(f, 1); + }); + + expect(mockInitializer1.run).toHaveBeenCalledTimes(1); + expect(mockInitializer2.run).toHaveBeenCalledTimes(1); + expect(dataCallback).toHaveBeenCalledTimes(0); + expect(errorCallback).toHaveBeenCalledTimes(3); + expect(errorCallback).toHaveBeenNthCalledWith(1, { + name: 'Error', + message: 'I am initializer1 error!', + }); + expect(errorCallback).toHaveBeenNthCalledWith(2, { + name: 'Error', + message: 'I am initializer2 error!', + }); + expect(errorCallback).toHaveBeenNthCalledWith(3, { + name: 'ExhaustedDataSources', + message: 'CompositeDataSource has exhausted all configured datasources.', + }); +}); diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts new file mode 100644 index 0000000000..29212222df --- /dev/null +++ b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts @@ -0,0 +1,93 @@ +import { Data, HealthStatus } from './DataSource'; + +/** + * Represents a transition between data sources. + */ +export enum Transition { + /** + * Transition from current data source to the first synchronizer. + */ + SwitchToSync, + + /** + * Transition to the next data source of the same kind. + */ + Fallback, + + /** + * Transition to the first data source of the same kind. + */ + Recover, + + /** + * A no-op transition. + */ + None, +} + +/** + * Evaluated to determine if a transition should occur. + */ +export type TransitionCondition = (status: HealthStatus, durationMS: number) => boolean; + +/** + * Handler that connects the current {@link DataSource} to the {@link CompositeDataSource}. A single + * {@link CallbackHandler} should only be given to one {@link DataSource}. Once an instance of + * a {@link CallbackHandler} triggers a transition, it will disable itself so that future invocatons + * on it are no-op. + */ +export class CallbackHandler { + private _disabled: boolean = false; + + constructor( + private readonly _dataCallback: (basis: boolean, data: Data) => void, + private readonly _errorCallback: (err: any) => void, + private readonly _triggerTransition: (value: Transition | PromiseLike) => void, + private readonly _isInitializer: boolean, + private readonly _recoveryCondition: (status: HealthStatus, durationMS: number) => boolean, + private readonly _fallbackCondition: (status: HealthStatus, durationMS: number) => boolean, + ) {} + + dataHanlder = async (basis: boolean, data: Data) => { + if (this._disabled) { + return; + } + + // report data up + this._dataCallback(basis, data); + + // TODO: SDK-1044 track selector for future synchronizer to use + if (basis && this._isInitializer) { + this._disabled = true; // getting basis means this initializer has done its job, time to move on to sync! + this._triggerTransition(Transition.SwitchToSync); + } + }; + + statusHandler = async (status: HealthStatus, durationMS: number) => { + if (this._disabled) { + return; + } + + if (this._recoveryCondition(status, durationMS)) { + this._disabled = true; + this._triggerTransition(Transition.Recover); + } else if (this._fallbackCondition(status, durationMS)) { + this._disabled = true; + this._triggerTransition(Transition.Fallback); + } + }; + + errorHandler = async (err: any) => { + // TODO: unrecoverable error handling + if (this._disabled) { + return; + } + this._disabled = true; + + // TODO: should this error be reported or contained silently if we have a fallback? + // report error up, discuss with others on team. + this._errorCallback(err); + + this._triggerTransition(Transition.Fallback); + }; +} diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts new file mode 100644 index 0000000000..132b3f7095 --- /dev/null +++ b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts @@ -0,0 +1,153 @@ +import { CallbackHandler, Transition, TransitionCondition } from './CallbackHandler'; +import { Data, DataSource, HealthStatus } from './DataSource'; +import { DataSystemInitializer, InitializerFactory } from './DataSystemInitializer'; +import { DataSystemSynchronizer, SynchronizerFactory } from './DataSystemSynchronizer'; + +/** + * The {@link CompositeDataSource} can combine a number of {@link DataSystemInitializer}s and {@link DataSystemSynchronizer}s + * into a single {@link DataSource}, implementing fallback and recovery logic internally to choose where data is sourced from. + */ +export class CompositeDataSource implements DataSource { + // TODO: SDK-856 async notification if initializer takes too long + // TODO: SDK-1044 utilize selector from initializers + + // TODO: add datasource status APIs + + private readonly _defaultFallbackTimeMs = 2 * 60 * 1000; + private readonly _defaultRecoveryTimeMs = 5 * 60 * 1000; + + private _initPhaseActive: boolean = true; + private _currentPosition: number = 0; + + private _stopped: boolean = true; + private _externalStopPromise: Promise; + private _externalStopResolve?: (value: Transition) => void; + + /** + * @param _initializers factories to create {@link DataSystemInitializer}s, in priority order. + * @param _synchronizers factories to create {@link DataSystemSynchronizer}s, in priority order. + */ + constructor( + private readonly _initializers: InitializerFactory[], + private readonly _synchronizers: SynchronizerFactory[], + ) { + this._externalStopPromise = new Promise((transition) => { + this._externalStopResolve = transition; + }); + this._initPhaseActive = true; + this._currentPosition = 0; + } + + async run( + dataCallback: (basis: boolean, data: Data) => void, + errorCallback: (err: Error) => void, + ): Promise { + if (!this._stopped) { + // don't allow multiple simultaneous runs + return; + } + this._stopped = false; + + let transition: Transition = Transition.None; // first loop has no transition + while (!this._stopped) { + const current: DataSystemInitializer | DataSystemSynchronizer | undefined = + this._pickDataSource(transition); + if (current === undefined) { + errorCallback({ + name: 'ExhaustedDataSources', + message: 'CompositeDataSource has exhausted all configured datasources.', + }); + return; + } + + const internalTransitionPromise = new Promise((transitionResolve) => { + const recoveryCondition = this._makeRecoveryCondition(); + const fallbackCondition = this._makeFallbackCondition(); + const callbackHandler = new CallbackHandler( + dataCallback, + errorCallback, + transitionResolve, + this._initPhaseActive, + recoveryCondition, + fallbackCondition, + ); + current.run( + callbackHandler.dataHanlder, + callbackHandler.statusHandler, + callbackHandler.errorHandler, + ); + }); + + // await transition triggered by internal data source or an external stop request + // eslint-disable-next-line no-await-in-loop + transition = await Promise.race([internalTransitionPromise, this._externalStopPromise]); + + // stop the current datasource before transitioning to next state + current.stop(); + } + + // reset so that run can be called again in the future + this._reset(); + } + + async stop() { + this._stopped = true; + this._externalStopResolve?.(Transition.None); // TODO: this feels a little hacky. + } + + private _reset() { + this._initPhaseActive = true; + this._currentPosition = 0; + this._externalStopPromise = new Promise((transition) => { + this._externalStopResolve = transition; + }); + } + + private _pickDataSource( + transition: Transition | undefined, + ): DataSystemInitializer | DataSystemSynchronizer | undefined { + switch (transition) { + case Transition.SwitchToSync: + this._initPhaseActive = false; // one way toggle to false, unless this class is reset() + this._currentPosition = 0; + break; + case Transition.Fallback: + this._currentPosition += 1; + break; + case Transition.Recover: + this._currentPosition = 0; + break; + case Transition.None: + default: + // don't do anything in this case + break; + } + + if (this._initPhaseActive) { + // if outside range of initializers, don't loop back to start, instead return undefined + if (this._currentPosition > this._initializers.length - 1) { + return undefined; + } + + return this._initializers[this._currentPosition].create(); + } + + // getting here indicates we are using a synchronizer + this._currentPosition %= this._synchronizers.length; // modulate position to loop back to start + if (this._currentPosition > this._synchronizers.length - 1) { + // this is only possible if no synchronizers were provided + return undefined; + } + return this._synchronizers[this._currentPosition].create(); + } + + private _makeFallbackCondition(): TransitionCondition { + return (status: HealthStatus, durationMS: number) => + status === HealthStatus.Interrupted && durationMS >= this._defaultFallbackTimeMs; + } + + private _makeRecoveryCondition(): TransitionCondition { + return (status: HealthStatus, durationMS: number) => + status === HealthStatus.Online && durationMS >= this._defaultRecoveryTimeMs; + } +} diff --git a/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts new file mode 100644 index 0000000000..3e80f7324a --- /dev/null +++ b/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts @@ -0,0 +1,35 @@ +export interface Data {} + +export enum HealthStatus { + Online, + Interrupted, +} + +export interface DataSource { + /** + * May be called any number of times, if already started, has no effect + * @param cb may be invoked many times + * @returns + */ + run(dataCallback: (basis: boolean, data: Data) => void, errorHander: (err: Error) => void): void; + + /** + * May be called any number of times, if already stopped, has no effect. + * @param cb + * @returns + */ + stop(): void; +} + +export interface DataSourceWithStatus { + /** + * May be called any number of times, if already started, has no effect + * @param cb may be invoked many times + * @returns + */ + run( + dataCallback: (basis: boolean, data: Data) => void, + statusCallback: (status: HealthStatus, durationMS: number) => void, + errorHander: (err: any) => void, + ): void; +} diff --git a/packages/shared/common/src/api/subsystem/DataSystem/DataSystemInitializer.ts b/packages/shared/common/src/api/subsystem/DataSystem/DataSystemInitializer.ts new file mode 100644 index 0000000000..4d53b79bd5 --- /dev/null +++ b/packages/shared/common/src/api/subsystem/DataSystem/DataSystemInitializer.ts @@ -0,0 +1,24 @@ +import { Data, HealthStatus } from './DataSource'; + +/** + * Will make best effort to retrieve all data. Data recieved will be reported via the {@link dataCallback}. Status changes + * will be reported via the {@link statusCallback}. Errors will be reported via the {@link errorCallback}. + */ +export interface DataSystemInitializer { + run( + dataCallback: (basis: boolean, data: Data) => void, + statusCallback: (status: HealthStatus, durationMS: number) => void, + errorCallback: (err: Error) => void, + ): void; + + /** + * May be called any number of times, if already stopped, has no effect. + * @param cb + * @returns + */ + stop(): void; +} + +export interface InitializerFactory { + create(): DataSystemInitializer; +} diff --git a/packages/shared/common/src/api/subsystem/DataSystem/DataSystemSynchronizer.ts b/packages/shared/common/src/api/subsystem/DataSystem/DataSystemSynchronizer.ts new file mode 100644 index 0000000000..09babe13ea --- /dev/null +++ b/packages/shared/common/src/api/subsystem/DataSystem/DataSystemSynchronizer.ts @@ -0,0 +1,24 @@ +import { Data, HealthStatus } from './DataSource'; + +/** + * Will make best effort to retrieve data. Data recieved will be reported via the {@link dataCallback}. Status changes + * will be reported via the {@link statusCallback}. Errors will be reported via the {@link errorCallback}. + */ +export interface DataSystemSynchronizer { + run( + dataCallback: (basis: boolean, data: Data) => void, + statusCallback: (status: HealthStatus, durationMS: number) => void, + errorCallback: (err: Error) => void, + ): void; + + /** + * May be called any number of times, if already stopped, has no effect. + * @param cb + * @returns + */ + stop(): void; +} + +export interface SynchronizerFactory { + create(): DataSystemSynchronizer; +} diff --git a/packages/shared/common/src/api/subsystem/DataSystem/index.ts b/packages/shared/common/src/api/subsystem/DataSystem/index.ts new file mode 100644 index 0000000000..8cedfee2a4 --- /dev/null +++ b/packages/shared/common/src/api/subsystem/DataSystem/index.ts @@ -0,0 +1,3 @@ +export { DataSystemInitializer, InitializerFactory } from './DataSystemInitializer'; +export { DataSystemSynchronizer, SynchronizerFactory } from './DataSystemSynchronizer'; +export { CompositeDataSource } from './CompositeDataSource'; diff --git a/packages/shared/sdk-server/src/options/Configuration.ts b/packages/shared/sdk-server/src/options/Configuration.ts index 77baf0de39..fe14a43f53 100644 --- a/packages/shared/sdk-server/src/options/Configuration.ts +++ b/packages/shared/sdk-server/src/options/Configuration.ts @@ -247,10 +247,6 @@ export default class Configuration { this.pollInterval = validatedOptions.pollInterval; this.proxyOptions = validatedOptions.proxyOptions; - this.offline = validatedOptions.offline; - this.stream = validatedOptions.stream; - this.streamInitialReconnectDelay = validatedOptions.streamInitialReconnectDelay; - this.useLdd = validatedOptions.useLdd; this.sendEvents = validatedOptions.sendEvents; this.allAttributesPrivate = validatedOptions.allAttributesPrivate; this.privateAttributes = validatedOptions.privateAttributes; @@ -264,6 +260,11 @@ export default class Configuration { this.tags = new ApplicationTags(validatedOptions); this.diagnosticRecordingInterval = validatedOptions.diagnosticRecordingInterval; + this.offline = validatedOptions.offline; + this.stream = validatedOptions.stream; + this.streamInitialReconnectDelay = validatedOptions.streamInitialReconnectDelay; + this.useLdd = validatedOptions.useLdd; + if (TypeValidators.Function.is(validatedOptions.updateProcessor)) { // @ts-ignore this.updateProcessorFactory = validatedOptions.updateProcessor; From 3e9953b4a8b5f66f50a122a4000021e34c3f2c02 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 21 Feb 2025 12:51:57 -0600 Subject: [PATCH 02/14] implementing status based scheduled transitions and backoff support --- .../src/platform/DefaultBrowserEventSource.ts | 2 +- packages/sdk/browser/tsconfig.json | 2 +- .../DataSystem/CompositeDataSource.test.ts | 348 +++++++++++++++--- .../subsystem/DataSystem/CallbackHandler.ts | 78 +--- .../DataSystem/CompositeDataSource.ts | 228 +++++++++--- .../api/subsystem/DataSystem/DataSource.ts | 39 +- .../DataSystem/DataSystemInitializer.ts | 24 -- .../DataSystem/DataSystemSynchronizer.ts | 24 -- .../common/src/datasource}/Backoff.ts | 0 9 files changed, 511 insertions(+), 234 deletions(-) delete mode 100644 packages/shared/common/src/api/subsystem/DataSystem/DataSystemInitializer.ts delete mode 100644 packages/shared/common/src/api/subsystem/DataSystem/DataSystemSynchronizer.ts rename packages/{sdk/browser/src/platform => shared/common/src/datasource}/Backoff.ts (100%) diff --git a/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts b/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts index 3ecdeb3a12..4d7af58fe4 100644 --- a/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts +++ b/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts @@ -6,7 +6,7 @@ import { EventSource as LDEventSource, } from '@launchdarkly/js-client-sdk-common'; -import Backoff from './Backoff'; +import Backoff from '../../../../shared/common/src/datasource/Backoff'; /** * Implementation Notes: diff --git a/packages/sdk/browser/tsconfig.json b/packages/sdk/browser/tsconfig.json index 7306c5b0c6..82f7f65361 100644 --- a/packages/sdk/browser/tsconfig.json +++ b/packages/sdk/browser/tsconfig.json @@ -18,7 +18,7 @@ "types": ["node", "jest"], "allowJs": true }, - "include": ["src"], + "include": ["src", "../../shared/common/src/datasource/Backoff.ts"], "exclude": [ "vite.config.ts", "__tests__", diff --git a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts index bb4f9ef49e..89097461ab 100644 --- a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts +++ b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts @@ -1,13 +1,16 @@ -import { CompositeDataSource } from '../../../src/api/subsystem/DataSystem/CompositeDataSource'; -import { Data, HealthStatus } from '../../../src/api/subsystem/DataSystem/DataSource'; import { - DataSystemInitializer, - InitializerFactory, -} from '../../../src/api/subsystem/DataSystem/DataSystemInitializer'; + CompositeDataSource, + Transition, + TransitionConditions, +} from '../../../src/api/subsystem/DataSystem/CompositeDataSource'; import { + Data, + DataSourceState, + DataSystemInitializer, DataSystemSynchronizer, + InitializerFactory, SynchronizerFactory, -} from '../../../src/api/subsystem/DataSystem/DataSystemSynchronizer'; +} from '../../../src/api/subsystem/DataSystem/DataSource'; function makeInitializerFactory(internal: DataSystemInitializer): InitializerFactory { return { @@ -21,6 +24,27 @@ function makeSynchronizerFactory(internal: DataSystemSynchronizer): Synchronizer }; } +function makeTestTransitionConditions(): TransitionConditions { + return { + [DataSourceState.Initializing]: { + durationMS: 10000, + transition: Transition.Fallback, + }, + [DataSourceState.Interrupted]: { + durationMS: 10000, + transition: Transition.Fallback, + }, + [DataSourceState.Closed]: { + durationMS: 10000, + transition: Transition.Fallback, + }, + [DataSourceState.Valid]: { + durationMS: 10000, + transition: Transition.Fallback, + }, + }; +} + it('initializer gets basis, switch to syncrhonizer', async () => { const mockInitializer1 = { run: jest @@ -28,8 +52,7 @@ it('initializer gets basis, switch to syncrhonizer', async () => { .mockImplementation( ( _dataCallback: (basis: boolean, data: Data) => void, - _statusCallback: (status: HealthStatus, durationMS: number) => void, - _errorHander: (err: Error) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(true, { key: 'init1' }); }, @@ -37,16 +60,16 @@ it('initializer gets basis, switch to syncrhonizer', async () => { stop: jest.fn(), }; + const mockSynchronizer1Data = { key: 'sync1' }; const mockSynchronizer1 = { run: jest .fn() .mockImplementation( ( _dataCallback: (basis: boolean, data: Data) => void, - _statusCallback: (status: HealthStatus, durationMS: number) => void, - _errorHander: (err: Error) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, ) => { - _dataCallback(false, { key: 'sync1' }); + _dataCallback(false, mockSynchronizer1Data); }, ), stop: jest.fn(), @@ -55,13 +78,20 @@ it('initializer gets basis, switch to syncrhonizer', async () => { const underTest = new CompositeDataSource( [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1)], + makeTestTransitionConditions(), + 0, + 0, ); - const callback = jest.fn(); - underTest.run(callback, jest.fn()); - // pause so scheduler can resolve awaits - await new Promise((f) => { - setTimeout(f, 1); + let callback; + await new Promise((resolve) => { + callback = jest.fn((_: boolean, data: Data) => { + if (data === mockSynchronizer1Data) { + resolve(); + } + }); + + underTest.run(callback, jest.fn()); }); expect(mockInitializer1.run).toHaveBeenCalledTimes(1); @@ -78,8 +108,7 @@ it('initializer gets basis, switch to synchronizer 1, fallback to synchronizer 2 .mockImplementation( ( _dataCallback: (basis: boolean, data: Data) => void, - _statusCallback: (status: HealthStatus, durationMS: number) => void, - _errorHander: (err: Error) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(true, { key: 'init1' }); }, @@ -88,19 +117,22 @@ it('initializer gets basis, switch to synchronizer 1, fallback to synchronizer 2 }; let sync1RunCount = 0; + const mockSynchronizer1Data = { key: 'sync1' }; const mockSynchronizer1 = { run: jest .fn() .mockImplementation( ( _dataCallback: (basis: boolean, data: Data) => void, - _statusCallback: (status: HealthStatus, durationMS: number) => void, - _errorHander: (err: Error) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, ) => { if (sync1RunCount === 0) { - _errorHander({ name: 'Error', message: 'I am error...man!' }); // error that will lead to fallback + _statusCallback(DataSourceState.Closed, { + name: 'Error', + message: 'I am error...man!', + }); // error that will lead to fallback } else { - _dataCallback(false, { key: 'sync1' }); // second run will lead to data + _dataCallback(false, mockSynchronizer1Data); // second run will lead to data } sync1RunCount += 1; }, @@ -108,17 +140,17 @@ it('initializer gets basis, switch to synchronizer 1, fallback to synchronizer 2 stop: jest.fn(), }; + const mockSynchronizer2Data = { key: 'sync2' }; const mockSynchronizer2 = { run: jest .fn() .mockImplementation( ( _dataCallback: (basis: boolean, data: Data) => void, - _statusCallback: (status: HealthStatus, durationMS: number) => void, - _errorHander: (err: Error) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, ) => { - _dataCallback(false, { key: 'sync2' }); - _statusCallback(HealthStatus.Online, Number.MAX_VALUE); // this should lead to recovery + _dataCallback(false, mockSynchronizer2Data); + _statusCallback(DataSourceState.Valid, null); // this should lead to recovery }, ), stop: jest.fn(), @@ -127,14 +159,22 @@ it('initializer gets basis, switch to synchronizer 1, fallback to synchronizer 2 const underTest = new CompositeDataSource( [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1), makeSynchronizerFactory(mockSynchronizer2)], + makeTestTransitionConditions(), + 0, + 0, ); - const callback = jest.fn(); - underTest.run(callback, jest.fn()); - // pause so scheduler can resolve awaits - await new Promise((f) => { - setTimeout(f, 1); + let callback; + await new Promise((resolve) => { + callback = jest.fn((_: boolean, data: Data) => { + if (data === mockSynchronizer1Data) { + resolve(); + } + }); + + underTest.run(callback, jest.fn()); }); + expect(mockInitializer1.run).toHaveBeenCalledTimes(1); expect(mockSynchronizer1.run).toHaveBeenCalledTimes(2); expect(mockSynchronizer2.run).toHaveBeenCalledTimes(1); @@ -145,31 +185,37 @@ it('initializer gets basis, switch to synchronizer 1, fallback to synchronizer 2 }); it('it reports error when all initializers fail', async () => { + const mockInitializer1Error = { + name: 'Error', + message: 'I am initializer1 error!', + }; const mockInitializer1: DataSystemInitializer = { run: jest .fn() .mockImplementation( ( _dataCallback: (basis: boolean, data: Data) => void, - _statusCallback: (status: HealthStatus, durationMS: number) => void, - _errorHander: (err: Error) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, ) => { - _errorHander({ name: 'Error', message: 'I am initializer1 error!' }); + _statusCallback(DataSourceState.Closed, mockInitializer1Error); }, ), stop: jest.fn(), }; + const mockInitializer2Error = { + name: 'Error', + message: 'I am initializer2 error!', + }; const mockInitializer2: DataSystemInitializer = { run: jest .fn() .mockImplementation( ( _dataCallback: (basis: boolean, data: Data) => void, - _statusCallback: (status: HealthStatus, durationMS: number) => void, - _errorHander: (err: Error) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, ) => { - _errorHander({ name: 'Error', message: 'I am initializer2 error!' }); + _statusCallback(DataSourceState.Closed, mockInitializer2Error); }, ), stop: jest.fn(), @@ -178,31 +224,231 @@ it('it reports error when all initializers fail', async () => { const underTest = new CompositeDataSource( [makeInitializerFactory(mockInitializer1), makeInitializerFactory(mockInitializer2)], [], // no synchronizers for this test + makeTestTransitionConditions(), + 0, + 0, ); const dataCallback = jest.fn(); - const errorCallback = jest.fn(); - underTest.run(dataCallback, errorCallback); + let statusCallback; + await new Promise((resolve) => { + statusCallback = jest.fn((_: DataSourceState, err?: any) => { + if (err?.name === 'ExhaustedDataSources') { + resolve(); + } + }); - // pause so scheduler can resolve awaits - await new Promise((f) => { - setTimeout(f, 1); + underTest.run(dataCallback, statusCallback); }); expect(mockInitializer1.run).toHaveBeenCalledTimes(1); expect(mockInitializer2.run).toHaveBeenCalledTimes(1); expect(dataCallback).toHaveBeenCalledTimes(0); - expect(errorCallback).toHaveBeenCalledTimes(3); - expect(errorCallback).toHaveBeenNthCalledWith(1, { - name: 'Error', - message: 'I am initializer1 error!', + expect(statusCallback).toHaveBeenCalledTimes(3); + expect(statusCallback).toHaveBeenNthCalledWith(1, DataSourceState.Interrupted, null); + expect(statusCallback).toHaveBeenNthCalledWith(2, DataSourceState.Interrupted, null); + expect(statusCallback).toHaveBeenNthCalledWith(3, DataSourceState.Closed, { + name: 'ExhaustedDataSources', + message: + 'CompositeDataSource has exhausted all configured datasources (2 initializers, 0 synchronizers).', }); - expect(errorCallback).toHaveBeenNthCalledWith(2, { - name: 'Error', - message: 'I am initializer2 error!', +}); + +it('it can be stopped when in thrashing synchronizer fallback loop', async () => { + const mockInitializer1 = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, + ) => { + _dataCallback(true, { key: 'init1' }); + }, + ), + stop: jest.fn(), + }; + + const mockSynchronizer1 = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, + ) => { + _statusCallback(DataSourceState.Closed, { name: 'Error', message: 'I am error...man!' }); // error that will lead to fallback + }, + ), + stop: jest.fn(), + }; + + const underTest = new CompositeDataSource( + [makeInitializerFactory(mockInitializer1)], + [makeSynchronizerFactory(mockSynchronizer1)], // will continuously fallback onto itself + makeTestTransitionConditions(), + 0, + 0, + ); + + const dataCallback = jest.fn(); + let statusCallback; + await new Promise((resolve) => { + let statusCount = 0; + statusCallback = jest.fn((_1: DataSourceState, _2: any) => { + statusCount += 1; + if (statusCount >= 5) { + resolve(); + } + }); + + underTest.run(dataCallback, statusCallback); }); - expect(errorCallback).toHaveBeenNthCalledWith(3, { + + expect(mockInitializer1.run).toHaveBeenCalled(); + expect(mockSynchronizer1.run).toHaveBeenCalled(); + expect(dataCallback).toHaveBeenNthCalledWith(1, true, { key: 'init1' }); + + // wait for trashing to occur + await new Promise((f) => { + setTimeout(f, 1); + }).then(() => underTest.stop()); + + // wait for stop to take effect before checking status is closed + await new Promise((f) => { + setTimeout(f, 1); + }); + + expect(statusCallback).toHaveBeenLastCalledWith(DataSourceState.Closed, null); +}); + +it('it can be stopped and restarted', async () => { + const mockInitializer1Data = { key: 'init1' }; + const mockInitializer1 = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, + ) => { + _dataCallback(true, mockInitializer1Data); + }, + ), + stop: jest.fn(), + }; + + const mockSynchronizer1Data = { key: 'sync1' }; + const mockSynchronizer1 = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, + ) => { + _dataCallback(false, mockSynchronizer1Data); + }, + ), + stop: jest.fn(), + }; + + const underTest = new CompositeDataSource( + [makeInitializerFactory(mockInitializer1)], + [makeSynchronizerFactory(mockSynchronizer1)], + makeTestTransitionConditions(), + 0, + 0, + ); + + let callback1; + await new Promise((resolve) => { + callback1 = jest.fn((_: boolean, data: Data) => { + if (data === mockSynchronizer1Data) { + underTest.stop(); + resolve(); + } + }); + // first run + underTest.run(callback1, jest.fn()); + }); + + // check first run triggered underlying data sources + expect(mockInitializer1.run).toHaveBeenCalledTimes(1); + expect(mockSynchronizer1.run).toHaveBeenCalledTimes(1); + expect(callback1).toHaveBeenCalledTimes(2); + + let callback2; + await new Promise((resolve) => { + callback2 = jest.fn((_: boolean, data: Data) => { + if (data === mockSynchronizer1Data) { + resolve(); + } + }); + // second run + underTest.run(callback2, jest.fn()); + }); + + // check that second run triggers underlying data sources again + expect(mockInitializer1.run).toHaveBeenCalledTimes(2); + expect(mockSynchronizer1.run).toHaveBeenCalledTimes(2); + expect(callback2).toHaveBeenCalledTimes(4); +}); + +it('it is well behaved with no initializers and no synchronizers configured', async () => { + const underTest = new CompositeDataSource([], [], makeTestTransitionConditions(), 0, 0); + + let statusCallback; + await new Promise((resolve) => { + statusCallback = jest.fn((_1: DataSourceState, _2: any) => { + resolve(); + }); + + underTest.run(jest.fn(), statusCallback); + }); + + expect(statusCallback).toHaveBeenNthCalledWith(1, DataSourceState.Closed, { + name: 'ExhaustedDataSources', + message: + 'CompositeDataSource has exhausted all configured datasources (0 initializers, 0 synchronizers).', + }); +}); + +it('it is well behaved with an initializer and no synchronizers configured', async () => { + const mockInitializer1 = { + run: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: Data) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, + ) => { + _dataCallback(true, { key: 'init1' }); + }, + ), + stop: jest.fn(), + }; + + const underTest = new CompositeDataSource( + [makeInitializerFactory(mockInitializer1)], + [], + makeTestTransitionConditions(), + 0, + 0, + ); + + let statusCallback; + await new Promise((resolve) => { + statusCallback = jest.fn((_1: DataSourceState, _2: any) => { + resolve(); + }); + + underTest.run(jest.fn(), statusCallback); + }); + + expect(statusCallback).toHaveBeenNthCalledWith(1, DataSourceState.Closed, { name: 'ExhaustedDataSources', - message: 'CompositeDataSource has exhausted all configured datasources.', + message: + 'CompositeDataSource has exhausted all configured datasources (1 initializers, 0 synchronizers).', }); }); diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts index 29212222df..7ad446ddcd 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts @@ -1,93 +1,37 @@ -import { Data, HealthStatus } from './DataSource'; - -/** - * Represents a transition between data sources. - */ -export enum Transition { - /** - * Transition from current data source to the first synchronizer. - */ - SwitchToSync, - - /** - * Transition to the next data source of the same kind. - */ - Fallback, - - /** - * Transition to the first data source of the same kind. - */ - Recover, - - /** - * A no-op transition. - */ - None, -} - -/** - * Evaluated to determine if a transition should occur. - */ -export type TransitionCondition = (status: HealthStatus, durationMS: number) => boolean; +import { Data, DataSourceState } from './DataSource'; /** * Handler that connects the current {@link DataSource} to the {@link CompositeDataSource}. A single - * {@link CallbackHandler} should only be given to one {@link DataSource}. Once an instance of - * a {@link CallbackHandler} triggers a transition, it will disable itself so that future invocatons - * on it are no-op. + * {@link CallbackHandler} should only be given to one {@link DataSource}. Use {@link disable()} to + * prevent additional callback events. */ export class CallbackHandler { private _disabled: boolean = false; constructor( private readonly _dataCallback: (basis: boolean, data: Data) => void, - private readonly _errorCallback: (err: any) => void, - private readonly _triggerTransition: (value: Transition | PromiseLike) => void, - private readonly _isInitializer: boolean, - private readonly _recoveryCondition: (status: HealthStatus, durationMS: number) => boolean, - private readonly _fallbackCondition: (status: HealthStatus, durationMS: number) => boolean, + private readonly _statusCallback: (status: DataSourceState, err?: any) => void, ) {} + disable() { + this._disabled = true; + } + dataHanlder = async (basis: boolean, data: Data) => { if (this._disabled) { return; } + // TODO: SDK-1044 track selector for future synchronizer to use // report data up this._dataCallback(basis, data); - - // TODO: SDK-1044 track selector for future synchronizer to use - if (basis && this._isInitializer) { - this._disabled = true; // getting basis means this initializer has done its job, time to move on to sync! - this._triggerTransition(Transition.SwitchToSync); - } - }; - - statusHandler = async (status: HealthStatus, durationMS: number) => { - if (this._disabled) { - return; - } - - if (this._recoveryCondition(status, durationMS)) { - this._disabled = true; - this._triggerTransition(Transition.Recover); - } else if (this._fallbackCondition(status, durationMS)) { - this._disabled = true; - this._triggerTransition(Transition.Fallback); - } }; - errorHandler = async (err: any) => { - // TODO: unrecoverable error handling + statusHandler = async (status: DataSourceState, err?: any) => { if (this._disabled) { return; } - this._disabled = true; - - // TODO: should this error be reported or contained silently if we have a fallback? - // report error up, discuss with others on team. - this._errorCallback(err); - this._triggerTransition(Transition.Fallback); + this._statusCallback(status, err); }; } diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts index 132b3f7095..73e71d6c9e 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts @@ -1,7 +1,49 @@ -import { CallbackHandler, Transition, TransitionCondition } from './CallbackHandler'; -import { Data, DataSource, HealthStatus } from './DataSource'; -import { DataSystemInitializer, InitializerFactory } from './DataSystemInitializer'; -import { DataSystemSynchronizer, SynchronizerFactory } from './DataSystemSynchronizer'; +/* eslint-disable no-await-in-loop */ +import Backoff from '../../../datasource/Backoff'; +import { CallbackHandler } from './CallbackHandler'; +import { + Data, + DataSource, + DataSourceState, + InitializerFactory, + SynchronizerFactory, +} from './DataSource'; + +/** + * Represents a transition between data sources. + */ +export enum Transition { + /** + * A no-op transition. + */ + None, + /** + * Transition from current data source to the first synchronizer. + */ + SwitchToSync, + + /** + * Transition to the next data source of the same kind. + */ + Fallback, + + /** + * Transition to the first data source of the same kind. + */ + Recover, +} + +/** + * Given a {@link DataSourceState}, how long to wait before transitioning. + */ +export type TransitionConditions = { + [k in DataSourceState]?: { durationMS: number; transition: Transition }; +}; + +interface TransitionRequest { + transition: Transition; + err?: Error; +} /** * The {@link CompositeDataSource} can combine a number of {@link DataSystemInitializer}s and {@link DataSystemSynchronizer}s @@ -10,18 +52,16 @@ import { DataSystemSynchronizer, SynchronizerFactory } from './DataSystemSynchro export class CompositeDataSource implements DataSource { // TODO: SDK-856 async notification if initializer takes too long // TODO: SDK-1044 utilize selector from initializers - - // TODO: add datasource status APIs - private readonly _defaultFallbackTimeMs = 2 * 60 * 1000; private readonly _defaultRecoveryTimeMs = 5 * 60 * 1000; private _initPhaseActive: boolean = true; private _currentPosition: number = 0; + private _backoff: Backoff; private _stopped: boolean = true; - private _externalStopPromise: Promise; - private _externalStopResolve?: (value: Transition) => void; + private _externalStopPromise: Promise; + private _externalStopResolve?: (value: TransitionRequest) => void; /** * @param _initializers factories to create {@link DataSystemInitializer}s, in priority order. @@ -30,17 +70,21 @@ export class CompositeDataSource implements DataSource { constructor( private readonly _initializers: InitializerFactory[], private readonly _synchronizers: SynchronizerFactory[], + private readonly _transitionConditions: TransitionConditions, + initialRetryDelayMillis: number, + retryResetIntervalMillis: number, ) { - this._externalStopPromise = new Promise((transition) => { - this._externalStopResolve = transition; + this._externalStopPromise = new Promise((tr) => { + this._externalStopResolve = tr; }); this._initPhaseActive = true; this._currentPosition = 0; + this._backoff = new Backoff(initialRetryDelayMillis, retryResetIntervalMillis); } async run( dataCallback: (basis: boolean, data: Data) => void, - errorCallback: (err: Error) => void, + statusCallback: (status: DataSourceState, err?: any) => void, ): Promise { if (!this._stopped) { // don't allow multiple simultaneous runs @@ -48,42 +92,91 @@ export class CompositeDataSource implements DataSource { } this._stopped = false; - let transition: Transition = Transition.None; // first loop has no transition - while (!this._stopped) { - const current: DataSystemInitializer | DataSystemSynchronizer | undefined = - this._pickDataSource(transition); + let lastTransition: Transition = Transition.None; + // eslint-disable-next-line no-constant-condition + while (true) { + if (this._stopped) { + // report we are closed, no error as this was due to stop breaking the loop + statusCallback(DataSourceState.Closed, null); + break; + } + + const current: DataSource | undefined = this._pickDataSource(lastTransition); if (current === undefined) { - errorCallback({ + statusCallback(DataSourceState.Closed, { name: 'ExhaustedDataSources', - message: 'CompositeDataSource has exhausted all configured datasources.', + message: `CompositeDataSource has exhausted all configured datasources (${this._initializers.length} initializers, ${this._synchronizers.length} synchronizers).`, }); - return; + break; } - const internalTransitionPromise = new Promise((transitionResolve) => { - const recoveryCondition = this._makeRecoveryCondition(); - const fallbackCondition = this._makeFallbackCondition(); + const internalTransitionPromise = new Promise((transitionResolve) => { + // these local variables are used for handling automatic transition related to data source status (ex: recovering to primary after + // secondary has been valid for N many minutes) + let lastState: DataSourceState | undefined; + let cancelScheduledTransition: (() => void) | undefined; + const callbackHandler = new CallbackHandler( - dataCallback, - errorCallback, - transitionResolve, - this._initPhaseActive, - recoveryCondition, - fallbackCondition, - ); - current.run( - callbackHandler.dataHanlder, - callbackHandler.statusHandler, - callbackHandler.errorHandler, + (basis: boolean, data: Data) => { + dataCallback(basis, data); + if (basis && this._initPhaseActive) { + // transition to sync if we get basis during init + callbackHandler.disable(); + cancelScheduledTransition?.(); + transitionResolve({ transition: Transition.SwitchToSync }); + } + }, + (state: DataSourceState, err?: any) => { + // When we get a status update, we want to fallback if it is an error. We also want to schedule a transition for some + // time in the future if this status remains for some duration (ex: Recover to primary synchronizer after the secondary + // synchronizer has been Valid for some time). These scheduled transitions are configurable in the constructor. + + if (err || state === DataSourceState.Closed) { + callbackHandler.disable(); + statusCallback(DataSourceState.Interrupted, null); // underlying errors or closed states are masked as interrupted while we transition + cancelScheduledTransition?.(); + transitionResolve({ transition: Transition.Fallback, err }); // unrecoverable error has occurred, so fallback + } else { + if (state !== lastState) { + lastState = state; + cancelScheduledTransition?.(); // cancel previously scheduled status transition if one was scheduled + const excludeRecovery = this._currentPosition === 0; // primary source cannot recover to itself, so exclude it + const condition = this._lookupTransitionCondition(state, excludeRecovery); + if (condition) { + const { promise, cancel } = this._cancellableDelay(condition.durationMS); + cancelScheduledTransition = cancel; + promise.then(() => { + callbackHandler.disable(); + transitionResolve({ transition: condition.transition }); + }); + } else { + // this data source state does not have a transition condition, so don't schedule any transition + } + } + statusCallback(state, null); // report the status upward + } + }, ); + current.run(callbackHandler.dataHanlder, callbackHandler.statusHandler); }); // await transition triggered by internal data source or an external stop request - // eslint-disable-next-line no-await-in-loop - transition = await Promise.race([internalTransitionPromise, this._externalStopPromise]); + const transitionRequest = await Promise.race([ + internalTransitionPromise, + this._externalStopPromise, + ]); - // stop the current datasource before transitioning to next state + // if the transition was due to an error, throttle the transition + if (transitionRequest.err) { + const delay = this._backoff.fail(); + await new Promise((resolve) => { + setTimeout(resolve, delay); + }); + } + + // stop the underlying datasource before transitioning to next state current.stop(); + lastTransition = transitionRequest.transition; } // reset so that run can be called again in the future @@ -92,20 +185,19 @@ export class CompositeDataSource implements DataSource { async stop() { this._stopped = true; - this._externalStopResolve?.(Transition.None); // TODO: this feels a little hacky. + this._externalStopResolve?.({ transition: Transition.None }); // the value here doesn't matter, just needs to break the loop's await } private _reset() { this._initPhaseActive = true; this._currentPosition = 0; - this._externalStopPromise = new Promise((transition) => { - this._externalStopResolve = transition; + this._externalStopPromise = new Promise((tr) => { + this._externalStopResolve = tr; }); + // intentionally not resetting the backoff to avoid a code path that could circumvent throttling } - private _pickDataSource( - transition: Transition | undefined, - ): DataSystemInitializer | DataSystemSynchronizer | undefined { + private _pickDataSource(transition: Transition | undefined): DataSource | undefined { switch (transition) { case Transition.SwitchToSync: this._initPhaseActive = false; // one way toggle to false, unless this class is reset() @@ -124,16 +216,20 @@ export class CompositeDataSource implements DataSource { } if (this._initPhaseActive) { - // if outside range of initializers, don't loop back to start, instead return undefined + // We don't loop back through initializers, so if outside range of initializers, instead return undefined. if (this._currentPosition > this._initializers.length - 1) { return undefined; } return this._initializers[this._currentPosition].create(); } - // getting here indicates we are using a synchronizer - this._currentPosition %= this._synchronizers.length; // modulate position to loop back to start + + // if no synchronizers, return undefined + if (this._synchronizers.length <= 0) { + return undefined; + } + this._currentPosition %= this._synchronizers.length; // modulate position to loop back to start if necessary if (this._currentPosition > this._synchronizers.length - 1) { // this is only possible if no synchronizers were provided return undefined; @@ -141,13 +237,41 @@ export class CompositeDataSource implements DataSource { return this._synchronizers[this._currentPosition].create(); } - private _makeFallbackCondition(): TransitionCondition { - return (status: HealthStatus, durationMS: number) => - status === HealthStatus.Interrupted && durationMS >= this._defaultFallbackTimeMs; - } + /** + * @returns the transition condition for the provided data source state or undefined + * if there is no transition condition + */ + private _lookupTransitionCondition( + state: DataSourceState, + excludeRecover: boolean, + ): { durationMS: number; transition: Transition } | undefined { + const condition = this._transitionConditions[state]; - private _makeRecoveryCondition(): TransitionCondition { - return (status: HealthStatus, durationMS: number) => - status === HealthStatus.Online && durationMS >= this._defaultRecoveryTimeMs; + // exclude recovery can happen for certain initializers/synchronizers (ex: the primary synchronizer shouldn't recover to itself) + if (!condition || (excludeRecover && condition.transition === Transition.Recover)) { + return undefined; + } + + return condition; } + + private _cancellableDelay = (delayMS: number) => { + let timeout: ReturnType | undefined; + let reject: ((reason?: any) => void) | undefined; + const promise = new Promise((res, rej) => { + timeout = setTimeout(res, delayMS); + reject = rej; + }); + return { + promise, + cancel() { + if (timeout) { + clearTimeout(timeout); + reject?.(); + timeout = undefined; + reject = undefined; + } + }, + }; + }; } diff --git a/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts index 3e80f7324a..352d668f6b 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts @@ -1,8 +1,11 @@ export interface Data {} -export enum HealthStatus { - Online, +// TODO: refactor client-sdk to use this enum +export enum DataSourceState { + Initializing, + Valid, Interrupted, + Closed, } export interface DataSource { @@ -11,7 +14,10 @@ export interface DataSource { * @param cb may be invoked many times * @returns */ - run(dataCallback: (basis: boolean, data: Data) => void, errorHander: (err: Error) => void): void; + run( + dataCallback: (basis: boolean, data: Data) => void, + statusCallback: (status: DataSourceState, err?: any) => void, + ): void; /** * May be called any number of times, if already stopped, has no effect. @@ -21,15 +27,20 @@ export interface DataSource { stop(): void; } -export interface DataSourceWithStatus { - /** - * May be called any number of times, if already started, has no effect - * @param cb may be invoked many times - * @returns - */ - run( - dataCallback: (basis: boolean, data: Data) => void, - statusCallback: (status: HealthStatus, durationMS: number) => void, - errorHander: (err: any) => void, - ): void; +/** + * A data source that can be used to fetch the basis. + */ +export interface DataSystemInitializer extends DataSource {} + +/** + * A data source that can be used to fetch the basis or ongoing data changes. + */ +export interface DataSystemSynchronizer extends DataSource {} + +export interface InitializerFactory { + create(): DataSystemInitializer; +} + +export interface SynchronizerFactory { + create(): DataSystemSynchronizer; } diff --git a/packages/shared/common/src/api/subsystem/DataSystem/DataSystemInitializer.ts b/packages/shared/common/src/api/subsystem/DataSystem/DataSystemInitializer.ts deleted file mode 100644 index 4d53b79bd5..0000000000 --- a/packages/shared/common/src/api/subsystem/DataSystem/DataSystemInitializer.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { Data, HealthStatus } from './DataSource'; - -/** - * Will make best effort to retrieve all data. Data recieved will be reported via the {@link dataCallback}. Status changes - * will be reported via the {@link statusCallback}. Errors will be reported via the {@link errorCallback}. - */ -export interface DataSystemInitializer { - run( - dataCallback: (basis: boolean, data: Data) => void, - statusCallback: (status: HealthStatus, durationMS: number) => void, - errorCallback: (err: Error) => void, - ): void; - - /** - * May be called any number of times, if already stopped, has no effect. - * @param cb - * @returns - */ - stop(): void; -} - -export interface InitializerFactory { - create(): DataSystemInitializer; -} diff --git a/packages/shared/common/src/api/subsystem/DataSystem/DataSystemSynchronizer.ts b/packages/shared/common/src/api/subsystem/DataSystem/DataSystemSynchronizer.ts deleted file mode 100644 index 09babe13ea..0000000000 --- a/packages/shared/common/src/api/subsystem/DataSystem/DataSystemSynchronizer.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { Data, HealthStatus } from './DataSource'; - -/** - * Will make best effort to retrieve data. Data recieved will be reported via the {@link dataCallback}. Status changes - * will be reported via the {@link statusCallback}. Errors will be reported via the {@link errorCallback}. - */ -export interface DataSystemSynchronizer { - run( - dataCallback: (basis: boolean, data: Data) => void, - statusCallback: (status: HealthStatus, durationMS: number) => void, - errorCallback: (err: Error) => void, - ): void; - - /** - * May be called any number of times, if already stopped, has no effect. - * @param cb - * @returns - */ - stop(): void; -} - -export interface SynchronizerFactory { - create(): DataSystemSynchronizer; -} diff --git a/packages/sdk/browser/src/platform/Backoff.ts b/packages/shared/common/src/datasource/Backoff.ts similarity index 100% rename from packages/sdk/browser/src/platform/Backoff.ts rename to packages/shared/common/src/datasource/Backoff.ts From 80582bc267b126fa6e6bd3f5fdabb7e48f46f224 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Mon, 24 Feb 2025 15:18:04 -0600 Subject: [PATCH 03/14] Refactoring backoff and transitioning handling logic to handle more edge cases. Added tests. --- .../src/platform/DefaultBrowserEventSource.ts | 10 +- .../__tests__/datasource}/Backoff.test.ts | 18 +- .../DataSystem/CompositeDataSource.test.ts | 95 ++++++---- .../DataSystem/CompositeDataSource.ts | 174 ++++++++++-------- .../src/api/subsystem/DataSystem/index.ts | 8 +- .../shared/common/src/datasource/Backoff.ts | 7 +- .../shared/common/src/datasource/index.ts | 3 + packages/shared/common/src/index.ts | 4 + 8 files changed, 189 insertions(+), 130 deletions(-) rename packages/{sdk/browser/__tests__/platform => shared/common/__tests__/datasource}/Backoff.test.ts (79%) diff --git a/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts b/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts index 4d7af58fe4..742f1f1654 100644 --- a/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts +++ b/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts @@ -5,8 +5,7 @@ import { HttpErrorResponse, EventSource as LDEventSource, } from '@launchdarkly/js-client-sdk-common'; - -import Backoff from '../../../../shared/common/src/datasource/Backoff'; +import { DefaultBackoff } from '@launchdarkly/js-sdk-common'; /** * Implementation Notes: @@ -22,7 +21,7 @@ import Backoff from '../../../../shared/common/src/datasource/Backoff'; */ export default class DefaultBrowserEventSource implements LDEventSource { private _es?: EventSource; - private _backoff: Backoff; + private _backoff: DefaultBackoff; private _errorFilter: (err: HttpErrorResponse) => boolean; // The type of the handle can be platform specific and we treat is opaquely. @@ -34,7 +33,10 @@ export default class DefaultBrowserEventSource implements LDEventSource { private readonly _url: string, options: EventSourceInitDict, ) { - this._backoff = new Backoff(options.initialRetryDelayMillis, options.retryResetIntervalMillis); + this._backoff = new DefaultBackoff( + options.initialRetryDelayMillis, + options.retryResetIntervalMillis, + ); this._errorFilter = options.errorFilter; this._openConnection(); } diff --git a/packages/sdk/browser/__tests__/platform/Backoff.test.ts b/packages/shared/common/__tests__/datasource/Backoff.test.ts similarity index 79% rename from packages/sdk/browser/__tests__/platform/Backoff.test.ts rename to packages/shared/common/__tests__/datasource/Backoff.test.ts index 49cdd901cb..c4a0e25abf 100644 --- a/packages/sdk/browser/__tests__/platform/Backoff.test.ts +++ b/packages/shared/common/__tests__/datasource/Backoff.test.ts @@ -1,29 +1,29 @@ -import Backoff from '../../src/platform/Backoff'; +import DefaultBackoff from '../../src/datasource/Backoff'; const noJitter = (): number => 0; const maxJitter = (): number => 1; const defaultResetInterval = 60 * 1000; it.each([1, 1000, 5000])('has the correct starting delay', (initialDelay) => { - const backoff = new Backoff(initialDelay, defaultResetInterval, noJitter); + const backoff = new DefaultBackoff(initialDelay, defaultResetInterval, noJitter); expect(backoff.fail()).toEqual(initialDelay); }); it.each([1, 1000, 5000])('doubles delay on consecutive failures', (initialDelay) => { - const backoff = new Backoff(initialDelay, defaultResetInterval, noJitter); + const backoff = new DefaultBackoff(initialDelay, defaultResetInterval, noJitter); expect(backoff.fail()).toEqual(initialDelay); expect(backoff.fail()).toEqual(initialDelay * 2); expect(backoff.fail()).toEqual(initialDelay * 4); }); it('stops increasing delay when the max backoff is encountered', () => { - const backoff = new Backoff(5000, defaultResetInterval, noJitter); + const backoff = new DefaultBackoff(5000, defaultResetInterval, noJitter); expect(backoff.fail()).toEqual(5000); expect(backoff.fail()).toEqual(10000); expect(backoff.fail()).toEqual(20000); expect(backoff.fail()).toEqual(30000); - const backoff2 = new Backoff(1000, defaultResetInterval, noJitter); + const backoff2 = new DefaultBackoff(1000, defaultResetInterval, noJitter); expect(backoff2.fail()).toEqual(1000); expect(backoff2.fail()).toEqual(2000); expect(backoff2.fail()).toEqual(4000); @@ -33,12 +33,12 @@ it('stops increasing delay when the max backoff is encountered', () => { }); it('handles an initial retry delay longer than the maximum retry delay', () => { - const backoff = new Backoff(40000, defaultResetInterval, noJitter); + const backoff = new DefaultBackoff(40000, defaultResetInterval, noJitter); expect(backoff.fail()).toEqual(30000); }); it('jitters the backoff value', () => { - const backoff = new Backoff(1000, defaultResetInterval, maxJitter); + const backoff = new DefaultBackoff(1000, defaultResetInterval, maxJitter); expect(backoff.fail()).toEqual(500); expect(backoff.fail()).toEqual(1000); expect(backoff.fail()).toEqual(2000); @@ -51,7 +51,7 @@ it.each([10 * 1000, 60 * 1000])( 'resets the delay when the last successful connection was connected greater than the retry reset interval', (retryResetInterval) => { let time = 1000; - const backoff = new Backoff(1000, retryResetInterval, noJitter); + const backoff = new DefaultBackoff(1000, retryResetInterval, noJitter); expect(backoff.fail(time)).toEqual(1000); time += 1; backoff.success(time); @@ -69,7 +69,7 @@ it.each([10 * 1000, 60 * 1000])( it.each([10 * 1000, 60 * 1000])( 'does not reset the delay when the connection did not persist longer than the retry reset interval', (retryResetInterval) => { - const backoff = new Backoff(1000, retryResetInterval, noJitter); + const backoff = new DefaultBackoff(1000, retryResetInterval, noJitter); let time = 1000; expect(backoff.fail(time)).toEqual(1000); diff --git a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts index 89097461ab..70d917879e 100644 --- a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts +++ b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts @@ -11,6 +11,7 @@ import { InitializerFactory, SynchronizerFactory, } from '../../../src/api/subsystem/DataSystem/DataSource'; +import DefaultBackoff, { Backoff } from '../../../src/datasource/Backoff'; function makeInitializerFactory(internal: DataSystemInitializer): InitializerFactory { return { @@ -27,24 +28,35 @@ function makeSynchronizerFactory(internal: DataSystemSynchronizer): Synchronizer function makeTestTransitionConditions(): TransitionConditions { return { [DataSourceState.Initializing]: { - durationMS: 10000, + durationMS: 0, transition: Transition.Fallback, }, [DataSourceState.Interrupted]: { - durationMS: 10000, + durationMS: 0, transition: Transition.Fallback, }, [DataSourceState.Closed]: { - durationMS: 10000, + durationMS: 0, transition: Transition.Fallback, }, [DataSourceState.Valid]: { - durationMS: 10000, + durationMS: 0, transition: Transition.Fallback, }, }; } +function makeZeroBackoff(): Backoff { + return { + success(_timeStampMs) { + return 0; + }, + fail(_timeStampMs) { + return 0; + }, + }; +} + it('initializer gets basis, switch to syncrhonizer', async () => { const mockInitializer1 = { run: jest @@ -79,8 +91,7 @@ it('initializer gets basis, switch to syncrhonizer', async () => { [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1)], makeTestTransitionConditions(), - 0, - 0, + makeZeroBackoff(), ); let callback; @@ -160,8 +171,7 @@ it('initializer gets basis, switch to synchronizer 1, fallback to synchronizer 2 [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1), makeSynchronizerFactory(mockSynchronizer2)], makeTestTransitionConditions(), - 0, - 0, + makeZeroBackoff(), ); let callback; @@ -225,8 +235,7 @@ it('it reports error when all initializers fail', async () => { [makeInitializerFactory(mockInitializer1), makeInitializerFactory(mockInitializer2)], [], // no synchronizers for this test makeTestTransitionConditions(), - 0, - 0, + makeZeroBackoff(), ); const dataCallback = jest.fn(); @@ -244,14 +253,22 @@ it('it reports error when all initializers fail', async () => { expect(mockInitializer1.run).toHaveBeenCalledTimes(1); expect(mockInitializer2.run).toHaveBeenCalledTimes(1); expect(dataCallback).toHaveBeenCalledTimes(0); - expect(statusCallback).toHaveBeenCalledTimes(3); - expect(statusCallback).toHaveBeenNthCalledWith(1, DataSourceState.Interrupted, null); - expect(statusCallback).toHaveBeenNthCalledWith(2, DataSourceState.Interrupted, null); + expect(statusCallback).toHaveBeenNthCalledWith( + 1, + DataSourceState.Interrupted, + mockInitializer1Error, + ); + expect(statusCallback).toHaveBeenNthCalledWith( + 2, + DataSourceState.Interrupted, + mockInitializer2Error, + ); expect(statusCallback).toHaveBeenNthCalledWith(3, DataSourceState.Closed, { name: 'ExhaustedDataSources', message: 'CompositeDataSource has exhausted all configured datasources (2 initializers, 0 synchronizers).', }); + expect(statusCallback).toHaveBeenCalledTimes(3); }); it('it can be stopped when in thrashing synchronizer fallback loop', async () => { @@ -269,6 +286,7 @@ it('it can be stopped when in thrashing synchronizer fallback loop', async () => stop: jest.fn(), }; + const mockSynchronizer1Error = { name: 'Error', message: 'I am error...man!' }; const mockSynchronizer1 = { run: jest .fn() @@ -277,7 +295,7 @@ it('it can be stopped when in thrashing synchronizer fallback loop', async () => _dataCallback: (basis: boolean, data: Data) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { - _statusCallback(DataSourceState.Closed, { name: 'Error', message: 'I am error...man!' }); // error that will lead to fallback + _statusCallback(DataSourceState.Closed, mockSynchronizer1Error); // error that will lead to fallback }, ), stop: jest.fn(), @@ -287,18 +305,15 @@ it('it can be stopped when in thrashing synchronizer fallback loop', async () => [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1)], // will continuously fallback onto itself makeTestTransitionConditions(), - 0, - 0, + makeZeroBackoff(), ); const dataCallback = jest.fn(); let statusCallback; await new Promise((resolve) => { - let statusCount = 0; - statusCallback = jest.fn((_1: DataSourceState, _2: any) => { - statusCount += 1; - if (statusCount >= 5) { - resolve(); + statusCallback = jest.fn((state: DataSourceState, _: any) => { + if (state === DataSourceState.Interrupted) { + resolve(); // waiting interruption due to sync error } }); @@ -308,18 +323,21 @@ it('it can be stopped when in thrashing synchronizer fallback loop', async () => expect(mockInitializer1.run).toHaveBeenCalled(); expect(mockSynchronizer1.run).toHaveBeenCalled(); expect(dataCallback).toHaveBeenNthCalledWith(1, true, { key: 'init1' }); - - // wait for trashing to occur - await new Promise((f) => { - setTimeout(f, 1); - }).then(() => underTest.stop()); + console.log(`About to call stop on composite source.`); + underTest.stop(); + console.log(`Got past stop`); // wait for stop to take effect before checking status is closed await new Promise((f) => { - setTimeout(f, 1); + setTimeout(f, 100); }); - expect(statusCallback).toHaveBeenLastCalledWith(DataSourceState.Closed, null); + expect(statusCallback).toHaveBeenNthCalledWith( + 1, + DataSourceState.Interrupted, + mockSynchronizer1Error, + ); + expect(statusCallback).toHaveBeenNthCalledWith(2, DataSourceState.Closed, undefined); }); it('it can be stopped and restarted', async () => { @@ -357,14 +375,14 @@ it('it can be stopped and restarted', async () => { [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1)], makeTestTransitionConditions(), - 0, - 0, + makeZeroBackoff(), ); let callback1; await new Promise((resolve) => { callback1 = jest.fn((_: boolean, data: Data) => { if (data === mockSynchronizer1Data) { + console.log(`About to call stop`); underTest.stop(); resolve(); } @@ -378,6 +396,11 @@ it('it can be stopped and restarted', async () => { expect(mockSynchronizer1.run).toHaveBeenCalledTimes(1); expect(callback1).toHaveBeenCalledTimes(2); + // wait a moment for pending awaits to resolve the stop request + await new Promise((f) => { + setTimeout(f, 1); + }); + let callback2; await new Promise((resolve) => { callback2 = jest.fn((_: boolean, data: Data) => { @@ -392,11 +415,16 @@ it('it can be stopped and restarted', async () => { // check that second run triggers underlying data sources again expect(mockInitializer1.run).toHaveBeenCalledTimes(2); expect(mockSynchronizer1.run).toHaveBeenCalledTimes(2); - expect(callback2).toHaveBeenCalledTimes(4); + expect(callback2).toHaveBeenCalledTimes(2); }); it('it is well behaved with no initializers and no synchronizers configured', async () => { - const underTest = new CompositeDataSource([], [], makeTestTransitionConditions(), 0, 0); + const underTest = new CompositeDataSource( + [], + [], + makeTestTransitionConditions(), + makeZeroBackoff(), + ); let statusCallback; await new Promise((resolve) => { @@ -433,8 +461,7 @@ it('it is well behaved with an initializer and no synchronizers configured', asy [makeInitializerFactory(mockInitializer1)], [], makeTestTransitionConditions(), - 0, - 0, + makeZeroBackoff(), ); let statusCallback; diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts index 73e71d6c9e..50da6160ba 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts @@ -1,5 +1,5 @@ /* eslint-disable no-await-in-loop */ -import Backoff from '../../../datasource/Backoff'; +import { Backoff } from '../../../datasource/Backoff'; import { CallbackHandler } from './CallbackHandler'; import { Data, @@ -31,6 +31,11 @@ export enum Transition { * Transition to the first data source of the same kind. */ Recover, + + /** + * Transition to idle and reset + */ + Stop, } /** @@ -57,11 +62,10 @@ export class CompositeDataSource implements DataSource { private _initPhaseActive: boolean = true; private _currentPosition: number = 0; - private _backoff: Backoff; private _stopped: boolean = true; - private _externalStopPromise: Promise; - private _externalStopResolve?: (value: TransitionRequest) => void; + private _externalTransitionPromise: Promise; + private _externalTransitionResolve?: (value: TransitionRequest) => void; /** * @param _initializers factories to create {@link DataSystemInitializer}s, in priority order. @@ -71,15 +75,13 @@ export class CompositeDataSource implements DataSource { private readonly _initializers: InitializerFactory[], private readonly _synchronizers: SynchronizerFactory[], private readonly _transitionConditions: TransitionConditions, - initialRetryDelayMillis: number, - retryResetIntervalMillis: number, + private readonly _backoff: Backoff, ) { - this._externalStopPromise = new Promise((tr) => { - this._externalStopResolve = tr; + this._externalTransitionPromise = new Promise((tr) => { + this._externalTransitionResolve = tr; }); this._initPhaseActive = true; this._currentPosition = 0; - this._backoff = new Backoff(initialRetryDelayMillis, retryResetIntervalMillis); } async run( @@ -95,87 +97,99 @@ export class CompositeDataSource implements DataSource { let lastTransition: Transition = Transition.None; // eslint-disable-next-line no-constant-condition while (true) { - if (this._stopped) { - // report we are closed, no error as this was due to stop breaking the loop - statusCallback(DataSourceState.Closed, null); - break; - } - - const current: DataSource | undefined = this._pickDataSource(lastTransition); - if (current === undefined) { - statusCallback(DataSourceState.Closed, { - name: 'ExhaustedDataSources', - message: `CompositeDataSource has exhausted all configured datasources (${this._initializers.length} initializers, ${this._synchronizers.length} synchronizers).`, - }); - break; - } - + const currentDS: DataSource | undefined = this._pickDataSource(lastTransition); const internalTransitionPromise = new Promise((transitionResolve) => { - // these local variables are used for handling automatic transition related to data source status (ex: recovering to primary after - // secondary has been valid for N many minutes) - let lastState: DataSourceState | undefined; - let cancelScheduledTransition: (() => void) | undefined; + if (currentDS) { + // these local variables are used for handling automatic transition related to data source status (ex: recovering to primary after + // secondary has been valid for N many seconds) + let lastState: DataSourceState | undefined; + let cancelScheduledTransition: (() => void) | undefined; - const callbackHandler = new CallbackHandler( - (basis: boolean, data: Data) => { - dataCallback(basis, data); - if (basis && this._initPhaseActive) { - // transition to sync if we get basis during init - callbackHandler.disable(); - cancelScheduledTransition?.(); - transitionResolve({ transition: Transition.SwitchToSync }); - } - }, - (state: DataSourceState, err?: any) => { - // When we get a status update, we want to fallback if it is an error. We also want to schedule a transition for some - // time in the future if this status remains for some duration (ex: Recover to primary synchronizer after the secondary - // synchronizer has been Valid for some time). These scheduled transitions are configurable in the constructor. + // this callback handler can be disabled and ensures only one transition request occurs + const callbackHandler = new CallbackHandler( + (basis: boolean, data: Data) => { + this._backoff.success(Date.now()); + dataCallback(basis, data); + if (basis && this._initPhaseActive) { + // transition to sync if we get basis during init + callbackHandler.disable(); + cancelScheduledTransition?.(); + transitionResolve({ transition: Transition.SwitchToSync }); + } + }, + (state: DataSourceState, err?: any) => { + // When we get a status update, we want to fallback if it is an error. We also want to schedule a transition for some + // time in the future if this status remains for some duration (ex: Recover to primary synchronizer after the secondary + // synchronizer has been Valid for some time). These scheduled transitions are configurable in the constructor. - if (err || state === DataSourceState.Closed) { - callbackHandler.disable(); - statusCallback(DataSourceState.Interrupted, null); // underlying errors or closed states are masked as interrupted while we transition - cancelScheduledTransition?.(); - transitionResolve({ transition: Transition.Fallback, err }); // unrecoverable error has occurred, so fallback - } else { - if (state !== lastState) { - lastState = state; - cancelScheduledTransition?.(); // cancel previously scheduled status transition if one was scheduled - const excludeRecovery = this._currentPosition === 0; // primary source cannot recover to itself, so exclude it - const condition = this._lookupTransitionCondition(state, excludeRecovery); - if (condition) { - const { promise, cancel } = this._cancellableDelay(condition.durationMS); - cancelScheduledTransition = cancel; - promise.then(() => { - callbackHandler.disable(); - transitionResolve({ transition: condition.transition }); - }); - } else { - // this data source state does not have a transition condition, so don't schedule any transition + if (err || state === DataSourceState.Closed) { + callbackHandler.disable(); + statusCallback(DataSourceState.Interrupted, err); // underlying errors or closed states are masked as interrupted while we transition + cancelScheduledTransition?.(); + transitionResolve({ transition: Transition.Fallback, err }); // unrecoverable error has occurred, so fallback + } else { + statusCallback(state, null); // report the status upward + if (state !== lastState) { + lastState = state; + cancelScheduledTransition?.(); // cancel previously scheduled status transition if one was scheduled + const excludeRecovery = this._currentPosition === 0; // primary source cannot recover to itself, so exclude it + const condition = this._lookupTransitionCondition(state, excludeRecovery); + if (condition) { + const { promise, cancel } = this._cancellableDelay(condition.durationMS); + cancelScheduledTransition = cancel; + promise.then(() => { + callbackHandler.disable(); + transitionResolve({ transition: condition.transition }); + }); + } else { + // this data source state does not have a transition condition, so don't schedule any transition + } } } - statusCallback(state, null); // report the status upward - } - }, - ); - current.run(callbackHandler.dataHanlder, callbackHandler.statusHandler); + }, + ); + currentDS.run(callbackHandler.dataHanlder, callbackHandler.statusHandler); + } else { + // we don't have a data source to use! + transitionResolve({ + transition: Transition.Stop, + err: { + name: 'ExhaustedDataSources', + message: `CompositeDataSource has exhausted all configured datasources (${this._initializers.length} initializers, ${this._synchronizers.length} synchronizers).`, + }, + }); + } }); // await transition triggered by internal data source or an external stop request - const transitionRequest = await Promise.race([ + let transitionRequest = await Promise.race([ internalTransitionPromise, - this._externalStopPromise, + this._externalTransitionPromise, ]); - // if the transition was due to an error, throttle the transition - if (transitionRequest.err) { - const delay = this._backoff.fail(); - await new Promise((resolve) => { + // stop the underlying datasource before transitioning to next state + currentDS?.stop(); + + if (transitionRequest.err && transitionRequest.transition !== Transition.Stop) { + // if the transition was due to an error, throttle the transition + const delay = this._backoff.fail(Date.now()); + const delayedTransition = new Promise((resolve) => { setTimeout(resolve, delay); - }); + }).then(() => transitionRequest); + + // race the delayed transition and external transition requests to be responsive + transitionRequest = await Promise.race([ + delayedTransition, + this._externalTransitionPromise, + ]); + } + + if (transitionRequest.transition === Transition.Stop) { + // exit the loop + statusCallback(DataSourceState.Closed, transitionRequest.err); + break; } - // stop the underlying datasource before transitioning to next state - current.stop(); lastTransition = transitionRequest.transition; } @@ -184,15 +198,15 @@ export class CompositeDataSource implements DataSource { } async stop() { - this._stopped = true; - this._externalStopResolve?.({ transition: Transition.None }); // the value here doesn't matter, just needs to break the loop's await + this._externalTransitionResolve?.({ transition: Transition.Stop }); } private _reset() { + this._stopped = true; this._initPhaseActive = true; this._currentPosition = 0; - this._externalStopPromise = new Promise((tr) => { - this._externalStopResolve = tr; + this._externalTransitionPromise = new Promise((tr) => { + this._externalTransitionResolve = tr; }); // intentionally not resetting the backoff to avoid a code path that could circumvent throttling } diff --git a/packages/shared/common/src/api/subsystem/DataSystem/index.ts b/packages/shared/common/src/api/subsystem/DataSystem/index.ts index 8cedfee2a4..d49e3438fd 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/index.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/index.ts @@ -1,3 +1,7 @@ -export { DataSystemInitializer, InitializerFactory } from './DataSystemInitializer'; -export { DataSystemSynchronizer, SynchronizerFactory } from './DataSystemSynchronizer'; +export { + DataSystemInitializer, + DataSystemSynchronizer, + InitializerFactory, + SynchronizerFactory, +} from './DataSource'; export { CompositeDataSource } from './CompositeDataSource'; diff --git a/packages/shared/common/src/datasource/Backoff.ts b/packages/shared/common/src/datasource/Backoff.ts index ce0e931ee5..6a0a99c052 100644 --- a/packages/shared/common/src/datasource/Backoff.ts +++ b/packages/shared/common/src/datasource/Backoff.ts @@ -1,6 +1,11 @@ const MAX_RETRY_DELAY = 30 * 1000; // Maximum retry delay 30 seconds. const JITTER_RATIO = 0.5; // Delay should be 50%-100% of calculated time. +export interface Backoff { + success(timeStampMs: number): void; + fail(timeStampMs: number): number; +} + /** * Implements exponential backoff and jitter. This class tracks successful connections and failures * and produces a retry delay. @@ -11,7 +16,7 @@ const JITTER_RATIO = 0.5; // Delay should be 50%-100% of calculated time. * initialRetryDelayMillis and capping at MAX_RETRY_DELAY. If RESET_INTERVAL has elapsed after a * success, without an intervening faulure, then the backoff is reset to initialRetryDelayMillis. */ -export default class Backoff { +export class DefaultBackoff { private _retryCount: number = 0; private _activeSince?: number; private _initialRetryDelayMillis: number; diff --git a/packages/shared/common/src/datasource/index.ts b/packages/shared/common/src/datasource/index.ts index fe4b250c5e..e727947dce 100644 --- a/packages/shared/common/src/datasource/index.ts +++ b/packages/shared/common/src/datasource/index.ts @@ -1,3 +1,4 @@ +import { Backoff, DefaultBackoff } from './Backoff'; import { DataSourceErrorKind } from './DataSourceErrorKinds'; import { LDFileDataSourceError, @@ -7,6 +8,8 @@ import { } from './errors'; export { + Backoff, + DefaultBackoff, DataSourceErrorKind, LDFileDataSourceError, LDPollingError, diff --git a/packages/shared/common/src/index.ts b/packages/shared/common/src/index.ts index ae3f9daf08..7df308f674 100644 --- a/packages/shared/common/src/index.ts +++ b/packages/shared/common/src/index.ts @@ -2,7 +2,9 @@ import AttributeReference from './AttributeReference'; import Context from './Context'; import ContextFilter from './ContextFilter'; import { + Backoff, DataSourceErrorKind, + DefaultBackoff, LDFileDataSourceError, LDPollingError, LDStreamingError, @@ -23,6 +25,8 @@ export { Context, ContextFilter, DataSourceErrorKind, + Backoff, + DefaultBackoff, LDPollingError, LDStreamingError, StreamingErrorHandler, From eeaa3d10cc4267e2d799ecc02014b6c552facb4f Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Mon, 24 Feb 2025 15:29:49 -0600 Subject: [PATCH 04/14] removing accidental source include --- packages/sdk/browser/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/browser/tsconfig.json b/packages/sdk/browser/tsconfig.json index 82f7f65361..7306c5b0c6 100644 --- a/packages/sdk/browser/tsconfig.json +++ b/packages/sdk/browser/tsconfig.json @@ -18,7 +18,7 @@ "types": ["node", "jest"], "allowJs": true }, - "include": ["src", "../../shared/common/src/datasource/Backoff.ts"], + "include": ["src"], "exclude": [ "vite.config.ts", "__tests__", From 6d0a08be685fd42e1543dfed10275a822fe72053 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 4 Mar 2025 15:19:10 -0600 Subject: [PATCH 05/14] Fixing review comments. --- .../src/platform/DefaultBrowserEventSource.ts | 2 +- .../__tests__/datasource/Backoff.test.ts | 2 +- .../DataSystem/CompositeDataSource.test.ts | 17 ++--- .../subsystem/DataSystem/CallbackHandler.ts | 8 +-- .../DataSystem/CompositeDataSource.ts | 71 +++++++------------ .../api/subsystem/DataSystem/DataSource.ts | 7 +- .../shared/common/src/datasource/Backoff.ts | 4 +- 7 files changed, 45 insertions(+), 66 deletions(-) diff --git a/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts b/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts index 742f1f1654..d753931aa8 100644 --- a/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts +++ b/packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts @@ -1,11 +1,11 @@ import { + DefaultBackoff, EventListener, EventName, EventSourceInitDict, HttpErrorResponse, EventSource as LDEventSource, } from '@launchdarkly/js-client-sdk-common'; -import { DefaultBackoff } from '@launchdarkly/js-sdk-common'; /** * Implementation Notes: diff --git a/packages/shared/common/__tests__/datasource/Backoff.test.ts b/packages/shared/common/__tests__/datasource/Backoff.test.ts index c4a0e25abf..77b09ce0fa 100644 --- a/packages/shared/common/__tests__/datasource/Backoff.test.ts +++ b/packages/shared/common/__tests__/datasource/Backoff.test.ts @@ -1,4 +1,4 @@ -import DefaultBackoff from '../../src/datasource/Backoff'; +import { DefaultBackoff } from '../../src/datasource/Backoff'; const noJitter = (): number => 0; const maxJitter = (): number => 1; diff --git a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts index 70d917879e..4f9ad66fe1 100644 --- a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts +++ b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts @@ -11,7 +11,7 @@ import { InitializerFactory, SynchronizerFactory, } from '../../../src/api/subsystem/DataSystem/DataSource'; -import DefaultBackoff, { Backoff } from '../../../src/datasource/Backoff'; +import { Backoff } from '../../../src/datasource/Backoff'; function makeInitializerFactory(internal: DataSystemInitializer): InitializerFactory { return { @@ -29,29 +29,29 @@ function makeTestTransitionConditions(): TransitionConditions { return { [DataSourceState.Initializing]: { durationMS: 0, - transition: Transition.Fallback, + transition: 'fallback', }, [DataSourceState.Interrupted]: { durationMS: 0, - transition: Transition.Fallback, + transition: 'fallback', }, [DataSourceState.Closed]: { durationMS: 0, - transition: Transition.Fallback, + transition: 'fallback', }, [DataSourceState.Valid]: { durationMS: 0, - transition: Transition.Fallback, + transition: 'fallback', }, }; } function makeZeroBackoff(): Backoff { return { - success(_timeStampMs) { + success() { return 0; }, - fail(_timeStampMs) { + fail() { return 0; }, }; @@ -323,9 +323,7 @@ it('it can be stopped when in thrashing synchronizer fallback loop', async () => expect(mockInitializer1.run).toHaveBeenCalled(); expect(mockSynchronizer1.run).toHaveBeenCalled(); expect(dataCallback).toHaveBeenNthCalledWith(1, true, { key: 'init1' }); - console.log(`About to call stop on composite source.`); underTest.stop(); - console.log(`Got past stop`); // wait for stop to take effect before checking status is closed await new Promise((f) => { @@ -382,7 +380,6 @@ it('it can be stopped and restarted', async () => { await new Promise((resolve) => { callback1 = jest.fn((_: boolean, data: Data) => { if (data === mockSynchronizer1Data) { - console.log(`About to call stop`); underTest.stop(); resolve(); } diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts index 7ad446ddcd..cbd7f94337 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts @@ -17,7 +17,7 @@ export class CallbackHandler { this._disabled = true; } - dataHanlder = async (basis: boolean, data: Data) => { + async dataHanlder(basis: boolean, data: Data) { if (this._disabled) { return; } @@ -25,13 +25,13 @@ export class CallbackHandler { // TODO: SDK-1044 track selector for future synchronizer to use // report data up this._dataCallback(basis, data); - }; + } - statusHandler = async (status: DataSourceState, err?: any) => { + async statusHandler(status: DataSourceState, err?: any) { if (this._disabled) { return; } this._statusCallback(status, err); - }; + } } diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts index 50da6160ba..3e2c0f7b27 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts @@ -9,34 +9,16 @@ import { SynchronizerFactory, } from './DataSource'; +// TODO: SDK-858, specify these constants when CompositeDataSource is used. +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const DEFAULT_FALLBACK_TIME_MS = 2 * 60 * 1000; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const DEFAULT_RECOVERY_TIME_MS = 5 * 60 * 1000; + /** * Represents a transition between data sources. */ -export enum Transition { - /** - * A no-op transition. - */ - None, - /** - * Transition from current data source to the first synchronizer. - */ - SwitchToSync, - - /** - * Transition to the next data source of the same kind. - */ - Fallback, - - /** - * Transition to the first data source of the same kind. - */ - Recover, - - /** - * Transition to idle and reset - */ - Stop, -} +export type Transition = 'none' | 'switchToSync' | 'fallback' | 'recover' | 'stop'; /** * Given a {@link DataSourceState}, how long to wait before transitioning. @@ -57,8 +39,6 @@ interface TransitionRequest { export class CompositeDataSource implements DataSource { // TODO: SDK-856 async notification if initializer takes too long // TODO: SDK-1044 utilize selector from initializers - private readonly _defaultFallbackTimeMs = 2 * 60 * 1000; - private readonly _defaultRecoveryTimeMs = 5 * 60 * 1000; private _initPhaseActive: boolean = true; private _currentPosition: number = 0; @@ -77,8 +57,8 @@ export class CompositeDataSource implements DataSource { private readonly _transitionConditions: TransitionConditions, private readonly _backoff: Backoff, ) { - this._externalTransitionPromise = new Promise((tr) => { - this._externalTransitionResolve = tr; + this._externalTransitionPromise = new Promise((resolveTransition) => { + this._externalTransitionResolve = resolveTransition; }); this._initPhaseActive = true; this._currentPosition = 0; @@ -94,7 +74,7 @@ export class CompositeDataSource implements DataSource { } this._stopped = false; - let lastTransition: Transition = Transition.None; + let lastTransition: Transition = 'none'; // eslint-disable-next-line no-constant-condition while (true) { const currentDS: DataSource | undefined = this._pickDataSource(lastTransition); @@ -108,13 +88,13 @@ export class CompositeDataSource implements DataSource { // this callback handler can be disabled and ensures only one transition request occurs const callbackHandler = new CallbackHandler( (basis: boolean, data: Data) => { - this._backoff.success(Date.now()); + this._backoff.success(); dataCallback(basis, data); if (basis && this._initPhaseActive) { // transition to sync if we get basis during init callbackHandler.disable(); cancelScheduledTransition?.(); - transitionResolve({ transition: Transition.SwitchToSync }); + transitionResolve({ transition: 'switchToSync' }); } }, (state: DataSourceState, err?: any) => { @@ -126,7 +106,7 @@ export class CompositeDataSource implements DataSource { callbackHandler.disable(); statusCallback(DataSourceState.Interrupted, err); // underlying errors or closed states are masked as interrupted while we transition cancelScheduledTransition?.(); - transitionResolve({ transition: Transition.Fallback, err }); // unrecoverable error has occurred, so fallback + transitionResolve({ transition: 'fallback', err }); // unrecoverable error has occurred, so fallback } else { statusCallback(state, null); // report the status upward if (state !== lastState) { @@ -148,11 +128,14 @@ export class CompositeDataSource implements DataSource { } }, ); - currentDS.run(callbackHandler.dataHanlder, callbackHandler.statusHandler); + currentDS.run( + (basis, data) => callbackHandler.dataHanlder(basis, data), + (status, err) => callbackHandler.statusHandler(status, err), + ); } else { // we don't have a data source to use! transitionResolve({ - transition: Transition.Stop, + transition: 'stop', err: { name: 'ExhaustedDataSources', message: `CompositeDataSource has exhausted all configured datasources (${this._initializers.length} initializers, ${this._synchronizers.length} synchronizers).`, @@ -170,9 +153,9 @@ export class CompositeDataSource implements DataSource { // stop the underlying datasource before transitioning to next state currentDS?.stop(); - if (transitionRequest.err && transitionRequest.transition !== Transition.Stop) { + if (transitionRequest.err && transitionRequest.transition !== 'stop') { // if the transition was due to an error, throttle the transition - const delay = this._backoff.fail(Date.now()); + const delay = this._backoff.fail(); const delayedTransition = new Promise((resolve) => { setTimeout(resolve, delay); }).then(() => transitionRequest); @@ -184,7 +167,7 @@ export class CompositeDataSource implements DataSource { ]); } - if (transitionRequest.transition === Transition.Stop) { + if (transitionRequest.transition === 'stop') { // exit the loop statusCallback(DataSourceState.Closed, transitionRequest.err); break; @@ -198,7 +181,7 @@ export class CompositeDataSource implements DataSource { } async stop() { - this._externalTransitionResolve?.({ transition: Transition.Stop }); + this._externalTransitionResolve?.({ transition: 'stop' }); } private _reset() { @@ -213,17 +196,17 @@ export class CompositeDataSource implements DataSource { private _pickDataSource(transition: Transition | undefined): DataSource | undefined { switch (transition) { - case Transition.SwitchToSync: + case 'switchToSync': this._initPhaseActive = false; // one way toggle to false, unless this class is reset() this._currentPosition = 0; break; - case Transition.Fallback: + case 'fallback': this._currentPosition += 1; break; - case Transition.Recover: + case 'recover': this._currentPosition = 0; break; - case Transition.None: + case 'none': default: // don't do anything in this case break; @@ -262,7 +245,7 @@ export class CompositeDataSource implements DataSource { const condition = this._transitionConditions[state]; // exclude recovery can happen for certain initializers/synchronizers (ex: the primary synchronizer shouldn't recover to itself) - if (!condition || (excludeRecover && condition.transition === Transition.Recover)) { + if (!condition || (excludeRecover && condition.transition === 'recover')) { return undefined; } diff --git a/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts index 352d668f6b..80885a401e 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts @@ -11,8 +11,9 @@ export enum DataSourceState { export interface DataSource { /** * May be called any number of times, if already started, has no effect - * @param cb may be invoked many times - * @returns + * @param dataCallback that will be called when data arrives, may be called multiple times. + * @param statusCallback that will be called when data source state changes or an unrecoverable error + * has been encountered. */ run( dataCallback: (basis: boolean, data: Data) => void, @@ -21,8 +22,6 @@ export interface DataSource { /** * May be called any number of times, if already stopped, has no effect. - * @param cb - * @returns */ stop(): void; } diff --git a/packages/shared/common/src/datasource/Backoff.ts b/packages/shared/common/src/datasource/Backoff.ts index 6a0a99c052..8b2a7f8be9 100644 --- a/packages/shared/common/src/datasource/Backoff.ts +++ b/packages/shared/common/src/datasource/Backoff.ts @@ -2,8 +2,8 @@ const MAX_RETRY_DELAY = 30 * 1000; // Maximum retry delay 30 seconds. const JITTER_RATIO = 0.5; // Delay should be 50%-100% of calculated time. export interface Backoff { - success(timeStampMs: number): void; - fail(timeStampMs: number): number; + success(): void; + fail(): number; } /** From ea9845da3948fea042cfd786f648f2efe4954ef8 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 14 Mar 2025 17:09:36 -0500 Subject: [PATCH 06/14] Addressing review comments --- .../DataSystem/CompositeDataSource.test.ts | 15 +++++++------ .../subsystem/DataSystem/CallbackHandler.ts | 2 +- .../DataSystem/CompositeDataSource.ts | 21 +++++++++++-------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts index 4f9ad66fe1..3415ece2fe 100644 --- a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts +++ b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts @@ -1,6 +1,5 @@ import { CompositeDataSource, - Transition, TransitionConditions, } from '../../../src/api/subsystem/DataSystem/CompositeDataSource'; import { @@ -57,7 +56,7 @@ function makeZeroBackoff(): Backoff { }; } -it('initializer gets basis, switch to syncrhonizer', async () => { +it('handles initializer getting basis, switching to syncrhonizer', async () => { const mockInitializer1 = { run: jest .fn() @@ -112,7 +111,7 @@ it('initializer gets basis, switch to syncrhonizer', async () => { expect(callback).toHaveBeenNthCalledWith(2, false, { key: 'sync1' }); }); -it('initializer gets basis, switch to synchronizer 1, fallback to synchronizer 2, recover to synchronizer 1', async () => { +it('handles initializer getting basis, switches to synchronizer 1, falls back to synchronizer 2, recovers to synchronizer 1', async () => { const mockInitializer1: DataSystemInitializer = { run: jest .fn() @@ -194,7 +193,7 @@ it('initializer gets basis, switch to synchronizer 1, fallback to synchronizer 2 expect(callback).toHaveBeenNthCalledWith(3, false, { key: 'sync1' }); // sync2 recovers back to sync1 }); -it('it reports error when all initializers fail', async () => { +it('reports error when all initializers fail', async () => { const mockInitializer1Error = { name: 'Error', message: 'I am initializer1 error!', @@ -271,7 +270,7 @@ it('it reports error when all initializers fail', async () => { expect(statusCallback).toHaveBeenCalledTimes(3); }); -it('it can be stopped when in thrashing synchronizer fallback loop', async () => { +it('can be stopped when in thrashing synchronizer fallback loop', async () => { const mockInitializer1 = { run: jest .fn() @@ -338,7 +337,7 @@ it('it can be stopped when in thrashing synchronizer fallback loop', async () => expect(statusCallback).toHaveBeenNthCalledWith(2, DataSourceState.Closed, undefined); }); -it('it can be stopped and restarted', async () => { +it('can be stopped and restarted', async () => { const mockInitializer1Data = { key: 'init1' }; const mockInitializer1 = { run: jest @@ -415,7 +414,7 @@ it('it can be stopped and restarted', async () => { expect(callback2).toHaveBeenCalledTimes(2); }); -it('it is well behaved with no initializers and no synchronizers configured', async () => { +it('is well behaved with no initializers and no synchronizers configured', async () => { const underTest = new CompositeDataSource( [], [], @@ -439,7 +438,7 @@ it('it is well behaved with no initializers and no synchronizers configured', as }); }); -it('it is well behaved with an initializer and no synchronizers configured', async () => { +it('is well behaved with an initializer and no synchronizers configured', async () => { const mockInitializer1 = { run: jest .fn() diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts index cbd7f94337..12ec52ce79 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts @@ -17,7 +17,7 @@ export class CallbackHandler { this._disabled = true; } - async dataHanlder(basis: boolean, data: Data) { + async dataHandler(basis: boolean, data: Data) { if (this._disabled) { return; } diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts index 3e2c0f7b27..4b0d19a5b5 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts @@ -1,5 +1,6 @@ /* eslint-disable no-await-in-loop */ import { Backoff } from '../../../datasource/Backoff'; +import { LDLogger } from '../../logging'; import { CallbackHandler } from './CallbackHandler'; import { Data, @@ -46,6 +47,7 @@ export class CompositeDataSource implements DataSource { private _stopped: boolean = true; private _externalTransitionPromise: Promise; private _externalTransitionResolve?: (value: TransitionRequest) => void; + private _cancelTokens: (() => void)[] = []; /** * @param _initializers factories to create {@link DataSystemInitializer}s, in priority order. @@ -56,6 +58,7 @@ export class CompositeDataSource implements DataSource { private readonly _synchronizers: SynchronizerFactory[], private readonly _transitionConditions: TransitionConditions, private readonly _backoff: Backoff, + private readonly _logger?: LDLogger, ) { this._externalTransitionPromise = new Promise((resolveTransition) => { this._externalTransitionResolve = resolveTransition; @@ -70,6 +73,7 @@ export class CompositeDataSource implements DataSource { ): Promise { if (!this._stopped) { // don't allow multiple simultaneous runs + this._logger?.info('CompositeDataSource already running. Ignoring call to start.'); return; } this._stopped = false; @@ -116,6 +120,7 @@ export class CompositeDataSource implements DataSource { const condition = this._lookupTransitionCondition(state, excludeRecovery); if (condition) { const { promise, cancel } = this._cancellableDelay(condition.durationMS); + this._cancelTokens.push(cancel); cancelScheduledTransition = cancel; promise.then(() => { callbackHandler.disable(); @@ -129,7 +134,7 @@ export class CompositeDataSource implements DataSource { }, ); currentDS.run( - (basis, data) => callbackHandler.dataHanlder(basis, data), + (basis, data) => callbackHandler.dataHandler(basis, data), (status, err) => callbackHandler.statusHandler(status, err), ); } else { @@ -156,9 +161,9 @@ export class CompositeDataSource implements DataSource { if (transitionRequest.err && transitionRequest.transition !== 'stop') { // if the transition was due to an error, throttle the transition const delay = this._backoff.fail(); - const delayedTransition = new Promise((resolve) => { - setTimeout(resolve, delay); - }).then(() => transitionRequest); + const { promise, cancel } = this._cancellableDelay(delay); + this._cancelTokens.push(cancel); + const delayedTransition = promise.then(() => transitionRequest); // race the delayed transition and external transition requests to be responsive transitionRequest = await Promise.race([ @@ -170,6 +175,7 @@ export class CompositeDataSource implements DataSource { if (transitionRequest.transition === 'stop') { // exit the loop statusCallback(DataSourceState.Closed, transitionRequest.err); + lastTransition = transitionRequest.transition; break; } @@ -181,6 +187,7 @@ export class CompositeDataSource implements DataSource { } async stop() { + this._cancelTokens.forEach((cancel) => cancel()); this._externalTransitionResolve?.({ transition: 'stop' }); } @@ -254,19 +261,15 @@ export class CompositeDataSource implements DataSource { private _cancellableDelay = (delayMS: number) => { let timeout: ReturnType | undefined; - let reject: ((reason?: any) => void) | undefined; - const promise = new Promise((res, rej) => { + const promise = new Promise((res, _) => { timeout = setTimeout(res, delayMS); - reject = rej; }); return { promise, cancel() { if (timeout) { clearTimeout(timeout); - reject?.(); timeout = undefined; - reject = undefined; } }, }; From 33c873e2b0b46f073bf8ed741d06b94bbfdd836f Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 14 Mar 2025 17:33:23 -0500 Subject: [PATCH 07/14] Fixing cancellation token issue --- .../DataSystem/CompositeDataSource.ts | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts index 4b0d19a5b5..d77c3916a3 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts @@ -87,7 +87,7 @@ export class CompositeDataSource implements DataSource { // these local variables are used for handling automatic transition related to data source status (ex: recovering to primary after // secondary has been valid for N many seconds) let lastState: DataSourceState | undefined; - let cancelScheduledTransition: (() => void) | undefined; + let cancelScheduledTransition: () => void = () => {}; // this callback handler can be disabled and ensures only one transition request occurs const callbackHandler = new CallbackHandler( @@ -97,7 +97,7 @@ export class CompositeDataSource implements DataSource { if (basis && this._initPhaseActive) { // transition to sync if we get basis during init callbackHandler.disable(); - cancelScheduledTransition?.(); + this._consumeCancelToken(cancelScheduledTransition); transitionResolve({ transition: 'switchToSync' }); } }, @@ -109,19 +109,19 @@ export class CompositeDataSource implements DataSource { if (err || state === DataSourceState.Closed) { callbackHandler.disable(); statusCallback(DataSourceState.Interrupted, err); // underlying errors or closed states are masked as interrupted while we transition - cancelScheduledTransition?.(); + this._consumeCancelToken(cancelScheduledTransition); transitionResolve({ transition: 'fallback', err }); // unrecoverable error has occurred, so fallback } else { statusCallback(state, null); // report the status upward if (state !== lastState) { lastState = state; - cancelScheduledTransition?.(); // cancel previously scheduled status transition if one was scheduled + this._consumeCancelToken(cancelScheduledTransition); // cancel previously scheduled status transition if one was scheduled const excludeRecovery = this._currentPosition === 0; // primary source cannot recover to itself, so exclude it const condition = this._lookupTransitionCondition(state, excludeRecovery); if (condition) { const { promise, cancel } = this._cancellableDelay(condition.durationMS); - this._cancelTokens.push(cancel); cancelScheduledTransition = cancel; + this._cancelTokens.push(cancelScheduledTransition); promise.then(() => { callbackHandler.disable(); transitionResolve({ transition: condition.transition }); @@ -161,8 +161,8 @@ export class CompositeDataSource implements DataSource { if (transitionRequest.err && transitionRequest.transition !== 'stop') { // if the transition was due to an error, throttle the transition const delay = this._backoff.fail(); - const { promise, cancel } = this._cancellableDelay(delay); - this._cancelTokens.push(cancel); + const { promise, cancel: cancelDelay } = this._cancellableDelay(delay); + this._cancelTokens.push(cancelDelay); const delayedTransition = promise.then(() => transitionRequest); // race the delayed transition and external transition requests to be responsive @@ -170,6 +170,9 @@ export class CompositeDataSource implements DataSource { delayedTransition, this._externalTransitionPromise, ]); + + // consume the delay cancel token (even if it resolved, need to stop tracking its token) + this._consumeCancelToken(cancelDelay); } if (transitionRequest.transition === 'stop') { @@ -188,6 +191,7 @@ export class CompositeDataSource implements DataSource { async stop() { this._cancelTokens.forEach((cancel) => cancel()); + this._cancelTokens = []; this._externalTransitionResolve?.({ transition: 'stop' }); } @@ -274,4 +278,12 @@ export class CompositeDataSource implements DataSource { }, }; }; + + private _consumeCancelToken(cancel: () => void) { + cancel(); + const index = this._cancelTokens.indexOf(cancel, 0); + if (index > -1) { + this._cancelTokens.splice(index, 1); + } + } } From fa634f5783a48b8fd032f446de7a557ada4ae6d2 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Mon, 24 Mar 2025 10:56:32 -0500 Subject: [PATCH 08/14] Additional changes after working on contract test integration --- package.json | 2 +- .../DataSystem/CompositeDataSource.test.ts | 179 +++++++++++------- .../subsystem/DataSystem/CallbackHandler.ts | 6 +- .../api/subsystem/DataSystem/DataSource.ts | 22 +-- .../src/api/subsystem/DataSystem/index.ts | 7 +- .../shared/common/src/api/subsystem/index.ts | 14 ++ .../CompositeDataSource.ts | 63 +++--- .../shared/common/src/datasource/index.ts | 2 + packages/shared/common/src/index.ts | 2 + 9 files changed, 184 insertions(+), 113 deletions(-) rename packages/shared/common/src/{api/subsystem/DataSystem => datasource}/CompositeDataSource.ts (86%) diff --git a/package.json b/package.json index b7b0bdc24b..53163bd25c 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "test": "echo Please run tests for individual packages.", "coverage": "npm run test -- --coverage", "contract-test-service": "npm --prefix contract-tests install && npm --prefix contract-tests start", - "contract-test-harness": "curl -s https://raw.githubusercontent.com/launchdarkly/sdk-test-harness/master/downloader/run.sh \\ | VERSION=v2 PARAMS=\"-url http://localhost:8000 -debug -stop-service-at-end $TEST_HARNESS_PARAMS\" sh", + "contract-test-harness": "curl -s https://raw.githubusercontent.com/launchdarkly/sdk-test-harness/master/downloader/run.sh \\ | VERSION=v2 PARAMS=\"-url http://localhost:8000 -debug --skip-from=./contract-tests/testharness-suppressions.txt -stop-service-at-end $TEST_HARNESS_PARAMS\" sh", "contract-tests": "npm run contract-test-service & npm run contract-test-harness", "prettier": "npx prettier --write \"**/*.{js,ts,tsx,json,yaml,yml,md}\" --log-level warn", "check": "yarn && yarn prettier && yarn lint && tsc && yarn build" diff --git a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts index 3415ece2fe..5b69a0d40f 100644 --- a/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts +++ b/packages/shared/common/__tests__/subsystem/DataSystem/CompositeDataSource.test.ts @@ -1,27 +1,22 @@ import { - CompositeDataSource, - TransitionConditions, -} from '../../../src/api/subsystem/DataSystem/CompositeDataSource'; -import { - Data, DataSourceState, DataSystemInitializer, DataSystemSynchronizer, - InitializerFactory, - SynchronizerFactory, + LDInitializerFactory, + LDSynchronizerFactory, } from '../../../src/api/subsystem/DataSystem/DataSource'; import { Backoff } from '../../../src/datasource/Backoff'; +import { + CompositeDataSource, + TransitionConditions, +} from '../../../src/datasource/CompositeDataSource'; -function makeInitializerFactory(internal: DataSystemInitializer): InitializerFactory { - return { - create: () => internal, - }; +function makeInitializerFactory(internal: DataSystemInitializer): LDInitializerFactory { + return () => internal; } -function makeSynchronizerFactory(internal: DataSystemSynchronizer): SynchronizerFactory { - return { - create: () => internal, - }; +function makeSynchronizerFactory(internal: DataSystemSynchronizer): LDSynchronizerFactory { + return () => internal; } function makeTestTransitionConditions(): TransitionConditions { @@ -58,11 +53,11 @@ function makeZeroBackoff(): Backoff { it('handles initializer getting basis, switching to syncrhonizer', async () => { const mockInitializer1 = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(true, { key: 'init1' }); @@ -73,11 +68,11 @@ it('handles initializer getting basis, switching to syncrhonizer', async () => { const mockSynchronizer1Data = { key: 'sync1' }; const mockSynchronizer1 = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(false, mockSynchronizer1Data); @@ -89,23 +84,24 @@ it('handles initializer getting basis, switching to syncrhonizer', async () => { const underTest = new CompositeDataSource( [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1)], + undefined, makeTestTransitionConditions(), makeZeroBackoff(), ); let callback; await new Promise((resolve) => { - callback = jest.fn((_: boolean, data: Data) => { + callback = jest.fn((_: boolean, data: any) => { if (data === mockSynchronizer1Data) { resolve(); } }); - underTest.run(callback, jest.fn()); + underTest.start(callback, jest.fn()); }); - expect(mockInitializer1.run).toHaveBeenCalledTimes(1); - expect(mockSynchronizer1.run).toHaveBeenCalledTimes(1); + expect(mockInitializer1.start).toHaveBeenCalledTimes(1); + expect(mockSynchronizer1.start).toHaveBeenCalledTimes(1); expect(callback).toHaveBeenCalledTimes(2); expect(callback).toHaveBeenNthCalledWith(1, true, { key: 'init1' }); expect(callback).toHaveBeenNthCalledWith(2, false, { key: 'sync1' }); @@ -113,11 +109,11 @@ it('handles initializer getting basis, switching to syncrhonizer', async () => { it('handles initializer getting basis, switches to synchronizer 1, falls back to synchronizer 2, recovers to synchronizer 1', async () => { const mockInitializer1: DataSystemInitializer = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(true, { key: 'init1' }); @@ -129,11 +125,11 @@ it('handles initializer getting basis, switches to synchronizer 1, falls back to let sync1RunCount = 0; const mockSynchronizer1Data = { key: 'sync1' }; const mockSynchronizer1 = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { if (sync1RunCount === 0) { @@ -142,7 +138,7 @@ it('handles initializer getting basis, switches to synchronizer 1, falls back to message: 'I am error...man!', }); // error that will lead to fallback } else { - _dataCallback(false, mockSynchronizer1Data); // second run will lead to data + _dataCallback(false, mockSynchronizer1Data); // second start will lead to data } sync1RunCount += 1; }, @@ -152,11 +148,11 @@ it('handles initializer getting basis, switches to synchronizer 1, falls back to const mockSynchronizer2Data = { key: 'sync2' }; const mockSynchronizer2 = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(false, mockSynchronizer2Data); @@ -169,24 +165,25 @@ it('handles initializer getting basis, switches to synchronizer 1, falls back to const underTest = new CompositeDataSource( [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1), makeSynchronizerFactory(mockSynchronizer2)], + undefined, makeTestTransitionConditions(), makeZeroBackoff(), ); let callback; await new Promise((resolve) => { - callback = jest.fn((_: boolean, data: Data) => { + callback = jest.fn((_: boolean, data: any) => { if (data === mockSynchronizer1Data) { resolve(); } }); - underTest.run(callback, jest.fn()); + underTest.start(callback, jest.fn()); }); - expect(mockInitializer1.run).toHaveBeenCalledTimes(1); - expect(mockSynchronizer1.run).toHaveBeenCalledTimes(2); - expect(mockSynchronizer2.run).toHaveBeenCalledTimes(1); + expect(mockInitializer1.start).toHaveBeenCalledTimes(1); + expect(mockSynchronizer1.start).toHaveBeenCalledTimes(2); + expect(mockSynchronizer2.start).toHaveBeenCalledTimes(1); expect(callback).toHaveBeenCalledTimes(3); expect(callback).toHaveBeenNthCalledWith(1, true, { key: 'init1' }); expect(callback).toHaveBeenNthCalledWith(2, false, { key: 'sync2' }); // sync1 errors and fallsback @@ -199,11 +196,11 @@ it('reports error when all initializers fail', async () => { message: 'I am initializer1 error!', }; const mockInitializer1: DataSystemInitializer = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _statusCallback(DataSourceState.Closed, mockInitializer1Error); @@ -217,11 +214,11 @@ it('reports error when all initializers fail', async () => { message: 'I am initializer2 error!', }; const mockInitializer2: DataSystemInitializer = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _statusCallback(DataSourceState.Closed, mockInitializer2Error); @@ -233,6 +230,7 @@ it('reports error when all initializers fail', async () => { const underTest = new CompositeDataSource( [makeInitializerFactory(mockInitializer1), makeInitializerFactory(mockInitializer2)], [], // no synchronizers for this test + undefined, makeTestTransitionConditions(), makeZeroBackoff(), ); @@ -246,11 +244,11 @@ it('reports error when all initializers fail', async () => { } }); - underTest.run(dataCallback, statusCallback); + underTest.start(dataCallback, statusCallback); }); - expect(mockInitializer1.run).toHaveBeenCalledTimes(1); - expect(mockInitializer2.run).toHaveBeenCalledTimes(1); + expect(mockInitializer1.start).toHaveBeenCalledTimes(1); + expect(mockInitializer2.start).toHaveBeenCalledTimes(1); expect(dataCallback).toHaveBeenCalledTimes(0); expect(statusCallback).toHaveBeenNthCalledWith( 1, @@ -272,11 +270,11 @@ it('reports error when all initializers fail', async () => { it('can be stopped when in thrashing synchronizer fallback loop', async () => { const mockInitializer1 = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(true, { key: 'init1' }); @@ -287,11 +285,11 @@ it('can be stopped when in thrashing synchronizer fallback loop', async () => { const mockSynchronizer1Error = { name: 'Error', message: 'I am error...man!' }; const mockSynchronizer1 = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _statusCallback(DataSourceState.Closed, mockSynchronizer1Error); // error that will lead to fallback @@ -303,6 +301,7 @@ it('can be stopped when in thrashing synchronizer fallback loop', async () => { const underTest = new CompositeDataSource( [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1)], // will continuously fallback onto itself + undefined, makeTestTransitionConditions(), makeZeroBackoff(), ); @@ -316,11 +315,11 @@ it('can be stopped when in thrashing synchronizer fallback loop', async () => { } }); - underTest.run(dataCallback, statusCallback); + underTest.start(dataCallback, statusCallback); }); - expect(mockInitializer1.run).toHaveBeenCalled(); - expect(mockSynchronizer1.run).toHaveBeenCalled(); + expect(mockInitializer1.start).toHaveBeenCalled(); + expect(mockSynchronizer1.start).toHaveBeenCalled(); expect(dataCallback).toHaveBeenNthCalledWith(1, true, { key: 'init1' }); underTest.stop(); @@ -340,11 +339,11 @@ it('can be stopped when in thrashing synchronizer fallback loop', async () => { it('can be stopped and restarted', async () => { const mockInitializer1Data = { key: 'init1' }; const mockInitializer1 = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(true, mockInitializer1Data); @@ -355,11 +354,11 @@ it('can be stopped and restarted', async () => { const mockSynchronizer1Data = { key: 'sync1' }; const mockSynchronizer1 = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(false, mockSynchronizer1Data); @@ -371,25 +370,26 @@ it('can be stopped and restarted', async () => { const underTest = new CompositeDataSource( [makeInitializerFactory(mockInitializer1)], [makeSynchronizerFactory(mockSynchronizer1)], + undefined, makeTestTransitionConditions(), makeZeroBackoff(), ); let callback1; await new Promise((resolve) => { - callback1 = jest.fn((_: boolean, data: Data) => { + callback1 = jest.fn((_: boolean, data: any) => { if (data === mockSynchronizer1Data) { underTest.stop(); resolve(); } }); - // first run - underTest.run(callback1, jest.fn()); + // first start + underTest.start(callback1, jest.fn()); }); - // check first run triggered underlying data sources - expect(mockInitializer1.run).toHaveBeenCalledTimes(1); - expect(mockSynchronizer1.run).toHaveBeenCalledTimes(1); + // check first start triggered underlying data sources + expect(mockInitializer1.start).toHaveBeenCalledTimes(1); + expect(mockSynchronizer1.start).toHaveBeenCalledTimes(1); expect(callback1).toHaveBeenCalledTimes(2); // wait a moment for pending awaits to resolve the stop request @@ -399,18 +399,18 @@ it('can be stopped and restarted', async () => { let callback2; await new Promise((resolve) => { - callback2 = jest.fn((_: boolean, data: Data) => { + callback2 = jest.fn((_: boolean, data: any) => { if (data === mockSynchronizer1Data) { resolve(); } }); - // second run - underTest.run(callback2, jest.fn()); + // second start + underTest.start(callback2, jest.fn()); }); - // check that second run triggers underlying data sources again - expect(mockInitializer1.run).toHaveBeenCalledTimes(2); - expect(mockSynchronizer1.run).toHaveBeenCalledTimes(2); + // check that second start triggers underlying data sources again + expect(mockInitializer1.start).toHaveBeenCalledTimes(2); + expect(mockSynchronizer1.start).toHaveBeenCalledTimes(2); expect(callback2).toHaveBeenCalledTimes(2); }); @@ -418,6 +418,7 @@ it('is well behaved with no initializers and no synchronizers configured', async const underTest = new CompositeDataSource( [], [], + undefined, makeTestTransitionConditions(), makeZeroBackoff(), ); @@ -428,7 +429,7 @@ it('is well behaved with no initializers and no synchronizers configured', async resolve(); }); - underTest.run(jest.fn(), statusCallback); + underTest.start(jest.fn(), statusCallback); }); expect(statusCallback).toHaveBeenNthCalledWith(1, DataSourceState.Closed, { @@ -438,13 +439,49 @@ it('is well behaved with no initializers and no synchronizers configured', async }); }); +it('is well behaved with no initializer and synchronizer configured', async () => { + const mockSynchronizer1Data = { key: 'sync1' }; + const mockSynchronizer1 = { + start: jest + .fn() + .mockImplementation( + ( + _dataCallback: (basis: boolean, data: any) => void, + _statusCallback: (status: DataSourceState, err?: any) => void, + ) => { + _dataCallback(false, mockSynchronizer1Data); + }, + ), + stop: jest.fn(), + }; + + const underTest = new CompositeDataSource( + [], + [makeSynchronizerFactory(mockSynchronizer1)], + undefined, + makeTestTransitionConditions(), + makeZeroBackoff(), + ); + + let dataCallback; + await new Promise((resolve) => { + dataCallback = jest.fn(() => { + resolve(); + }); + + underTest.start(dataCallback, jest.fn()); + }); + + expect(dataCallback).toHaveBeenNthCalledWith(1, false, { key: 'sync1' }); +}); + it('is well behaved with an initializer and no synchronizers configured', async () => { const mockInitializer1 = { - run: jest + start: jest .fn() .mockImplementation( ( - _dataCallback: (basis: boolean, data: Data) => void, + _dataCallback: (basis: boolean, data: any) => void, _statusCallback: (status: DataSourceState, err?: any) => void, ) => { _dataCallback(true, { key: 'init1' }); @@ -456,19 +493,23 @@ it('is well behaved with an initializer and no synchronizers configured', async const underTest = new CompositeDataSource( [makeInitializerFactory(mockInitializer1)], [], + undefined, makeTestTransitionConditions(), makeZeroBackoff(), ); + let dataCallback; let statusCallback; await new Promise((resolve) => { + dataCallback = jest.fn(); statusCallback = jest.fn((_1: DataSourceState, _2: any) => { resolve(); }); - underTest.run(jest.fn(), statusCallback); + underTest.start(dataCallback, statusCallback); }); + expect(dataCallback).toHaveBeenNthCalledWith(1, true, { key: 'init1' }); expect(statusCallback).toHaveBeenNthCalledWith(1, DataSourceState.Closed, { name: 'ExhaustedDataSources', message: diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts index 12ec52ce79..5ae69fb1c6 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/CallbackHandler.ts @@ -1,4 +1,4 @@ -import { Data, DataSourceState } from './DataSource'; +import { DataSourceState } from './DataSource'; /** * Handler that connects the current {@link DataSource} to the {@link CompositeDataSource}. A single @@ -9,7 +9,7 @@ export class CallbackHandler { private _disabled: boolean = false; constructor( - private readonly _dataCallback: (basis: boolean, data: Data) => void, + private readonly _dataCallback: (basis: boolean, data: any) => void, private readonly _statusCallback: (status: DataSourceState, err?: any) => void, ) {} @@ -17,7 +17,7 @@ export class CallbackHandler { this._disabled = true; } - async dataHandler(basis: boolean, data: Data) { + async dataHandler(basis: boolean, data: any) { if (this._disabled) { return; } diff --git a/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts b/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts index 80885a401e..f7a5834761 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/DataSource.ts @@ -1,10 +1,12 @@ -export interface Data {} - // TODO: refactor client-sdk to use this enum export enum DataSourceState { + // Spinning up to make first connection attempt Initializing, + // Positive confirmation of connection/data receipt Valid, + // Transient issue, automatic retry is expected Interrupted, + // Permanent issue, external intervention required Closed, } @@ -15,8 +17,8 @@ export interface DataSource { * @param statusCallback that will be called when data source state changes or an unrecoverable error * has been encountered. */ - run( - dataCallback: (basis: boolean, data: Data) => void, + start( + dataCallback: (basis: boolean, data: any) => void, statusCallback: (status: DataSourceState, err?: any) => void, ): void; @@ -26,6 +28,10 @@ export interface DataSource { stop(): void; } +export type LDInitializerFactory = () => DataSystemInitializer; + +export type LDSynchronizerFactory = () => DataSystemSynchronizer; + /** * A data source that can be used to fetch the basis. */ @@ -35,11 +41,3 @@ export interface DataSystemInitializer extends DataSource {} * A data source that can be used to fetch the basis or ongoing data changes. */ export interface DataSystemSynchronizer extends DataSource {} - -export interface InitializerFactory { - create(): DataSystemInitializer; -} - -export interface SynchronizerFactory { - create(): DataSystemSynchronizer; -} diff --git a/packages/shared/common/src/api/subsystem/DataSystem/index.ts b/packages/shared/common/src/api/subsystem/DataSystem/index.ts index d49e3438fd..3ad2737e2d 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/index.ts +++ b/packages/shared/common/src/api/subsystem/DataSystem/index.ts @@ -1,7 +1,8 @@ export { + DataSource, + DataSourceState, DataSystemInitializer, DataSystemSynchronizer, - InitializerFactory, - SynchronizerFactory, + LDInitializerFactory, + LDSynchronizerFactory, } from './DataSource'; -export { CompositeDataSource } from './CompositeDataSource'; diff --git a/packages/shared/common/src/api/subsystem/index.ts b/packages/shared/common/src/api/subsystem/index.ts index 000f60f686..7dc2af969b 100644 --- a/packages/shared/common/src/api/subsystem/index.ts +++ b/packages/shared/common/src/api/subsystem/index.ts @@ -1,9 +1,23 @@ +import { + DataSource, + DataSourceState, + DataSystemInitializer, + DataSystemSynchronizer, + LDInitializerFactory, + LDSynchronizerFactory, +} from './DataSystem'; import LDContextDeduplicator from './LDContextDeduplicator'; import LDEventProcessor from './LDEventProcessor'; import LDEventSender, { LDDeliveryStatus, LDEventSenderResult, LDEventType } from './LDEventSender'; import { LDStreamProcessor } from './LDStreamProcessor'; export { + DataSource, + DataSourceState, + DataSystemInitializer, + DataSystemSynchronizer, + LDInitializerFactory, + LDSynchronizerFactory, LDEventProcessor, LDContextDeduplicator, LDEventSender, diff --git a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts b/packages/shared/common/src/datasource/CompositeDataSource.ts similarity index 86% rename from packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts rename to packages/shared/common/src/datasource/CompositeDataSource.ts index d77c3916a3..e68dc6f7dd 100644 --- a/packages/shared/common/src/api/subsystem/DataSystem/CompositeDataSource.ts +++ b/packages/shared/common/src/datasource/CompositeDataSource.ts @@ -1,19 +1,15 @@ /* eslint-disable no-await-in-loop */ -import { Backoff } from '../../../datasource/Backoff'; -import { LDLogger } from '../../logging'; -import { CallbackHandler } from './CallbackHandler'; +import { LDLogger } from '../api/logging'; +import { CallbackHandler } from '../api/subsystem/DataSystem/CallbackHandler'; import { - Data, DataSource, DataSourceState, - InitializerFactory, - SynchronizerFactory, -} from './DataSource'; + LDInitializerFactory, + LDSynchronizerFactory, +} from '../api/subsystem/DataSystem/DataSource'; +import { Backoff, DefaultBackoff } from './Backoff'; -// TODO: SDK-858, specify these constants when CompositeDataSource is used. -// eslint-disable-next-line @typescript-eslint/no-unused-vars const DEFAULT_FALLBACK_TIME_MS = 2 * 60 * 1000; -// eslint-disable-next-line @typescript-eslint/no-unused-vars const DEFAULT_RECOVERY_TIME_MS = 5 * 60 * 1000; /** @@ -41,8 +37,8 @@ export class CompositeDataSource implements DataSource { // TODO: SDK-856 async notification if initializer takes too long // TODO: SDK-1044 utilize selector from initializers - private _initPhaseActive: boolean = true; - private _currentPosition: number = 0; + private _initPhaseActive: boolean; + private _currentPosition: number; private _stopped: boolean = true; private _externalTransitionPromise: Promise; @@ -54,21 +50,33 @@ export class CompositeDataSource implements DataSource { * @param _synchronizers factories to create {@link DataSystemSynchronizer}s, in priority order. */ constructor( - private readonly _initializers: InitializerFactory[], - private readonly _synchronizers: SynchronizerFactory[], - private readonly _transitionConditions: TransitionConditions, - private readonly _backoff: Backoff, + private readonly _initializers: LDInitializerFactory[], + private readonly _synchronizers: LDSynchronizerFactory[], private readonly _logger?: LDLogger, + private readonly _transitionConditions: TransitionConditions = { + [DataSourceState.Valid]: { + durationMS: DEFAULT_RECOVERY_TIME_MS, + transition: 'recover', + }, + [DataSourceState.Interrupted]: { + durationMS: DEFAULT_FALLBACK_TIME_MS, + transition: 'fallback', + }, + }, + private readonly _backoff: Backoff = new DefaultBackoff( + 1000, // TODO SDK-1137: handle blacklisting perpetually failing sources + 30000, + ), ) { this._externalTransitionPromise = new Promise((resolveTransition) => { this._externalTransitionResolve = resolveTransition; }); - this._initPhaseActive = true; + this._initPhaseActive = _initializers.length > 0; // init phase if we have initializers this._currentPosition = 0; } - async run( - dataCallback: (basis: boolean, data: Data) => void, + async start( + dataCallback: (basis: boolean, data: any) => void, statusCallback: (status: DataSourceState, err?: any) => void, ): Promise { if (!this._stopped) { @@ -78,6 +86,9 @@ export class CompositeDataSource implements DataSource { } this._stopped = false; + this._logger?.debug( + `CompositeDataSource starting with (${this._initializers.length} initializers, ${this._synchronizers.length} synchronizers).`, + ); let lastTransition: Transition = 'none'; // eslint-disable-next-line no-constant-condition while (true) { @@ -91,7 +102,7 @@ export class CompositeDataSource implements DataSource { // this callback handler can be disabled and ensures only one transition request occurs const callbackHandler = new CallbackHandler( - (basis: boolean, data: Data) => { + (basis: boolean, data: any) => { this._backoff.success(); dataCallback(basis, data); if (basis && this._initPhaseActive) { @@ -105,7 +116,9 @@ export class CompositeDataSource implements DataSource { // When we get a status update, we want to fallback if it is an error. We also want to schedule a transition for some // time in the future if this status remains for some duration (ex: Recover to primary synchronizer after the secondary // synchronizer has been Valid for some time). These scheduled transitions are configurable in the constructor. - + this._logger?.debug( + `CompositeDataSource received state ${state} from underlying data source.`, + ); if (err || state === DataSourceState.Closed) { callbackHandler.disable(); statusCallback(DataSourceState.Interrupted, err); // underlying errors or closed states are masked as interrupted while we transition @@ -133,7 +146,7 @@ export class CompositeDataSource implements DataSource { } }, ); - currentDS.run( + currentDS.start( (basis, data) => callbackHandler.dataHandler(basis, data), (status, err) => callbackHandler.statusHandler(status, err), ); @@ -197,7 +210,7 @@ export class CompositeDataSource implements DataSource { private _reset() { this._stopped = true; - this._initPhaseActive = true; + this._initPhaseActive = this._initializers.length > 0; // init phase if we have initializers; this._currentPosition = 0; this._externalTransitionPromise = new Promise((tr) => { this._externalTransitionResolve = tr; @@ -229,7 +242,7 @@ export class CompositeDataSource implements DataSource { return undefined; } - return this._initializers[this._currentPosition].create(); + return this._initializers[this._currentPosition](); } // getting here indicates we are using a synchronizer @@ -242,7 +255,7 @@ export class CompositeDataSource implements DataSource { // this is only possible if no synchronizers were provided return undefined; } - return this._synchronizers[this._currentPosition].create(); + return this._synchronizers[this._currentPosition](); } /** diff --git a/packages/shared/common/src/datasource/index.ts b/packages/shared/common/src/datasource/index.ts index e727947dce..237da787e0 100644 --- a/packages/shared/common/src/datasource/index.ts +++ b/packages/shared/common/src/datasource/index.ts @@ -1,4 +1,5 @@ import { Backoff, DefaultBackoff } from './Backoff'; +import { CompositeDataSource } from './CompositeDataSource'; import { DataSourceErrorKind } from './DataSourceErrorKinds'; import { LDFileDataSourceError, @@ -9,6 +10,7 @@ import { export { Backoff, + CompositeDataSource, DefaultBackoff, DataSourceErrorKind, LDFileDataSourceError, diff --git a/packages/shared/common/src/index.ts b/packages/shared/common/src/index.ts index 7df308f674..060ed1bf5a 100644 --- a/packages/shared/common/src/index.ts +++ b/packages/shared/common/src/index.ts @@ -3,6 +3,7 @@ import Context from './Context'; import ContextFilter from './ContextFilter'; import { Backoff, + CompositeDataSource, DataSourceErrorKind, DefaultBackoff, LDFileDataSourceError, @@ -24,6 +25,7 @@ export { AttributeReference, Context, ContextFilter, + CompositeDataSource, DataSourceErrorKind, Backoff, DefaultBackoff, From a3d2489d47d2d9d922c2a4bd46eaf731772cc932 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 4 Mar 2025 15:46:29 -0600 Subject: [PATCH 09/14] chore: Adds LDDataSystemOptions for configuring the Data System. --- .../src/datasource/CompositeDataSource.ts | 1 + .../__tests__/options/Configuration.test.ts | 4 + .../shared/sdk-server/src/LDClientImpl.ts | 52 +++--- .../src/api/options/LDDataSystemOptions.ts | 137 +++++++++++++++ .../sdk-server/src/api/options/LDOptions.ts | 65 +++++++- .../sdk-server/src/api/options/index.ts | 1 + .../src/data_sources/PollingProcessor.ts | 13 +- .../createDiagnosticsInitConfig.ts | 18 +- .../sdk-server/src/options/Configuration.ts | 156 ++++++++++++++++-- .../src/options/ValidatedOptions.ts | 2 + 10 files changed, 398 insertions(+), 51 deletions(-) create mode 100644 packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts diff --git a/packages/shared/common/src/datasource/CompositeDataSource.ts b/packages/shared/common/src/datasource/CompositeDataSource.ts index e68dc6f7dd..a694b4e339 100644 --- a/packages/shared/common/src/datasource/CompositeDataSource.ts +++ b/packages/shared/common/src/datasource/CompositeDataSource.ts @@ -29,6 +29,7 @@ interface TransitionRequest { err?: Error; } +// TODO SDK-858: move this out of API directory to neighbor datasource folder /** * The {@link CompositeDataSource} can combine a number of {@link DataSystemInitializer}s and {@link DataSystemSynchronizer}s * into a single {@link DataSource}, implementing fallback and recovery logic internally to choose where data is sourced from. diff --git a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts index 399439be73..70442be8e9 100644 --- a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts +++ b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts @@ -408,4 +408,8 @@ describe('when setting different options', () => { }, ]); }); + + it('drop', () => { + + } }); diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 44a355158e..f46fde5ab2 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -30,6 +30,11 @@ import { import { Hook } from './api/integrations/Hook'; import { BigSegmentStoreMembership } from './api/interfaces'; import { LDWaitForInitializationOptions } from './api/LDWaitForInitializationOptions'; +import { + isPollingOptions, + isStandardOptions, + isStreamingOptions, +} from './api/options/LDDataSystemOptions'; import BigSegmentsManager from './BigSegmentsManager'; import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; import { createStreamListeners } from './data_sources/createStreamListeners'; @@ -219,25 +224,34 @@ export default class LDClientImpl implements LDClient { const listeners = createStreamListeners(dataSourceUpdates, this._logger, { put: () => this._initSuccess(), }); - const makeDefaultProcessor = () => - config.stream - ? new StreamingProcessor( - clientContext, - '/all', - [], - listeners, - baseHeaders, - this._diagnosticsManager, - (e) => this._dataSourceErrorHandler(e), - this._config.streamInitialReconnectDelay, - ) - : new PollingProcessor( - config, - new Requestor(config, this._platform.requests, baseHeaders), - dataSourceUpdates, - () => this._initSuccess(), - (e) => this._dataSourceErrorHandler(e), - ); + const makeDefaultProcessor = () => { + if (isPollingOptions(config.dataSystem.dataSource)) { + return new PollingProcessor( + new Requestor(config, this._platform.requests, baseHeaders), + config.dataSystem.dataSource.pollInterval ?? 30, + dataSourceUpdates, + config.logger, + () => this._initSuccess(), + (e) => this._dataSourceErrorHandler(e), + ); + } + // TODO: SDK-858 Hook up composite data source and config + const reconnectDelay = + isStandardOptions(config.dataSystem.dataSource) || + isStreamingOptions(config.dataSystem.dataSource) + ? config.dataSystem.dataSource.streamInitialReconnectDelay + : 1; + return new StreamingProcessor( + clientContext, + '/all', + [], + listeners, + baseHeaders, + this._diagnosticsManager, + (e) => this._dataSourceErrorHandler(e), + reconnectDelay, + ); + }; if (!(config.offline || config.useLdd)) { this._updateProcessor = diff --git a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts new file mode 100644 index 0000000000..33ad8b27cb --- /dev/null +++ b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts @@ -0,0 +1,137 @@ +import { LDClientContext, subsystem } from '@launchdarkly/js-sdk-common'; + +import { LDDataSourceUpdates, LDFeatureStore } from '../subsystems'; + +/** + * Configuration options for the Data System that the SDK uses to get and maintain flags and other + * data from LaunchDarkly and other sources. + * + * Example (Recommended): + * ```typescript + * let dataSystemOptions = { + * dataSource: { + * type: 'standard'; + * }, + * } + * + * Example (Polling with DynamoDB Persistent Store): + * ```typescript + * import { DynamoDBFeatureStore } from '@launchdarkly/node-server-sdk-dynamodb'; + * + * let dataSystemOptions = { + * dataSource: { + * type: 'polling'; + * pollInterval: 300; + * }, + * persistentStore: DynamoDBFeatureStore('your-table', { cacheTTL: 30 }); + * } + * const client = init('my-sdk-key', { hooks: [new TracingHook()] }); + * ``` + */ +export interface LDDataSystemOptions { + /** + * Configuration options for the Data Source that the SDK uses to get flags and other + * data from the LaunchDarkly servers. Choose one of {@link StandardDataSourceOptions}, + * {@link StreamingDataSourceOptions}, or {@link PollingDataSourceOptions}; setting the + * type and the optional fields you want to customize. + * + * If not specified, this defaults to using the {@link StandardDataSourceOptions} which + * pefroms a combination of streaming and polling. + * + * See {@link LDDataSystemOptions} decoumentation for exmaples. + */ + dataSource?: DataSourceOptions; + + /** + * A component that obtains feature flag data and puts it in the feature store. Setting + * this supercedes {@link LDDataSystemOptions#dataSource}. + */ + updateProcessor?: + | object + | (( + clientContext: LDClientContext, + dataSourceUpdates: LDDataSourceUpdates, + initSuccessHandler: VoidFunction, + errorHandler?: (e: Error) => void, + ) => subsystem.LDStreamProcessor); + + /** + * Before data has arrived from LaunchDarkly, the SDK is able to evaluate flags using + * data from the persistent store. Once fresh data is available, the SDK will no longer + * read from the persistent store, although it will keep it up-to-date for future startups. + */ + persistentStore?: LDFeatureStore | ((clientContext: LDClientContext) => LDFeatureStore); +} + +export type DataSourceOptions = + | StandardDataSourceOptions + | StreamingDataSourceOptions + | PollingDataSourceOptions; + +/** + * This standard data source is the recommended datasource for most customers. It will use + * a combination of streaming and polling to initialize the SDK, provide real time updates, + * and can switch between streaming and polling automatically to provide redundancy. + */ +export interface StandardDataSourceOptions { + type: 'standard'; + + /** + * Sets the initial reconnect delay for the streaming connection, in seconds. Default if omitted. + * + * The streaming service uses a backoff algorithm (with jitter) every time the connection needs + * to be reestablished. The delay for the first reconnection will start near this value, and then + * increase exponentially for any subsequent connection failures. + * + * The default value is 1. + */ + streamInitialReconnectDelay?: number; + + /** + * The time between polling requests, in seconds. Default if omitted. + */ + pollInterval?: number; +} + +/** + * This data source will make best effort to maintain a streaming connection to LaunchDarkly services + * to provide real time data updates. + */ +export interface StreamingDataSourceOptions { + type: 'streaming'; + + /** + * Sets the initial reconnect delay for the streaming connection, in seconds. Default if omitted. + * + * The streaming service uses a backoff algorithm (with jitter) every time the connection needs + * to be reestablished. The delay for the first reconnection will start near this value, and then + * increase exponentially up to a maximum for any subsequent connection failures. + * + * The default value is 1. + */ + streamInitialReconnectDelay?: number; +} + +/** + * This data source will periodically make a request to LaunchDarkly services to retrieve updated data. + */ +export interface PollingDataSourceOptions { + type: 'polling'; + + /** + * The time between polling requests, in seconds. Default if omitted. + */ + pollInterval?: number; +} + +export function isStandardOptions(u: any): u is StandardDataSourceOptions { + return u.kind === 'standard'; +} + +export function isStreamingOptions(u: any): u is StreamingDataSourceOptions { + return u.kind === 'streaming'; +} + +export function isPollingOptions(u: any): u is PollingDataSourceOptions { + return u.kind === 'polling'; +} diff --git a/packages/shared/sdk-server/src/api/options/LDOptions.ts b/packages/shared/sdk-server/src/api/options/LDOptions.ts index 7697839730..b109f6271a 100644 --- a/packages/shared/sdk-server/src/api/options/LDOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDOptions.ts @@ -3,6 +3,7 @@ import { LDClientContext, LDLogger, subsystem, VoidFunction } from '@launchdarkl import { Hook } from '../integrations/Hook'; import { LDDataSourceUpdates, LDFeatureStore } from '../subsystems'; import { LDBigSegmentsOptions } from './LDBigSegmentsOptions'; +import { LDDataSystemOptions } from './LDDataSystemOptions'; import { LDProxyOptions } from './LDProxyOptions'; import { LDTLSOptions } from './LDTLSOptions'; @@ -67,9 +68,48 @@ export interface LDOptions { * Some implementations provide the store implementation object itself, while others * provide a factory function that creates the store implementation based on the SDK * configuration; this property accepts either. + * + * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. + * + * @deprecated This is now superceded by {@link LDOptions#dataSystem} using property + * {@link LDDataSystemOptions#persistentStore}. The scope of the {@link LDFeatureStore} + * that you provide is being reduced in the next major version to only being responsible + * for persistence. See documention on {@link LDDataSystemOptions#persistentStore} for + * more information. */ featureStore?: LDFeatureStore | ((clientContext: LDClientContext) => LDFeatureStore); + /** + * Configuration options for the Data System that the SDK uses to get and maintain flags and other + * data from LaunchDarkly and other sources. + * + * Setting this option supercedes + * + * Example (Recommended): + * ```typescript + * let dataSystemOptions = { + * dataSource: { + * type: 'standard'; + * // options can be customized here, though defaults are recommended + * }, + * } + * + * Example (Polling with DynamoDB Persistent Store): + * ```typescript + * import { DynamoDBFeatureStore } from '@launchdarkly/node-server-sdk-dynamodb'; + * + * let dataSystemOptions = { + * dataSource: { + * type: 'polling'; + * pollInterval: 300; + * }, + * persistentStore: DynamoDBFeatureStore('your-table', { cacheTTL: 30 }); + * } + * const client = init('my-sdk-key', { hooks: [new TracingHook()] }); + * ``` + */ + dataSystem?: LDDataSystemOptions; + /** * Additional parameters for configuring the SDK's Big Segments behavior. * @@ -86,7 +126,10 @@ export interface LDOptions { /** * A component that obtains feature flag data and puts it in the feature store. * - * By default, this is the client's default streaming or polling component. + * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. + * + * @deprecated This has moved to {@link LDOptions#dataSystem}. Use property + * {@link LDDataSystemOptions#updateProcessor} instead. */ updateProcessor?: | object @@ -104,6 +147,12 @@ export interface LDOptions { /** * The time between polling requests, in seconds. Ignored in streaming mode. + * + * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. + * + * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} by + * specifying the {@link LDDataSystemOptions#dataSource}. Specifying the polling interval is still + * available when using {@link StandardDataSourceOptions} or {@link PollingDataSourceOptions}. */ pollInterval?: number; @@ -122,6 +171,13 @@ export interface LDOptions { * * This is true by default. If you set it to false, the client will use polling. * Streaming should only be disabled on the advice of LaunchDarkly support. + * + * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. + * + * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} + * by specifying {@link LDDataSystemOptions#dataSource} options. To opt out of + * streaming, you can use the {@link PollingDataSourceOptions}. Streaming should + * only be disabled on the advice of LaunchDarkly support. */ stream?: boolean; @@ -133,6 +189,13 @@ export interface LDOptions { * increase exponentially for any subsequent connection failures. * * The default value is 1. + * + * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. + * + * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} + * by specifying {@link LDDataSystemOptions#dataSource} options. Specifying the reconnect + * delay is still available when using {@link StandardDataSourceOptions} or + * {@link StreamingDataSourceOptions}. */ streamInitialReconnectDelay?: number; diff --git a/packages/shared/sdk-server/src/api/options/index.ts b/packages/shared/sdk-server/src/api/options/index.ts index 1e7b63de7b..44b81cc99c 100644 --- a/packages/shared/sdk-server/src/api/options/index.ts +++ b/packages/shared/sdk-server/src/api/options/index.ts @@ -3,3 +3,4 @@ export * from './LDOptions'; export * from './LDProxyOptions'; export * from './LDTLSOptions'; export * from './LDMigrationOptions'; +export * from './LDDataSystemOptions'; diff --git a/packages/shared/sdk-server/src/data_sources/PollingProcessor.ts b/packages/shared/sdk-server/src/data_sources/PollingProcessor.ts index d376b45354..0f264ec047 100644 --- a/packages/shared/sdk-server/src/data_sources/PollingProcessor.ts +++ b/packages/shared/sdk-server/src/data_sources/PollingProcessor.ts @@ -13,6 +13,7 @@ import Configuration from '../options/Configuration'; import { deserializePoll } from '../store'; import VersionedDataKinds from '../store/VersionedDataKinds'; import Requestor from './Requestor'; +import { isPollingOptions, isStandardOptions } from '../api/options/LDDataSystemOptions'; export type PollingErrorHandler = (err: LDPollingError) => void; @@ -22,22 +23,16 @@ export type PollingErrorHandler = (err: LDPollingError) => void; export default class PollingProcessor implements subsystem.LDStreamProcessor { private _stopped = false; - private _logger?: LDLogger; - - private _pollInterval: number; - private _timeoutHandle: any; constructor( - config: Configuration, private readonly _requestor: Requestor, + private readonly _pollInterval: number, private readonly _featureStore: LDDataSourceUpdates, + private readonly _logger?: LDLogger, private readonly _initSuccessHandler: VoidFunction = () => {}, private readonly _errorHandler?: PollingErrorHandler, - ) { - this._logger = config.logger; - this._pollInterval = config.pollInterval; - } + ) {} private _poll() { if (this._stopped) { diff --git a/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts b/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts index 764efef6ea..5e61f5eb0c 100644 --- a/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts +++ b/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts @@ -1,6 +1,6 @@ import { Platform, secondsToMillis } from '@launchdarkly/js-sdk-common'; -import { LDFeatureStore } from '../api'; +import { isPollingOptions, isStandardOptions, isStreamingOptions, LDFeatureStore } from '../api'; import Configuration, { defaultValues } from '../options/Configuration'; const createDiagnosticsInitConfig = ( @@ -18,12 +18,22 @@ const createDiagnosticsInitConfig = ( connectTimeoutMillis: secondsToMillis(config.timeout), socketTimeoutMillis: secondsToMillis(config.timeout), eventsFlushIntervalMillis: secondsToMillis(config.flushInterval), - pollingIntervalMillis: secondsToMillis(config.pollInterval), - reconnectTimeMillis: secondsToMillis(config.streamInitialReconnectDelay), + // include polling interval if data source config has it + ...((isStandardOptions(config.dataSystem.dataSource) || + isPollingOptions(config.dataSystem.dataSource)) && + config.dataSystem.dataSource.pollInterval + ? { pollingIntervalMillis: config.dataSystem.dataSource.pollInterval } + : null), + // include reconnect delay if data source config has it + ...((isStandardOptions(config.dataSystem.dataSource) || + isStreamingOptions(config.dataSystem.dataSource)) && + config.dataSystem.dataSource.streamInitialReconnectDelay + ? { reconnectTimeMillis: config.dataSystem.dataSource.streamInitialReconnectDelay } + : null), contextKeysFlushIntervalMillis: secondsToMillis(config.contextKeysFlushInterval), diagnosticRecordingIntervalMillis: secondsToMillis(config.diagnosticRecordingInterval), - streamingDisabled: !config.stream, + streamingDisabled: isPollingOptions(config.dataSystem.dataSource), usingRelayDaemon: config.useLdd, offline: config.offline, allAttributesPrivate: config.allAttributesPrivate, diff --git a/packages/shared/sdk-server/src/options/Configuration.ts b/packages/shared/sdk-server/src/options/Configuration.ts index fe14a43f53..ce04388d59 100644 --- a/packages/shared/sdk-server/src/options/Configuration.ts +++ b/packages/shared/sdk-server/src/options/Configuration.ts @@ -14,6 +14,15 @@ import { import { LDBigSegmentsOptions, LDOptions, LDProxyOptions, LDTLSOptions } from '../api'; import { Hook } from '../api/integrations'; +import { + isPollingOptions, + isStandardOptions, + isStreamingOptions, + LDDataSystemOptions, + PollingDataSourceOptions, + StandardDataSourceOptions, + StreamingDataSourceOptions, +} from '../api/options/LDDataSystemOptions'; import { LDDataSourceUpdates, LDFeatureStore } from '../api/subsystems'; import InMemoryFeatureStore from '../store/InMemoryFeatureStore'; import { ValidatedOptions } from './ValidatedOptions'; @@ -35,6 +44,7 @@ const validations: Record = { capacity: TypeValidators.Number, logger: TypeValidators.Object, featureStore: TypeValidators.ObjectOrFactory, + dataSystem: TypeValidators.Object, bigSegments: TypeValidators.Object, updateProcessor: TypeValidators.ObjectOrFactory, flushInterval: TypeValidators.Number, @@ -57,6 +67,30 @@ const validations: Record = { application: TypeValidators.Object, payloadFilterKey: TypeValidators.stringMatchingRegex(/^[a-zA-Z0-9](\w|\.|-)*$/), hooks: TypeValidators.createTypeArray('Hook[]', {}), + type: TypeValidators.String, +}; + +const DEFAULT_POLL_INTERVAL = 30; +const DEFAULT_STREAM_RECONNECT_DELAY = 1; + +const defaultStandardDataSourceOptions: StandardDataSourceOptions = { + type: 'standard', + streamInitialReconnectDelay: DEFAULT_STREAM_RECONNECT_DELAY, + pollInterval: DEFAULT_POLL_INTERVAL, +}; + +const defaultStreamingDataSourceOptions: StreamingDataSourceOptions = { + type: 'streaming', + streamInitialReconnectDelay: DEFAULT_STREAM_RECONNECT_DELAY, +}; + +const defaultPollingDataSourceOptions: PollingDataSourceOptions = { + type: 'polling', + pollInterval: DEFAULT_POLL_INTERVAL, +}; + +const defaultDataSystemOptions = { + dataSource: defaultStandardDataSourceOptions, }; /** @@ -67,12 +101,12 @@ export const defaultValues: ValidatedOptions = { streamUri: 'https://stream.launchdarkly.com', eventsUri: ServiceEndpoints.DEFAULT_EVENTS, stream: true, - streamInitialReconnectDelay: 1, + streamInitialReconnectDelay: DEFAULT_STREAM_RECONNECT_DELAY, sendEvents: true, timeout: 5, capacity: 10000, flushInterval: 5, - pollInterval: 30, + pollInterval: DEFAULT_POLL_INTERVAL, offline: false, useLdd: false, allAttributesPrivate: false, @@ -82,14 +116,23 @@ export const defaultValues: ValidatedOptions = { diagnosticOptOut: false, diagnosticRecordingInterval: 900, featureStore: () => new InMemoryFeatureStore(), + dataSystem: defaultDataSystemOptions, +}; + +// General options type needed by validation algorithm. Specific types can be asserted after use. +type Options = { + [k: string]: any; }; -function validateTypesAndNames(options: LDOptions): { +function validateTypesAndNames( + options: Options, + defaults: Options, +): { errors: string[]; - validatedOptions: ValidatedOptions; + validatedOptions: Options; } { const errors: string[] = []; - const validatedOptions: ValidatedOptions = { ...defaultValues }; + const validatedOptions: Options = { ...defaults }; Object.keys(options).forEach((optionName) => { // We need to tell typescript it doesn't actually know what options are. // If we don't then it complains we are doing crazy things with it. @@ -123,7 +166,7 @@ function validateTypesAndNames(options: LDOptions): { return { errors, validatedOptions }; } -function validateEndpoints(options: LDOptions, validatedOptions: ValidatedOptions) { +function validateEndpoints(options: LDOptions, validatedOptions: Options) { const { baseUri, streamUri, eventsUri } = options; const streamingEndpointSpecified = streamUri !== undefined && streamUri !== null; const pollingEndpointSpecified = baseUri !== undefined && baseUri !== null; @@ -150,6 +193,62 @@ function validateEndpoints(options: LDOptions, validatedOptions: ValidatedOption } } +function validateDataSystemOptions(options: Options): { + errors: string[]; + validatedOptions: Options; +} { + const allErrors: string[] = []; + const validatedOptions: Options = { ...options }; + + if (!TypeValidators.ObjectOrFactory.is(options.persistentStore)) { + validatedOptions.persistentStore = undefined; // default is to not use this + allErrors.push( + OptionMessages.wrongOptionType( + 'persistentStore', + 'LDFeatureStore', + typeof options.persistentStore, + ), + ); + } + + if (!TypeValidators.ObjectOrFactory.is(options.updateProcessor)) { + validatedOptions.persistentStore = undefined; // default is to not use this + allErrors.push( + OptionMessages.wrongOptionType( + 'updateProcessor', + 'UpdateProcessor', + typeof options.persistentStore, + ), + ); + } + + let result: { errors: string[]; validatedOptions: Options }; + let validatedDataSourceOptions; + if (isStandardOptions(options.dataSource)) { + result = validateTypesAndNames(options.dataSource, defaultStandardDataSourceOptions); + } else if (isStreamingOptions(options.dataSource)) { + result = validateTypesAndNames(options.dataSource, defaultStreamingDataSourceOptions); + } else if (isPollingOptions(options.dataSource)) { + result = validateTypesAndNames(options.dataSource, defaultPollingDataSourceOptions); + } else { + // provided datasource options don't fit any expected form, drop them and use defaults + result = { + errors: [ + OptionMessages.wrongOptionType( + 'dataSource', + 'DataSourceOptions', + typeof options.dataSource, + ), + ], + validatedOptions: defaultStandardDataSourceOptions, + }; + } + + validatedOptions.dataSource = validatedDataSourceOptions; + allErrors.concat(result.errors); + return { errors: allErrors, validatedOptions }; +} + /** * Configuration options for the LDClient. * @@ -166,16 +265,10 @@ export default class Configuration { public readonly flushInterval: number; - public readonly pollInterval: number; - public readonly proxyOptions?: LDProxyOptions; public readonly offline: boolean; - public readonly stream: boolean; - - public readonly streamInitialReconnectDelay: number; - public readonly useLdd: boolean; public readonly sendEvents: boolean; @@ -204,6 +297,8 @@ export default class Configuration { public readonly featureStoreFactory: (clientContext: LDClientContext) => LDFeatureStore; + public readonly dataSystem: LDDataSystemOptions; + public readonly updateProcessorFactory?: ( clientContext: LDClientContext, dataSourceUpdates: LDDataSourceUpdates, @@ -223,13 +318,42 @@ export default class Configuration { // If there isn't a valid logger from the platform, then logs would go nowhere. this.logger = options.logger; - const { errors, validatedOptions } = validateTypesAndNames(options); + const { errors, validatedOptions: topLevelResult } = validateTypesAndNames( + options, + defaultValues, + ); + const validatedOptions = topLevelResult as ValidatedOptions; errors.forEach((error) => { this.logger?.warn(error); }); validateEndpoints(options, validatedOptions); + if (options.dataSystem) { + // validate the data system options, this will also apply reasonable defaults + const { errors: dsErrors, validatedOptions: dsResult } = validateDataSystemOptions( + options.dataSystem, + ); + this.dataSystem = dsResult as LDDataSystemOptions; + dsErrors.forEach((error) => { + this.logger?.warn(error); + }); + } else { + // if no data system was specified (such as implmentations using this SDK before data system + // was added), we'll make one based on the stream option + this.dataSystem = { + dataSource: options.stream + ? { + type: 'streaming', + streamInitialReconnectDelay: validatedOptions.streamInitialReconnectDelay, + } + : { + type: 'polling', + pollInterval: validatedOptions.pollInterval, + }, + }; + } + this.serviceEndpoints = new ServiceEndpoints( validatedOptions.streamUri, validatedOptions.baseUri, @@ -244,7 +368,6 @@ export default class Configuration { this.bigSegments = validatedOptions.bigSegments; this.flushInterval = validatedOptions.flushInterval; - this.pollInterval = validatedOptions.pollInterval; this.proxyOptions = validatedOptions.proxyOptions; this.sendEvents = validatedOptions.sendEvents; @@ -259,10 +382,9 @@ export default class Configuration { this.wrapperVersion = validatedOptions.wrapperVersion; this.tags = new ApplicationTags(validatedOptions); this.diagnosticRecordingInterval = validatedOptions.diagnosticRecordingInterval; + this.hooks = validatedOptions.hooks; this.offline = validatedOptions.offline; - this.stream = validatedOptions.stream; - this.streamInitialReconnectDelay = validatedOptions.streamInitialReconnectDelay; this.useLdd = validatedOptions.useLdd; if (TypeValidators.Function.is(validatedOptions.updateProcessor)) { @@ -282,7 +404,5 @@ export default class Configuration { // @ts-ignore this.featureStoreFactory = () => validatedOptions.featureStore; } - - this.hooks = validatedOptions.hooks; } } diff --git a/packages/shared/sdk-server/src/options/ValidatedOptions.ts b/packages/shared/sdk-server/src/options/ValidatedOptions.ts index ce9a58de9b..e4ab735a6e 100644 --- a/packages/shared/sdk-server/src/options/ValidatedOptions.ts +++ b/packages/shared/sdk-server/src/options/ValidatedOptions.ts @@ -2,6 +2,7 @@ import { LDLogger, subsystem } from '@launchdarkly/js-sdk-common'; import { LDBigSegmentsOptions, LDOptions, LDProxyOptions, LDTLSOptions } from '../api'; import { Hook } from '../api/integrations'; +import { LDDataSystemOptions } from '../api/options/LDDataSystemOptions'; import { LDFeatureStore } from '../api/subsystems'; /** @@ -30,6 +31,7 @@ export interface ValidatedOptions { diagnosticOptOut: boolean; diagnosticRecordingInterval: number; featureStore: LDFeatureStore | ((options: LDOptions) => LDFeatureStore); + dataSystem: LDDataSystemOptions; tlsParams?: LDTLSOptions; updateProcessor?: subsystem.LDStreamProcessor; wrapperName?: string; From d241af32f2a018943bb5dcd880b15af0ed80c2dc Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Mon, 17 Mar 2025 16:49:39 -0500 Subject: [PATCH 10/14] tests and some restructuring/fixes --- .../__tests__/options/Configuration.test.ts | 120 ++++++++++++- .../shared/sdk-server/src/LDClientImpl.ts | 6 +- .../src/api/options/LDDataSystemOptions.ts | 36 +++- .../sdk-server/src/api/options/LDOptions.ts | 16 +- .../sdk-server/src/options/Configuration.ts | 167 +++++++++++------- 5 files changed, 259 insertions(+), 86 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts index 70442be8e9..43784d26fd 100644 --- a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts +++ b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts @@ -1,5 +1,14 @@ -import { LDOptions } from '../../src'; +import { + ClientContext, + DataSourceOptions, + isStandardOptions, + LDFeatureStore, + LDOptions, + PollingDataSourceOptions, + StandardDataSourceOptions, +} from '../../src'; import Configuration from '../../src/options/Configuration'; +import InMemoryFeatureStore from '../../src/store/InMemoryFeatureStore'; import TestLogger, { LogLevel } from '../Logger'; function withLogger(options: LDOptions): LDOptions { @@ -25,15 +34,16 @@ describe.each([undefined, null, 'potat0', 17, [], {}])('constructed without opti expect(config.flushInterval).toBe(5); expect(config.logger).toBeUndefined(); expect(config.offline).toBe(false); - expect(config.pollInterval).toBe(30); + expect((config.dataSystem.dataSource as StandardDataSourceOptions).pollInterval).toEqual(30); expect(config.privateAttributes).toStrictEqual([]); expect(config.proxyOptions).toBeUndefined(); expect(config.sendEvents).toBe(true); expect(config.serviceEndpoints.streaming).toEqual('https://stream.launchdarkly.com'); expect(config.serviceEndpoints.polling).toEqual('https://sdk.launchdarkly.com'); expect(config.serviceEndpoints.events).toEqual('https://events.launchdarkly.com'); - expect(config.stream).toBe(true); - expect(config.streamInitialReconnectDelay).toEqual(1); + expect( + (config.dataSystem.dataSource as StandardDataSourceOptions).streamInitialReconnectDelay, + ).toEqual(1); expect(config.tags.value).toBeUndefined(); expect(config.timeout).toEqual(5); expect(config.tlsParams).toBeUndefined(); @@ -179,7 +189,9 @@ describe('when setting different options', () => { ])('allow setting and validates pollInterval', (value, expected, warnings) => { // @ts-ignore const config = new Configuration(withLogger({ pollInterval: value })); - expect(config.pollInterval).toEqual(expected); + expect((config.dataSystem.dataSource as StandardDataSourceOptions).pollInterval).toEqual( + expected, + ); expect(logger(config).getCount()).toEqual(warnings); }); @@ -207,7 +219,7 @@ describe('when setting different options', () => { ])('allows setting stream and validates stream', (value, expected, warnings) => { // @ts-ignore const config = new Configuration(withLogger({ stream: value })); - expect(config.stream).toEqual(expected); + expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(expected); expect(logger(config).getCount()).toEqual(warnings); }); @@ -409,7 +421,99 @@ describe('when setting different options', () => { ]); }); - it('drop', () => { + it('drops invalid dataystem data source options and replaces with defaults', () => { + const config = new Configuration( + withLogger({ + dataSystem: { dataSource: { bogus: 'myBogusOptions' } as unknown as DataSourceOptions }, + }), + ); + expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(true); + logger(config).expectMessages([ + { + level: LogLevel.Warn, + matches: /Config option "dataSource" should be of type DataSourceOptions/, + }, + ]); + }); - } + it('validates the datasystem persitent store is a factory or object', () => { + const config1 = new Configuration( + withLogger({ + dataSystem: { + persistentStore: () => new InMemoryFeatureStore(), + }, + }), + ); + expect(isStandardOptions(config1.dataSystem.dataSource)).toEqual(true); + expect(logger(config1).getCount()).toEqual(0); + + const config2 = new Configuration( + withLogger({ + dataSystem: { + persistentStore: 'bogus type' as unknown as LDFeatureStore, + }, + }), + ); + expect(isStandardOptions(config2.dataSystem.dataSource)).toEqual(true); + logger(config2).expectMessages([ + { + level: LogLevel.Warn, + matches: /Config option "persistentStore" should be of type LDFeatureStore/, + }, + ]); + }); + + it('it provides reasonable defaults when datasystem is provided, but some options are missing', () => { + const config = new Configuration( + withLogger({ + dataSystem: {}, + }), + ); + expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(true); + expect(logger(config).getCount()).toEqual(0); + }); + + it('it provides reasonable defaults within the dataSystem.dataSource options when they are missing', () => { + const config = new Configuration( + withLogger({ + dataSystem: { dataSource: { type: 'standard' } }, + }), + ); + expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(true); + expect(logger(config).getCount()).toEqual(0); + }); + + it('ignores deprecated top level options when dataSystem.dataSource options are provided', () => { + const config = new Configuration( + withLogger({ + pollInterval: 501, // should be ignored + streamInitialReconnectDelay: 502, // should be ignored + dataSystem: { + dataSource: { type: 'standard', pollInterval: 100, streamInitialReconnectDelay: 200 }, // should be used + }, + }), + ); + expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(true); + expect((config.dataSystem.dataSource as StandardDataSourceOptions).pollInterval).toEqual(100); + expect( + (config.dataSystem.dataSource as StandardDataSourceOptions).streamInitialReconnectDelay, + ).toEqual(200); + expect(logger(config).getCount()).toEqual(0); + }); + + it('ignores top level featureStore in favor of the datasystem persitent store', () => { + const shouldNotBeUsed = new InMemoryFeatureStore(); + const shouldBeUsed = new InMemoryFeatureStore(); + const config = new Configuration( + withLogger({ + featureStore: shouldNotBeUsed, + dataSystem: { + persistentStore: shouldBeUsed, + }, + }), + ); + // @ts-ignore + const result = config.dataSystem.featureStoreFactory(null); + expect(result).toEqual(shouldBeUsed); + }); }); diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index f46fde5ab2..1e480014fd 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -171,7 +171,7 @@ export default class LDClientImpl implements LDClient { const baseHeaders = defaultHeaders(_sdkKey, _platform.info, config.tags); const clientContext = new ClientContext(_sdkKey, config, _platform); - const featureStore = config.featureStoreFactory(clientContext); + const featureStore = config.dataSystem.featureStoreFactory(clientContext); const dataSourceUpdates = new DataSourceUpdates(featureStore, hasEventListeners, onUpdate); @@ -253,9 +253,9 @@ export default class LDClientImpl implements LDClient { ); }; - if (!(config.offline || config.useLdd)) { + if (!(config.offline || config.dataSystem.useLdd)) { this._updateProcessor = - config.updateProcessorFactory?.( + config.dataSystem.updateProcessorFactory?.( clientContext, dataSourceUpdates, () => this._initSuccess(), diff --git a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts index 33ad8b27cb..92e03e1b87 100644 --- a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts @@ -1,6 +1,7 @@ import { LDClientContext, subsystem } from '@launchdarkly/js-sdk-common'; import { LDDataSourceUpdates, LDFeatureStore } from '../subsystems'; +import { PersistentDataStore } from '../interfaces'; /** * Configuration options for the Data System that the SDK uses to get and maintain flags and other @@ -42,6 +43,29 @@ export interface LDDataSystemOptions { */ dataSource?: DataSourceOptions; + /** + * Before data has arrived from LaunchDarkly, the SDK is able to evaluate flags using + * data from the persistent store. Once fresh data has arrived from LaunchDarkly, the + * SDK will no longer read from the persistent store, although it will keep it up-to-date + * for future startups. + * + * Some implementations provide the store implementation object itself, while others + * provide a factory function that creates the store implementation based on the SDK + * configuration; this property accepts either. + * + * @param clientContext whose properties may be used to influence creation of the persistent store. + */ + persistentStore?: LDFeatureStore | ((clientContext: LDClientContext) => LDFeatureStore); + + /** + * Whether you are using the LaunchDarkly relay proxy in daemon mode. + * + * In this configuration, the client will not connect to LaunchDarkly to get feature flags, + * but will instead get feature state from a database (Redis or another supported feature + * store integration) that is populated by the relay. By default, this is false. + */ + useLdd?: boolean; + /** * A component that obtains feature flag data and puts it in the feature store. Setting * this supercedes {@link LDDataSystemOptions#dataSource}. @@ -55,12 +79,6 @@ export interface LDDataSystemOptions { errorHandler?: (e: Error) => void, ) => subsystem.LDStreamProcessor); - /** - * Before data has arrived from LaunchDarkly, the SDK is able to evaluate flags using - * data from the persistent store. Once fresh data is available, the SDK will no longer - * read from the persistent store, although it will keep it up-to-date for future startups. - */ - persistentStore?: LDFeatureStore | ((clientContext: LDClientContext) => LDFeatureStore); } export type DataSourceOptions = @@ -125,13 +143,13 @@ export interface PollingDataSourceOptions { } export function isStandardOptions(u: any): u is StandardDataSourceOptions { - return u.kind === 'standard'; + return u.type === 'standard'; } export function isStreamingOptions(u: any): u is StreamingDataSourceOptions { - return u.kind === 'streaming'; + return u.type === 'streaming'; } export function isPollingOptions(u: any): u is PollingDataSourceOptions { - return u.kind === 'polling'; + return u.type === 'polling'; } diff --git a/packages/shared/sdk-server/src/api/options/LDOptions.ts b/packages/shared/sdk-server/src/api/options/LDOptions.ts index b109f6271a..e6e212487d 100644 --- a/packages/shared/sdk-server/src/api/options/LDOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDOptions.ts @@ -61,6 +61,8 @@ export interface LDOptions { /** * A component that stores feature flags and related data received from LaunchDarkly. * + * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. + * * By default, this is an in-memory data structure. Database integrations are also * available, as described in the * [SDK features guide](https://docs.launchdarkly.com/sdk/concepts/data-stores). @@ -69,8 +71,6 @@ export interface LDOptions { * provide a factory function that creates the store implementation based on the SDK * configuration; this property accepts either. * - * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. - * * @deprecated This is now superceded by {@link LDOptions#dataSystem} using property * {@link LDDataSystemOptions#persistentStore}. The scope of the {@link LDFeatureStore} * that you provide is being reduced in the next major version to only being responsible @@ -169,11 +169,11 @@ export interface LDOptions { /** * Whether streaming mode should be used to receive flag updates. * + * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. + * * This is true by default. If you set it to false, the client will use polling. * Streaming should only be disabled on the advice of LaunchDarkly support. * - * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. - * * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} * by specifying {@link LDDataSystemOptions#dataSource} options. To opt out of * streaming, you can use the {@link PollingDataSourceOptions}. Streaming should @@ -184,13 +184,14 @@ export interface LDOptions { /** * Sets the initial reconnect delay for the streaming connection, in seconds. * + * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. + * * The streaming service uses a backoff algorithm (with jitter) every time the connection needs * to be reestablished. The delay for the first reconnection will start near this value, and then * increase exponentially for any subsequent connection failures. * * The default value is 1. * - * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. * * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} * by specifying {@link LDDataSystemOptions#dataSource} options. Specifying the reconnect @@ -202,9 +203,14 @@ export interface LDOptions { /** * Whether you are using the LaunchDarkly relay proxy in daemon mode. * + * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. + * * In this configuration, the client will not connect to LaunchDarkly to get feature flags, * but will instead get feature state from a database (Redis or another supported feature * store integration) that is populated by the relay. By default, this is false. + * + * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} + * by specifying {@link LDDataSystemOptions#useLdd}. */ useLdd?: boolean; diff --git a/packages/shared/sdk-server/src/options/Configuration.ts b/packages/shared/sdk-server/src/options/Configuration.ts index ce04388d59..0fe1192fd3 100644 --- a/packages/shared/sdk-server/src/options/Configuration.ts +++ b/packages/shared/sdk-server/src/options/Configuration.ts @@ -15,6 +15,7 @@ import { import { LDBigSegmentsOptions, LDOptions, LDProxyOptions, LDTLSOptions } from '../api'; import { Hook } from '../api/integrations'; import { + DataSourceOptions, isPollingOptions, isStandardOptions, isStreamingOptions, @@ -24,6 +25,7 @@ import { StreamingDataSourceOptions, } from '../api/options/LDDataSystemOptions'; import { LDDataSourceUpdates, LDFeatureStore } from '../api/subsystems'; +import { PersistentDataStoreWrapper } from '../store'; import InMemoryFeatureStore from '../store/InMemoryFeatureStore'; import { ValidatedOptions } from './ValidatedOptions'; @@ -200,7 +202,7 @@ function validateDataSystemOptions(options: Options): { const allErrors: string[] = []; const validatedOptions: Options = { ...options }; - if (!TypeValidators.ObjectOrFactory.is(options.persistentStore)) { + if (options.persistentStore && !TypeValidators.ObjectOrFactory.is(options.persistentStore)) { validatedOptions.persistentStore = undefined; // default is to not use this allErrors.push( OptionMessages.wrongOptionType( @@ -211,44 +213,73 @@ function validateDataSystemOptions(options: Options): { ); } - if (!TypeValidators.ObjectOrFactory.is(options.updateProcessor)) { - validatedOptions.persistentStore = undefined; // default is to not use this + if (options.updateProcessor && !TypeValidators.ObjectOrFactory.is(options.updateProcessor)) { + validatedOptions.updateProcessor = undefined; // default is to not use this allErrors.push( OptionMessages.wrongOptionType( 'updateProcessor', 'UpdateProcessor', - typeof options.persistentStore, + typeof options.updateProcessor, ), ); } - let result: { errors: string[]; validatedOptions: Options }; - let validatedDataSourceOptions; - if (isStandardOptions(options.dataSource)) { - result = validateTypesAndNames(options.dataSource, defaultStandardDataSourceOptions); - } else if (isStreamingOptions(options.dataSource)) { - result = validateTypesAndNames(options.dataSource, defaultStreamingDataSourceOptions); - } else if (isPollingOptions(options.dataSource)) { - result = validateTypesAndNames(options.dataSource, defaultPollingDataSourceOptions); - } else { - // provided datasource options don't fit any expected form, drop them and use defaults - result = { - errors: [ + if (options.dataSource) { + let errors: string[]; + let validatedDataSourceOptions: Options; + if (isStandardOptions(options.dataSource)) { + ({ errors, validatedOptions: validatedDataSourceOptions } = validateTypesAndNames( + options.dataSource, + defaultStandardDataSourceOptions, + )); + } else if (isStreamingOptions(options.dataSource)) { + ({ errors, validatedOptions: validatedDataSourceOptions } = validateTypesAndNames( + options.dataSource, + defaultStreamingDataSourceOptions, + )); + } else if (isPollingOptions(options.dataSource)) { + ({ errors, validatedOptions: validatedDataSourceOptions } = validateTypesAndNames( + options.dataSource, + defaultPollingDataSourceOptions, + )); + } else { + // provided datasource options don't fit any expected form, drop them and use defaults + validatedDataSourceOptions = defaultStandardDataSourceOptions; + errors = [ OptionMessages.wrongOptionType( 'dataSource', 'DataSourceOptions', typeof options.dataSource, ), - ], - validatedOptions: defaultStandardDataSourceOptions, - }; + ]; + } + validatedOptions.dataSource = validatedDataSourceOptions; + allErrors.push(...errors); + } else { + // use default datasource options if no datasource was specified + validatedOptions.dataSource = defaultStandardDataSourceOptions; } - validatedOptions.dataSource = validatedDataSourceOptions; - allErrors.concat(result.errors); return { errors: allErrors, validatedOptions }; } +/** + * Configuration for the Data System + * + * @internal + */ +export interface DataSystemConfiguration { + dataSource?: DataSourceOptions; + featureStoreFactory: (clientContext: LDClientContext) => LDFeatureStore; + useLdd?: boolean; + updateProcessorFactory?: ( + clientContext: LDClientContext, + dataSourceUpdates: LDDataSourceUpdates, + initSuccessHandler: VoidFunction, + errorHandler?: (e: Error) => void, + ) => subsystem.LDStreamProcessor; +} + /** * Configuration options for the LDClient. * @@ -295,16 +326,7 @@ export default class Configuration { public readonly diagnosticRecordingInterval: number; - public readonly featureStoreFactory: (clientContext: LDClientContext) => LDFeatureStore; - - public readonly dataSystem: LDDataSystemOptions; - - public readonly updateProcessorFactory?: ( - clientContext: LDClientContext, - dataSourceUpdates: LDDataSourceUpdates, - initSuccessHandler: VoidFunction, - errorHandler?: (e: Error) => void, - ) => subsystem.LDStreamProcessor; + public readonly dataSystem: DataSystemConfiguration; public readonly bigSegments?: LDBigSegmentsOptions; @@ -334,23 +356,64 @@ export default class Configuration { const { errors: dsErrors, validatedOptions: dsResult } = validateDataSystemOptions( options.dataSystem, ); - this.dataSystem = dsResult as LDDataSystemOptions; + const validatedDSOptions = dsResult as LDDataSystemOptions; + this.dataSystem = { + dataSource: validatedDSOptions.dataSource, + useLdd: validatedDSOptions.useLdd, + // TODO: Discuss typing error with Rlamb. This was existing before it seems. + // @ts-ignore + featureStoreFactory: (clientContext) => { + if (validatedDSOptions.persistentStore === undefined) { + // the persistent store provided was either undefined or invalid, default to memory store + return new InMemoryFeatureStore(); + } + if (TypeValidators.Function.is(validatedDSOptions.persistentStore)) { + return validatedDSOptions.persistentStore(clientContext); + } + return validatedDSOptions.persistentStore; + }, + // TODO: Discuss typing error with Rlamb. This was existing before it seems. + // @ts-ignore + updateProcessorFactory: TypeValidators.Function.is(validatedOptions.updateProcessor) + ? validatedOptions.updateProcessor + : () => validatedOptions.updateProcessor, + }; dsErrors.forEach((error) => { this.logger?.warn(error); }); } else { - // if no data system was specified (such as implmentations using this SDK before data system - // was added), we'll make one based on the stream option + // if data system is not specified, we will use the top level options + // that have been deprecated to make the data system configuration. this.dataSystem = { - dataSource: options.stream - ? { - type: 'streaming', - streamInitialReconnectDelay: validatedOptions.streamInitialReconnectDelay, - } - : { - type: 'polling', - pollInterval: validatedOptions.pollInterval, - }, + // pick data source based on the stream option + dataSource: + (options.stream ?? true) + ? { + // default to standard which has streaming support + type: 'standard', + streamInitialReconnectDelay: validatedOptions.streamInitialReconnectDelay, + pollInterval: validatedOptions.pollInterval, + } + : { + type: 'polling', + pollInterval: validatedOptions.pollInterval, + }, + useLdd: validatedOptions.useLdd, + /** + * TODO: Discuss typing error with Rlamb. This was existing before it seems. +Type '((LDFeatureStore | ((options: LDOptions) => LDFeatureStore)) & ((...args: any[]) => void)) | (() => LDFeatureStore | ((options: LDOptions) => LDFeatureStore))' is not assignable to type '((clientContext: LDClientContext) => LDFeatureStore) | undefined'. + Type 'LDFeatureStore & ((...args: any[]) => void)' is not assignable to type '((clientContext: LDClientContext) => LDFeatureStore) | undefined'. + Type 'LDFeatureStore & ((...args: any[]) => void)' is not assignable to type '(clientContext: LDClientContext) => LDFeatureStore'. + Type 'void' is not assignable to type 'LDFeatureStore'. + */ + // @ts-ignore + featureStoreFactory: TypeValidators.Function.is(validatedOptions.featureStore) + ? validatedOptions.featureStore + : () => validatedOptions.featureStore, + // @ts-ignore + updateProcessorFactory: TypeValidators.Function.is(validatedOptions.updateProcessor) + ? validatedOptions.updateProcessor + : () => validatedOptions.updateProcessor, }; } @@ -386,23 +449,5 @@ export default class Configuration { this.offline = validatedOptions.offline; this.useLdd = validatedOptions.useLdd; - - if (TypeValidators.Function.is(validatedOptions.updateProcessor)) { - // @ts-ignore - this.updateProcessorFactory = validatedOptions.updateProcessor; - } else { - // The processor is already created, just have the method return it. - // @ts-ignore - this.updateProcessorFactory = () => validatedOptions.updateProcessor; - } - - if (TypeValidators.Function.is(validatedOptions.featureStore)) { - // @ts-ignore - this.featureStoreFactory = validatedOptions.featureStore; - } else { - // The store is already created, just have the method return it. - // @ts-ignore - this.featureStoreFactory = () => validatedOptions.featureStore; - } } } From bb47ddef40c22747fea8c6e1e070c92f4733e248 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 18 Mar 2025 09:33:08 -0500 Subject: [PATCH 11/14] fixing unit test name --- .../shared/sdk-server/__tests__/options/Configuration.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts index 43784d26fd..7b34587c31 100644 --- a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts +++ b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts @@ -463,7 +463,7 @@ describe('when setting different options', () => { ]); }); - it('it provides reasonable defaults when datasystem is provided, but some options are missing', () => { + it('provides reasonable defaults when datasystem is provided, but some options are missing', () => { const config = new Configuration( withLogger({ dataSystem: {}, @@ -473,7 +473,7 @@ describe('when setting different options', () => { expect(logger(config).getCount()).toEqual(0); }); - it('it provides reasonable defaults within the dataSystem.dataSource options when they are missing', () => { + it('provides reasonable defaults within the dataSystem.dataSource options when they are missing', () => { const config = new Configuration( withLogger({ dataSystem: { dataSource: { type: 'standard' } }, From 5e6600ef5ed86e438016f8d1df95285415bcb5ff Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Mon, 24 Mar 2025 15:58:26 -0500 Subject: [PATCH 12/14] Additional changes after contract tests integration --- .../common/src/datasource/CompositeDataSource.ts | 1 - packages/shared/sdk-server/src/LDClientImpl.ts | 8 ++++---- .../src/api/options/LDDataSystemOptions.ts | 14 +++++++------- .../sdk-server/src/api/options/LDOptions.ts | 2 +- .../src/data_sources/PollingProcessor.ts | 2 -- .../diagnostics/createDiagnosticsInitConfig.ts | 13 +++++++++---- .../sdk-server/src/options/Configuration.ts | 15 +++++++-------- 7 files changed, 28 insertions(+), 27 deletions(-) diff --git a/packages/shared/common/src/datasource/CompositeDataSource.ts b/packages/shared/common/src/datasource/CompositeDataSource.ts index a694b4e339..e68dc6f7dd 100644 --- a/packages/shared/common/src/datasource/CompositeDataSource.ts +++ b/packages/shared/common/src/datasource/CompositeDataSource.ts @@ -29,7 +29,6 @@ interface TransitionRequest { err?: Error; } -// TODO SDK-858: move this out of API directory to neighbor datasource folder /** * The {@link CompositeDataSource} can combine a number of {@link DataSystemInitializer}s and {@link DataSystemSynchronizer}s * into a single {@link DataSource}, implementing fallback and recovery logic internally to choose where data is sourced from. diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 1e480014fd..f149306e48 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -31,9 +31,9 @@ import { Hook } from './api/integrations/Hook'; import { BigSegmentStoreMembership } from './api/interfaces'; import { LDWaitForInitializationOptions } from './api/LDWaitForInitializationOptions'; import { - isPollingOptions, + isPollingOnlyOptions, isStandardOptions, - isStreamingOptions, + isStreamingOnlyOptions, } from './api/options/LDDataSystemOptions'; import BigSegmentsManager from './BigSegmentsManager'; import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; @@ -225,7 +225,7 @@ export default class LDClientImpl implements LDClient { put: () => this._initSuccess(), }); const makeDefaultProcessor = () => { - if (isPollingOptions(config.dataSystem.dataSource)) { + if (isPollingOnlyOptions(config.dataSystem.dataSource)) { return new PollingProcessor( new Requestor(config, this._platform.requests, baseHeaders), config.dataSystem.dataSource.pollInterval ?? 30, @@ -238,7 +238,7 @@ export default class LDClientImpl implements LDClient { // TODO: SDK-858 Hook up composite data source and config const reconnectDelay = isStandardOptions(config.dataSystem.dataSource) || - isStreamingOptions(config.dataSystem.dataSource) + isStreamingOnlyOptions(config.dataSystem.dataSource) ? config.dataSystem.dataSource.streamInitialReconnectDelay : 1; return new StreamingProcessor( diff --git a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts index 92e03e1b87..cbceee60bc 100644 --- a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts @@ -21,7 +21,7 @@ import { PersistentDataStore } from '../interfaces'; * * let dataSystemOptions = { * dataSource: { - * type: 'polling'; + * type: 'pollingOnly'; * pollInterval: 300; * }, * persistentStore: DynamoDBFeatureStore('your-table', { cacheTTL: 30 }); @@ -116,7 +116,7 @@ export interface StandardDataSourceOptions { * to provide real time data updates. */ export interface StreamingDataSourceOptions { - type: 'streaming'; + type: 'streamingOnly'; /** * Sets the initial reconnect delay for the streaming connection, in seconds. Default if omitted. @@ -134,7 +134,7 @@ export interface StreamingDataSourceOptions { * This data source will periodically make a request to LaunchDarkly services to retrieve updated data. */ export interface PollingDataSourceOptions { - type: 'polling'; + type: 'pollingOnly'; /** * The time between polling requests, in seconds. Default if omitted. @@ -146,10 +146,10 @@ export function isStandardOptions(u: any): u is StandardDataSourceOptions { return u.type === 'standard'; } -export function isStreamingOptions(u: any): u is StreamingDataSourceOptions { - return u.type === 'streaming'; +export function isStreamingOnlyOptions(u: any): u is StreamingDataSourceOptions { + return u.type === 'streamingOnly'; } -export function isPollingOptions(u: any): u is PollingDataSourceOptions { - return u.type === 'polling'; +export function isPollingOnlyOptions(u: any): u is PollingDataSourceOptions { + return u.type === 'pollingOnly'; } diff --git a/packages/shared/sdk-server/src/api/options/LDOptions.ts b/packages/shared/sdk-server/src/api/options/LDOptions.ts index e6e212487d..c5d9602678 100644 --- a/packages/shared/sdk-server/src/api/options/LDOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDOptions.ts @@ -100,7 +100,7 @@ export interface LDOptions { * * let dataSystemOptions = { * dataSource: { - * type: 'polling'; + * type: 'pollingOnly'; * pollInterval: 300; * }, * persistentStore: DynamoDBFeatureStore('your-table', { cacheTTL: 30 }); diff --git a/packages/shared/sdk-server/src/data_sources/PollingProcessor.ts b/packages/shared/sdk-server/src/data_sources/PollingProcessor.ts index 0f264ec047..fc5fda2d33 100644 --- a/packages/shared/sdk-server/src/data_sources/PollingProcessor.ts +++ b/packages/shared/sdk-server/src/data_sources/PollingProcessor.ts @@ -9,11 +9,9 @@ import { } from '@launchdarkly/js-sdk-common'; import { LDDataSourceUpdates } from '../api/subsystems'; -import Configuration from '../options/Configuration'; import { deserializePoll } from '../store'; import VersionedDataKinds from '../store/VersionedDataKinds'; import Requestor from './Requestor'; -import { isPollingOptions, isStandardOptions } from '../api/options/LDDataSystemOptions'; export type PollingErrorHandler = (err: LDPollingError) => void; diff --git a/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts b/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts index 5e61f5eb0c..e580a8871f 100644 --- a/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts +++ b/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts @@ -1,6 +1,11 @@ import { Platform, secondsToMillis } from '@launchdarkly/js-sdk-common'; -import { isPollingOptions, isStandardOptions, isStreamingOptions, LDFeatureStore } from '../api'; +import { + isPollingOnlyOptions, + isStandardOptions, + isStreamingOnlyOptions, + LDFeatureStore, +} from '../api'; import Configuration, { defaultValues } from '../options/Configuration'; const createDiagnosticsInitConfig = ( @@ -20,20 +25,20 @@ const createDiagnosticsInitConfig = ( eventsFlushIntervalMillis: secondsToMillis(config.flushInterval), // include polling interval if data source config has it ...((isStandardOptions(config.dataSystem.dataSource) || - isPollingOptions(config.dataSystem.dataSource)) && + isPollingOnlyOptions(config.dataSystem.dataSource)) && config.dataSystem.dataSource.pollInterval ? { pollingIntervalMillis: config.dataSystem.dataSource.pollInterval } : null), // include reconnect delay if data source config has it ...((isStandardOptions(config.dataSystem.dataSource) || - isStreamingOptions(config.dataSystem.dataSource)) && + isStreamingOnlyOptions(config.dataSystem.dataSource)) && config.dataSystem.dataSource.streamInitialReconnectDelay ? { reconnectTimeMillis: config.dataSystem.dataSource.streamInitialReconnectDelay } : null), contextKeysFlushIntervalMillis: secondsToMillis(config.contextKeysFlushInterval), diagnosticRecordingIntervalMillis: secondsToMillis(config.diagnosticRecordingInterval), - streamingDisabled: isPollingOptions(config.dataSystem.dataSource), + streamingDisabled: isPollingOnlyOptions(config.dataSystem.dataSource), usingRelayDaemon: config.useLdd, offline: config.offline, allAttributesPrivate: config.allAttributesPrivate, diff --git a/packages/shared/sdk-server/src/options/Configuration.ts b/packages/shared/sdk-server/src/options/Configuration.ts index 0fe1192fd3..c813564048 100644 --- a/packages/shared/sdk-server/src/options/Configuration.ts +++ b/packages/shared/sdk-server/src/options/Configuration.ts @@ -16,16 +16,15 @@ import { LDBigSegmentsOptions, LDOptions, LDProxyOptions, LDTLSOptions } from '. import { Hook } from '../api/integrations'; import { DataSourceOptions, - isPollingOptions, + isPollingOnlyOptions, isStandardOptions, - isStreamingOptions, + isStreamingOnlyOptions, LDDataSystemOptions, PollingDataSourceOptions, StandardDataSourceOptions, StreamingDataSourceOptions, } from '../api/options/LDDataSystemOptions'; import { LDDataSourceUpdates, LDFeatureStore } from '../api/subsystems'; -import { PersistentDataStoreWrapper } from '../store'; import InMemoryFeatureStore from '../store/InMemoryFeatureStore'; import { ValidatedOptions } from './ValidatedOptions'; @@ -82,12 +81,12 @@ const defaultStandardDataSourceOptions: StandardDataSourceOptions = { }; const defaultStreamingDataSourceOptions: StreamingDataSourceOptions = { - type: 'streaming', + type: 'streamingOnly', streamInitialReconnectDelay: DEFAULT_STREAM_RECONNECT_DELAY, }; const defaultPollingDataSourceOptions: PollingDataSourceOptions = { - type: 'polling', + type: 'pollingOnly', pollInterval: DEFAULT_POLL_INTERVAL, }; @@ -232,12 +231,12 @@ function validateDataSystemOptions(options: Options): { options.dataSource, defaultStandardDataSourceOptions, )); - } else if (isStreamingOptions(options.dataSource)) { + } else if (isStreamingOnlyOptions(options.dataSource)) { ({ errors, validatedOptions: validatedDataSourceOptions } = validateTypesAndNames( options.dataSource, defaultStreamingDataSourceOptions, )); - } else if (isPollingOptions(options.dataSource)) { + } else if (isPollingOnlyOptions(options.dataSource)) { ({ errors, validatedOptions: validatedDataSourceOptions } = validateTypesAndNames( options.dataSource, defaultPollingDataSourceOptions, @@ -395,7 +394,7 @@ export default class Configuration { pollInterval: validatedOptions.pollInterval, } : { - type: 'polling', + type: 'pollingOnly', pollInterval: validatedOptions.pollInterval, }, useLdd: validatedOptions.useLdd, From fd42ba0e42d1af191888cf6a616af83bdc3669b7 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 28 Mar 2025 14:05:58 -0500 Subject: [PATCH 13/14] fixing top level ldd --- .../__tests__/options/Configuration.test.ts | 29 +++++++++++++++++-- .../createDiagnosticsInitConfig.ts | 2 +- .../sdk-server/src/options/Configuration.ts | 3 -- .../src/options/ValidatedOptions.ts | 1 - 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts index 7b34587c31..916bf75ae9 100644 --- a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts +++ b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts @@ -47,7 +47,7 @@ describe.each([undefined, null, 'potat0', 17, [], {}])('constructed without opti expect(config.tags.value).toBeUndefined(); expect(config.timeout).toEqual(5); expect(config.tlsParams).toBeUndefined(); - expect(config.useLdd).toBe(false); + expect(config.dataSystem.useLdd).toBe(false); expect(config.wrapperName).toBeUndefined(); expect(config.wrapperVersion).toBeUndefined(); expect(config.hooks).toBeUndefined(); @@ -233,7 +233,7 @@ describe('when setting different options', () => { ])('allows setting stream and validates useLdd', (value, expected, warnings) => { // @ts-ignore const config = new Configuration(withLogger({ useLdd: value })); - expect(config.useLdd).toEqual(expected); + expect(config.dataSystem.useLdd).toEqual(expected); expect(logger(config).getCount()).toEqual(warnings); }); @@ -516,4 +516,29 @@ describe('when setting different options', () => { const result = config.dataSystem.featureStoreFactory(null); expect(result).toEqual(shouldBeUsed); }); + + it('ignores top level useLdd option if datasystem is specified', () => { + const config = new Configuration( + withLogger({ + dataSystem: { + persistentStore: new InMemoryFeatureStore(), + }, + useLdd: true, + }), + ); + const result = config.dataSystem.useLdd; + expect(result).toEqual(undefined); + + const config2 = new Configuration( + withLogger({ + dataSystem: { + persistentStore: new InMemoryFeatureStore(), + useLdd: true, + }, + useLdd: false, + }), + ); + const result2 = config2.dataSystem.useLdd; + expect(result2).toEqual(true); + }); }); diff --git a/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts b/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts index e580a8871f..a4892e73a9 100644 --- a/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts +++ b/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts @@ -39,7 +39,7 @@ const createDiagnosticsInitConfig = ( diagnosticRecordingIntervalMillis: secondsToMillis(config.diagnosticRecordingInterval), streamingDisabled: isPollingOnlyOptions(config.dataSystem.dataSource), - usingRelayDaemon: config.useLdd, + usingRelayDaemon: config.dataSystem.useLdd, offline: config.offline, allAttributesPrivate: config.allAttributesPrivate, contextKeysCapacity: config.contextKeysCapacity, diff --git a/packages/shared/sdk-server/src/options/Configuration.ts b/packages/shared/sdk-server/src/options/Configuration.ts index c813564048..bfc3d79cd0 100644 --- a/packages/shared/sdk-server/src/options/Configuration.ts +++ b/packages/shared/sdk-server/src/options/Configuration.ts @@ -299,8 +299,6 @@ export default class Configuration { public readonly offline: boolean; - public readonly useLdd: boolean; - public readonly sendEvents: boolean; public readonly allAttributesPrivate: boolean; @@ -447,6 +445,5 @@ Type '((LDFeatureStore | ((options: LDOptions) => LDFeatureStore)) & ((...args: this.hooks = validatedOptions.hooks; this.offline = validatedOptions.offline; - this.useLdd = validatedOptions.useLdd; } } diff --git a/packages/shared/sdk-server/src/options/ValidatedOptions.ts b/packages/shared/sdk-server/src/options/ValidatedOptions.ts index e4ab735a6e..1e707954c8 100644 --- a/packages/shared/sdk-server/src/options/ValidatedOptions.ts +++ b/packages/shared/sdk-server/src/options/ValidatedOptions.ts @@ -23,7 +23,6 @@ export interface ValidatedOptions { flushInterval: number; pollInterval: number; offline: boolean; - useLdd: boolean; allAttributesPrivate: false; privateAttributes: string[]; contextKeysCapacity: number; From a22f21d0adb41d9107fc52c80841d9a1ce26f4a0 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Mon, 7 Apr 2025 11:38:18 -0500 Subject: [PATCH 14/14] Removing deprecations related to data system options --- .../__tests__/options/Configuration.test.ts | 8 ++--- .../src/api/options/LDDataSystemOptions.ts | 10 +++--- .../sdk-server/src/api/options/LDOptions.ts | 31 ++----------------- 3 files changed, 11 insertions(+), 38 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts index 916bf75ae9..dede3cc009 100644 --- a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts +++ b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts @@ -22,7 +22,7 @@ function logger(options: LDOptions): TestLogger { describe.each([undefined, null, 'potat0', 17, [], {}])('constructed without options', (input) => { it('should have default options', () => { // JavaScript is not going to stop you from calling this with whatever - // you want. So we need to tell TS to ingore our bad behavior. + // you want. So we need to tell TS to ignore our bad behavior. // @ts-ignore const config = new Configuration(input); @@ -421,7 +421,7 @@ describe('when setting different options', () => { ]); }); - it('drops invalid dataystem data source options and replaces with defaults', () => { + it('drops invalid datasystem data source options and replaces with defaults', () => { const config = new Configuration( withLogger({ dataSystem: { dataSource: { bogus: 'myBogusOptions' } as unknown as DataSourceOptions }, @@ -436,7 +436,7 @@ describe('when setting different options', () => { ]); }); - it('validates the datasystem persitent store is a factory or object', () => { + it('validates the datasystem persistent store is a factory or object', () => { const config1 = new Configuration( withLogger({ dataSystem: { @@ -501,7 +501,7 @@ describe('when setting different options', () => { expect(logger(config).getCount()).toEqual(0); }); - it('ignores top level featureStore in favor of the datasystem persitent store', () => { + it('ignores top level featureStore in favor of the datasystem persistent store', () => { const shouldNotBeUsed = new InMemoryFeatureStore(); const shouldBeUsed = new InMemoryFeatureStore(); const config = new Configuration( diff --git a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts index cbceee60bc..29335b52bf 100644 --- a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts @@ -37,9 +37,9 @@ export interface LDDataSystemOptions { * type and the optional fields you want to customize. * * If not specified, this defaults to using the {@link StandardDataSourceOptions} which - * pefroms a combination of streaming and polling. + * performs a combination of streaming and polling. * - * See {@link LDDataSystemOptions} decoumentation for exmaples. + * See {@link LDDataSystemOptions} documentation for examples. */ dataSource?: DataSourceOptions; @@ -48,11 +48,11 @@ export interface LDDataSystemOptions { * data from the persistent store. Once fresh data has arrived from LaunchDarkly, the * SDK will no longer read from the persistent store, although it will keep it up-to-date * for future startups. - * + * * Some implementations provide the store implementation object itself, while others * provide a factory function that creates the store implementation based on the SDK * configuration; this property accepts either. - * + * * @param clientContext whose properties may be used to influence creation of the persistent store. */ persistentStore?: LDFeatureStore | ((clientContext: LDClientContext) => LDFeatureStore); @@ -68,7 +68,7 @@ export interface LDDataSystemOptions { /** * A component that obtains feature flag data and puts it in the feature store. Setting - * this supercedes {@link LDDataSystemOptions#dataSource}. + * this supersedes {@link LDDataSystemOptions#dataSource}. */ updateProcessor?: | object diff --git a/packages/shared/sdk-server/src/api/options/LDOptions.ts b/packages/shared/sdk-server/src/api/options/LDOptions.ts index c5d9602678..b6d48f7be9 100644 --- a/packages/shared/sdk-server/src/api/options/LDOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDOptions.ts @@ -70,12 +70,6 @@ export interface LDOptions { * Some implementations provide the store implementation object itself, while others * provide a factory function that creates the store implementation based on the SDK * configuration; this property accepts either. - * - * @deprecated This is now superceded by {@link LDOptions#dataSystem} using property - * {@link LDDataSystemOptions#persistentStore}. The scope of the {@link LDFeatureStore} - * that you provide is being reduced in the next major version to only being responsible - * for persistence. See documention on {@link LDDataSystemOptions#persistentStore} for - * more information. */ featureStore?: LDFeatureStore | ((clientContext: LDClientContext) => LDFeatureStore); @@ -83,7 +77,7 @@ export interface LDOptions { * Configuration options for the Data System that the SDK uses to get and maintain flags and other * data from LaunchDarkly and other sources. * - * Setting this option supercedes + * Setting this option supersedes * * Example (Recommended): * ```typescript @@ -127,9 +121,6 @@ export interface LDOptions { * A component that obtains feature flag data and puts it in the feature store. * * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. - * - * @deprecated This has moved to {@link LDOptions#dataSystem}. Use property - * {@link LDDataSystemOptions#updateProcessor} instead. */ updateProcessor?: | object @@ -149,10 +140,6 @@ export interface LDOptions { * The time between polling requests, in seconds. Ignored in streaming mode. * * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored. - * - * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} by - * specifying the {@link LDDataSystemOptions#dataSource}. Specifying the polling interval is still - * available when using {@link StandardDataSourceOptions} or {@link PollingDataSourceOptions}. */ pollInterval?: number; @@ -173,11 +160,6 @@ export interface LDOptions { * * This is true by default. If you set it to false, the client will use polling. * Streaming should only be disabled on the advice of LaunchDarkly support. - * - * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} - * by specifying {@link LDDataSystemOptions#dataSource} options. To opt out of - * streaming, you can use the {@link PollingDataSourceOptions}. Streaming should - * only be disabled on the advice of LaunchDarkly support. */ stream?: boolean; @@ -191,12 +173,6 @@ export interface LDOptions { * increase exponentially for any subsequent connection failures. * * The default value is 1. - * - * - * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} - * by specifying {@link LDDataSystemOptions#dataSource} options. Specifying the reconnect - * delay is still available when using {@link StandardDataSourceOptions} or - * {@link StreamingDataSourceOptions}. */ streamInitialReconnectDelay?: number; @@ -208,9 +184,6 @@ export interface LDOptions { * In this configuration, the client will not connect to LaunchDarkly to get feature flags, * but will instead get feature state from a database (Redis or another supported feature * store integration) that is populated by the relay. By default, this is false. - * - * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem} - * by specifying {@link LDDataSystemOptions#useLdd}. */ useLdd?: boolean; @@ -220,7 +193,7 @@ export interface LDOptions { sendEvents?: boolean; /** - * Whether all context attributes (except the contexy key) should be marked as private, and + * Whether all context attributes (except the context key) should be marked as private, and * not sent to LaunchDarkly. * * By default, this is false.