From 285fc9fd225a6bf8e007df95ee08135032b2537c Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 7 Feb 2025 15:56:33 -0600 Subject: [PATCH 01/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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 6904c993e3de68e810b85c6b0beb1c2169ed4c2e Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Thu, 27 Mar 2025 14:30:23 -0500 Subject: [PATCH 13/20] feat: hooks up FDv2 configuration and contract tests to client impl --- contract-tests/sdkClientEntity.js | 58 ++++++++- contract-tests/testharness-suppressions.txt | 14 +++ .../internal/fdv2/PayloadReader.test.ts | 76 ++++++++++-- .../common/src/internal/fdv2/payloadReader.ts | 34 +++-- .../shared/sdk-server/src/LDClientImpl.ts | 116 +++++++++++------- .../src/data_sources/OneShotInitializer.ts | 90 ++++++++++++++ .../data_sources/StreamingProcessorFDv2.ts | 38 +++--- 7 files changed, 343 insertions(+), 83 deletions(-) create mode 100644 packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts diff --git a/contract-tests/sdkClientEntity.js b/contract-tests/sdkClientEntity.js index 7301ffacfa..fcb51ce9fe 100644 --- a/contract-tests/sdkClientEntity.js +++ b/contract-tests/sdkClientEntity.js @@ -23,6 +23,7 @@ export function makeSdkConfig(options, tag) { const maybeTime = (seconds) => seconds === undefined || seconds === null ? undefined : seconds / 1000; + if (options.streaming) { cf.streamUri = options.streaming.baseUri; cf.streamInitialReconnectDelay = maybeTime(options.streaming.initialRetryDelayMs); @@ -33,7 +34,7 @@ export function makeSdkConfig(options, tag) { if (options.polling) { cf.stream = false; cf.baseUri = options.polling.baseUri; - cf.pollInterface = options.polling.pollIntervalMs / 1000; + cf.pollInterval = options.polling.pollIntervalMs / 1000; if (options.polling.filter) { cf.payloadFilterKey = options.polling.filter; } @@ -81,6 +82,61 @@ export function makeSdkConfig(options, tag) { cf.wrapperVersion = options.wrapper.version; } } + if (options.dataSystem) { + const dataSourceStreamingOptions = options.dataSystem.synchronizers?.primary?.streaming ?? options.dataSystem.synchronizers?.secondary?.streaming; + const dataSourcePollingOptions = options.dataSystem.synchronizers?.primary?.polling ?? options.dataSystem.synchronizers?.secondary?.polling; + + if (dataSourceStreamingOptions) { + cf.streamUri = dataSourceStreamingOptions.baseUri; + cf.streamInitialReconnectDelay = maybeTime(dataSourceStreamingOptions.initialRetryDelayMs); + if (dataSourceStreamingOptions.filter) { + cf.payloadFilterKey = dataSourceStreamingOptions.filter; + } + } + if (dataSourcePollingOptions) { + cf.stream = false; + cf.baseUri = dataSourcePollingOptions.baseUri; + cf.pollInterval = dataSourcePollingOptions.pollIntervalMs / 1000; + if (dataSourcePollingOptions.filter) { + cf.payloadFilterKey = dataSourcePollingOptions.filter; + } + } + + let dataSourceOptions; + if (dataSourceStreamingOptions && dataSourcePollingOptions) { + dataSourceOptions = { + type: 'standard', + ...(dataSourceStreamingOptions.initialRetryDelayMs != null && + { streamInitialReconnectDelay: maybeTime(dataSourceStreamingOptions.initialRetryDelayMs) }), + ...(dataSourcePollingOptions.pollIntervalMs != null && + { pollInterval: dataSourcePollingOptions.pollIntervalMs }), + } + } else if (dataSourceStreamingOptions) { + dataSourceOptions = { + type: 'streamingOnly', + ...(dataSourceStreamingOptions.initialRetryDelayMs != null && + { streamInitialReconnectDelay: maybeTime(dataSourceStreamingOptions.initialRetryDelayMs) }), + } + } else if (dataSourcePollingOptions) { + dataSourceOptions = { + type: 'pollingOnly', + ...(dataSourcePollingOptions.pollIntervalMs != null && + { pollInterval: dataSourcePollingOptions.pollIntervalMs }), + } + } else { + // No data source options were specified + dataSourceOptions = undefined; + } + + if (options.dataSystem.payloadFilter) { + cf.payloadFilterKey = options.dataSystem.payloadFilter; + } + + cf.dataSystem = { + dataSource: dataSourceOptions, + } + } + return cf; } diff --git a/contract-tests/testharness-suppressions.txt b/contract-tests/testharness-suppressions.txt index 8a29da94d7..e6fdd9ffc9 100644 --- a/contract-tests/testharness-suppressions.txt +++ b/contract-tests/testharness-suppressions.txt @@ -1,2 +1,16 @@ +polling/requests +polling/payload/large payloads + streaming/validation/drop and reconnect if stream event has malformed JSON streaming/validation/drop and reconnect if stream event has well-formed JSON not matching schema + +wrapper/poll requests/wrapper name and version + +streaming/fdv2/reconnection state management/initializes from polling initializer +streaming/fdv2/reconnection state management/initializes from 2 polling initializers + +streaming/fdv2/reconnection state management/saves previously known state +streaming/fdv2/reconnection state management/replaces previously known state +streaming/fdv2/reconnection state management/updates previously known state +streaming/fdv2/ignores model version +streaming/fdv2/can discard partial events on errors \ No newline at end of file diff --git a/packages/shared/common/__tests__/internal/fdv2/PayloadReader.test.ts b/packages/shared/common/__tests__/internal/fdv2/PayloadReader.test.ts index 2212062121..33f22bd096 100644 --- a/packages/shared/common/__tests__/internal/fdv2/PayloadReader.test.ts +++ b/packages/shared/common/__tests__/internal/fdv2/PayloadReader.test.ts @@ -57,6 +57,48 @@ it('it sets basis to false when intent code is xfer-changes', () => { expect(receivedPayloads[0].basis).toEqual(false); }); +it('it handles xfer-full then xfer-changes', () => { + const mockStream = new MockEventStream(); + const receivedPayloads: Payload[] = []; + const readerUnderTest = new PayloadReader(mockStream, { + mockKind: (it) => it, // obj processor that just returns the same obj + }); + readerUnderTest.addPayloadListener((it) => { + receivedPayloads.push(it); + }); + + mockStream.simulateEvent('server-intent', { + data: '{"payloads": [{"code": "xfer-full", "id": "mockId"}]}', + }); + mockStream.simulateEvent('put-object', { + data: '{"kind": "mockKind", "key": "flagA", "version": 123, "object": {"objectFieldA": "objectValueA"}}', + }); + mockStream.simulateEvent('payload-transferred', { + data: '{"state": "mockState", "version": 1}', + }); + + mockStream.simulateEvent('put-object', { + data: '{"kind": "mockKind", "key": "flagA", "version": 456, "object": {"objectFieldA": "newValue"}}', + }); + mockStream.simulateEvent('payload-transferred', { + data: '{"state": "mockState", "version": 1}', + }); + expect(receivedPayloads.length).toEqual(2); + expect(receivedPayloads[0].id).toEqual('mockId'); + expect(receivedPayloads[0].state).toEqual('mockState'); + expect(receivedPayloads[0].basis).toEqual(true); + expect(receivedPayloads[0].updates.length).toEqual(1); + expect(receivedPayloads[0].updates[0].object).toEqual({ objectFieldA: 'objectValueA' }); + expect(receivedPayloads[0].updates[0].deleted).toEqual(undefined); + + expect(receivedPayloads[1].id).toEqual('mockId'); + expect(receivedPayloads[1].state).toEqual('mockState'); + expect(receivedPayloads[1].basis).toEqual(false); + expect(receivedPayloads[1].updates.length).toEqual(1); + expect(receivedPayloads[1].updates[0].object).toEqual({ objectFieldA: 'newValue' }); + expect(receivedPayloads[1].updates[0].deleted).toEqual(undefined); +}); + it('it includes multiple types of updates in payload', () => { const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; @@ -183,12 +225,15 @@ it('logs prescribed message when error event is encountered', () => { mockStream.simulateEvent('error', { data: '{"reason": "Womp womp"}', }); + mockStream.simulateEvent('put-object', { + data: '{"kind": "mockKind", "key": "flagB", "version": 123, "object": {"objectFieldB": "objectValueB"}}', + }); mockStream.simulateEvent('payload-transferred', { data: '{"state": "mockState", "version": 1}', }); - expect(receivedPayloads.length).toEqual(0); + expect(receivedPayloads.length).toEqual(1); expect(mockLogger.info).toHaveBeenCalledWith( - 'An issue was encountered receiving updates for payload mockId with reason: Womp womp. Automatic retry will occur.', + 'An issue was encountered receiving updates for payload mockId with reason: Womp womp.', ); }); @@ -222,6 +267,9 @@ it('discards partially transferred data when an error is encountered', () => { mockStream.simulateEvent('error', { data: '{"reason": "Womp womp"}', }); + mockStream.simulateEvent('put-object', { + data: '{"kind": "mockKind", "key": "flagB", "version": 123, "object": {"objectFieldB": "objectValueB"}}', + }); mockStream.simulateEvent('payload-transferred', { data: '{"state": "mockState", "version": 1}', }); @@ -240,17 +288,23 @@ it('discards partially transferred data when an error is encountered', () => { mockStream.simulateEvent('payload-transferred', { data: '{"state": "mockState2", "version": 1}', }); - expect(receivedPayloads.length).toEqual(1); - expect(receivedPayloads[0].id).toEqual('mockId2'); - expect(receivedPayloads[0].state).toEqual('mockState2'); + expect(receivedPayloads.length).toEqual(2); + expect(receivedPayloads[0].id).toEqual('mockId'); + expect(receivedPayloads[0].state).toEqual('mockState'); expect(receivedPayloads[0].basis).toEqual(true); - expect(receivedPayloads[0].updates.length).toEqual(3); - expect(receivedPayloads[0].updates[0].object).toEqual({ objectFieldX: 'objectValueX' }); + expect(receivedPayloads[0].updates.length).toEqual(1); + expect(receivedPayloads[0].updates[0].object).toEqual({ objectFieldB: 'objectValueB' }); expect(receivedPayloads[0].updates[0].deleted).toEqual(undefined); - expect(receivedPayloads[0].updates[1].object).toEqual(undefined); - expect(receivedPayloads[0].updates[1].deleted).toEqual(true); - expect(receivedPayloads[0].updates[2].object).toEqual({ objectFieldZ: 'objectValueZ' }); - expect(receivedPayloads[0].updates[2].deleted).toEqual(undefined); + expect(receivedPayloads[1].id).toEqual('mockId2'); + expect(receivedPayloads[1].state).toEqual('mockState2'); + expect(receivedPayloads[1].basis).toEqual(true); + expect(receivedPayloads[1].updates.length).toEqual(3); + expect(receivedPayloads[1].updates[0].object).toEqual({ objectFieldX: 'objectValueX' }); + expect(receivedPayloads[1].updates[0].deleted).toEqual(undefined); + expect(receivedPayloads[1].updates[1].object).toEqual(undefined); + expect(receivedPayloads[1].updates[1].deleted).toEqual(true); + expect(receivedPayloads[1].updates[2].object).toEqual({ objectFieldZ: 'objectValueZ' }); + expect(receivedPayloads[1].updates[2].deleted).toEqual(undefined); }); it('silently ignores unrecognized kinds', () => { diff --git a/packages/shared/common/src/internal/fdv2/payloadReader.ts b/packages/shared/common/src/internal/fdv2/payloadReader.ts index 68b1320fc3..e1f4fde0bf 100644 --- a/packages/shared/common/src/internal/fdv2/payloadReader.ts +++ b/packages/shared/common/src/internal/fdv2/payloadReader.ts @@ -44,7 +44,7 @@ export class PayloadReader { private _listeners: PayloadListener[] = []; private _tempId?: string = undefined; - private _tempBasis?: boolean = undefined; + private _tempBasis: boolean = false; private _tempUpdates: Update[] = []; /** @@ -105,7 +105,7 @@ export class PayloadReader { private _processServerIntent = (data: ServerIntentData) => { // clear state in prep for handling data - this._resetState(); + this._resetAll(); // if there's no payloads, return if (!data.payloads.length) { @@ -133,7 +133,7 @@ export class PayloadReader { private _processPutObject = (data: PutObject) => { // if the following properties haven't been provided by now, we should ignore the event if ( - !this._tempId || // server intent hasn't been recieved yet. + !this._tempId || // server intent hasn't been received yet. !data.kind || !data.key || !data.version || @@ -144,7 +144,7 @@ export class PayloadReader { const obj = this._processObj(data.kind, data.object); if (!obj) { - this._logger?.warn(`Unable to prcoess object for kind: '${data.kind}'`); + this._logger?.warn(`Unable to process object for kind: '${data.kind}'`); // ignore unrecognized kinds return; } @@ -178,10 +178,9 @@ export class PayloadReader { if ( !this._tempId || // server intent hasn't been recieved yet. !data.state || - !data.version || - this._tempBasis === undefined + !data.version ) { - this._resetState(); // a reset is best defensive action since payload transferred terminates a payload + this._resetAll(); // a reset is best defensive action since payload transferred terminates a payload return; } @@ -194,26 +193,35 @@ export class PayloadReader { }; this._listeners.forEach((it) => it(payload)); - this._resetState(); + this._resetAfterEmission(); }; private _processGoodbye = (data: any) => { this._logger?.info( `Goodbye was received from the LaunchDarkly connection with reason: ${data.reason}.`, ); - this._resetState(); + this._resetAll(); }; private _processError = (data: any) => { this._logger?.info( - `An issue was encountered receiving updates for payload ${this._tempId} with reason: ${data.reason}. Automatic retry will occur.`, + `An issue was encountered receiving updates for payload ${this._tempId} with reason: ${data.reason}.`, ); - this._resetState(); + this._resetAfterError(); }; - private _resetState() { + private _resetAfterEmission() { + this._tempBasis = false; + this._tempUpdates = []; + } + + private _resetAfterError() { + this._tempUpdates = []; + } + + private _resetAll() { this._tempId = undefined; - this._tempBasis = undefined; + this._tempBasis = false; this._tempUpdates = []; } } diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index f149306e48..74f4df9077 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -2,6 +2,7 @@ import { cancelableTimedPromise, ClientContext, + CompositeDataSource, Context, defaultHeaders, internal, @@ -37,11 +38,13 @@ import { } from './api/options/LDDataSystemOptions'; import BigSegmentsManager from './BigSegmentsManager'; import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; +import { createPayloadListener } from './data_sources/createPayloadListenerFDv2'; import { createStreamListeners } from './data_sources/createStreamListeners'; import DataSourceUpdates from './data_sources/DataSourceUpdates'; +import OneShotInitializer from './data_sources/OneShotInitializer'; import PollingProcessor from './data_sources/PollingProcessor'; import Requestor from './data_sources/Requestor'; -import StreamingProcessor from './data_sources/StreamingProcessor'; +import StreamingProcessorFDv2 from './data_sources/StreamingProcessorFDv2'; import createDiagnosticsInitConfig from './diagnostics/createDiagnosticsInitConfig'; import { allAsync } from './evaluation/collection'; import { Flag } from './evaluation/data/Flag'; @@ -102,6 +105,8 @@ export default class LDClientImpl implements LDClient { private _updateProcessor?: subsystem.LDStreamProcessor; + private _dataSource?: subsystem.DataSource; + private _eventFactoryDefault = new EventFactory(false); private _eventFactoryWithReasons = new EventFactory(true); @@ -221,50 +226,73 @@ export default class LDClientImpl implements LDClient { }; this._evaluator = new Evaluator(this._platform, queries); - const listeners = createStreamListeners(dataSourceUpdates, this._logger, { - put: () => this._initSuccess(), - }); - const makeDefaultProcessor = () => { - if (isPollingOnlyOptions(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) || - isStreamingOnlyOptions(config.dataSystem.dataSource) - ? config.dataSystem.dataSource.streamInitialReconnectDelay - : 1; - return new StreamingProcessor( + if (!(config.offline || config.dataSystem.useLdd)) { + // use configured update processor factory if one exists + const updateProcessor = config.dataSystem.updateProcessorFactory?.( clientContext, - '/all', - [], - listeners, - baseHeaders, - this._diagnosticsManager, + dataSourceUpdates, + () => this._initSuccess(), (e) => this._dataSourceErrorHandler(e), - reconnectDelay, ); - }; + if (updateProcessor) { + this._updateProcessor = updateProcessor; + this._updateProcessor?.start(); + } else { + // make the FDv2 composite datasource with initializers/synchronizers + let initializers: subsystem.LDSynchronizerFactory[] = []; + if (isStandardOptions(config.dataSystem.dataSource)) { + initializers = [ + () => + new OneShotInitializer( + new Requestor(config, this._platform.requests, baseHeaders), + config.logger, + ), + ]; + } else { + initializers = []; + } - if (!(config.offline || config.dataSystem.useLdd)) { - this._updateProcessor = - config.dataSystem.updateProcessorFactory?.( - clientContext, - dataSourceUpdates, - () => this._initSuccess(), - (e) => this._dataSourceErrorHandler(e), - ) ?? makeDefaultProcessor(); - } + let synchronizers: subsystem.LDSynchronizerFactory[] = []; + if (isPollingOnlyOptions(config.dataSystem.dataSource)) { + // TODO: SDK-851 - Make polling synchronizer + synchronizers = []; + } else if ( + isStandardOptions(config.dataSystem.dataSource) || + isStreamingOnlyOptions(config.dataSystem.dataSource) + ) { + const reconnectDelay = config.dataSystem.dataSource.streamInitialReconnectDelay; + synchronizers = [ + () => + new StreamingProcessorFDv2( + clientContext, + '/all', + [], + baseHeaders, + this._diagnosticsManager, + reconnectDelay, + ), + ]; + } else { + // TODO: this is an interesting case to be figured out later + synchronizers = []; + } + + this._dataSource = new CompositeDataSource(initializers, synchronizers, this.logger); + const payloadListener = createPayloadListener(dataSourceUpdates, this.logger, () => { + this._initSuccess(); + }); - if (this._updateProcessor) { - this._updateProcessor.start(); + this._dataSource.start( + (_, payload) => { + payloadListener(payload); + }, + (_, err) => { + if (err) { + this._dataSourceErrorHandler(err); + } + }, + ); + } } else { // Deferring the start callback should allow client construction to complete before we start // emitting events. Allowing the client an opportunity to register events. @@ -285,7 +313,10 @@ export default class LDClientImpl implements LDClient { // If there is no update processor, then there is functionally no initialization // so it is fine not to wait. - if (options?.timeout === undefined && this._updateProcessor !== undefined) { + if ( + options?.timeout === undefined && + (this._updateProcessor !== undefined || this._dataSource !== undefined) + ) { this._logger?.warn( 'The waitForInitialization function was called without a timeout specified.' + ' In a future version a default timeout will be applied.', @@ -294,7 +325,7 @@ export default class LDClientImpl implements LDClient { if ( options?.timeout !== undefined && options?.timeout > HIGH_TIMEOUT_THRESHOLD && - this._updateProcessor !== undefined + (this._updateProcessor !== undefined || this._dataSource !== undefined) ) { this._logger?.warn( 'The waitForInitialization function was called with a timeout greater than ' + @@ -740,6 +771,7 @@ export default class LDClientImpl implements LDClient { close(): void { this._eventProcessor.close(); this._updateProcessor?.close(); + this._dataSource?.stop(); this._featureStore.close(); this._bigSegmentsManager.close(); } diff --git a/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts b/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts new file mode 100644 index 0000000000..c1fd78de8c --- /dev/null +++ b/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts @@ -0,0 +1,90 @@ +import { + DataSourceErrorKind, + httpErrorMessage, + LDLogger, + LDPollingError, + subsystem as subsystemCommon, +} from '@launchdarkly/js-sdk-common'; + +import { deserializePoll } from '../store'; +import VersionedDataKinds from '../store/VersionedDataKinds'; +import Requestor from './Requestor'; + +/** + * @internal + */ +export default class OneShotInitializer implements subsystemCommon.DataSystemInitializer { + constructor( + private readonly _requestor: Requestor, + private readonly _logger?: LDLogger, + ) {} + + /** + * May be called any number of times, if already started, has no effect + * @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. + */ + start( + dataCallback: (basis: boolean, data: any) => void, + statusCallback: (status: subsystemCommon.DataSourceState, err?: any) => void, + ) { + statusCallback(subsystemCommon.DataSourceState.Initializing); + + // @ts-ignore + // eslint-disable-next-line no-underscore-dangle + console.log(this._requestor._headers); + + this._logger?.debug('Performing initialization request to LaunchDarkly for feature flag data.'); + this._requestor.requestAllData((err, body) => { + if (err) { + const { status } = err; + const message = httpErrorMessage(err, 'initializer', 'will not retry'); + this._logger?.error(message); + statusCallback( + subsystemCommon.DataSourceState.Closed, + new LDPollingError(DataSourceErrorKind.ErrorResponse, message, status), + ); + return; + } + + if (!body) { + this._logger?.error('Initialization response missing body'); + statusCallback( + subsystemCommon.DataSourceState.Closed, + new LDPollingError(DataSourceErrorKind.InvalidData, 'Polling response missing body'), + ); + } + + const parsed = deserializePoll(body); + if (!parsed) { + // We could not parse this JSON. Report the problem and fallthrough to + // start another poll. + this._logger?.error('Initialization response contained invalid data'); + this._logger?.debug(`Invalid JSON follows: ${body}`); + statusCallback( + subsystemCommon.DataSourceState.Closed, + new LDPollingError( + DataSourceErrorKind.InvalidData, + 'Malformed JSON data in polling response', + ), + ); + } else { + const initData = { + [VersionedDataKinds.Features.namespace]: parsed.flags, + [VersionedDataKinds.Segments.namespace]: parsed.segments, + }; + + // TODO: need to transform this into a payload + + dataCallback(true, initData); + statusCallback(subsystemCommon.DataSourceState.Closed); + } + }); + } + + stop() { + // TODO: at the moment no way to cancel the inflight request via the requester API, but could + // be added in the future. + } +} diff --git a/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts b/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts index 901c3d3ec7..d7715f9a7a 100644 --- a/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts +++ b/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts @@ -12,7 +12,7 @@ import { Requests, shouldRetry, StreamingErrorHandler, - subsystem, + subsystem as subsystemCommon, } from '@launchdarkly/js-sdk-common'; import { PayloadListener } from '@launchdarkly/js-sdk-common/dist/esm/internal'; @@ -21,7 +21,7 @@ import { Segment } from '../evaluation/data/Segment'; import { processFlag, processSegment } from '../store/serialization'; // TODO: consider naming this StreamingDatasource -export default class StreamingProcessorFDv2 implements subsystem.LDStreamProcessor { +export default class StreamingProcessorFDv2 implements subsystemCommon.DataSystemSynchronizer { private readonly _headers: { [key: string]: string | string[] }; private readonly _streamUri: string; private readonly _logger?: LDLogger; @@ -34,10 +34,8 @@ export default class StreamingProcessorFDv2 implements subsystem.LDStreamProcess clientContext: ClientContext, streamUriPath: string, parameters: { key: string; value: string }[], - private readonly _payloadListener: PayloadListener, baseHeaders: LDHeaders, private readonly _diagnosticsManager?: internal.DiagnosticsManager, - private readonly _errorHandler?: StreamingErrorHandler, private readonly _streamInitialReconnectDelay = 1, ) { const { basicConfiguration, platform } = clientContext; @@ -54,7 +52,7 @@ export default class StreamingProcessorFDv2 implements subsystem.LDStreamProcess ); } - private _logConnectionStarted() { + private _logConnectionAttempt() { this._connectionAttemptStartTime = Date.now(); } @@ -79,28 +77,32 @@ export default class StreamingProcessorFDv2 implements subsystem.LDStreamProcess * * @private */ - private _retryAndHandleError(err: HttpErrorResponse) { + private _retryAndHandleError(err: HttpErrorResponse, statusCallback: (status: subsystemCommon.DataSourceState, err?: any) => void) { + if (!shouldRetry(err)) { - this._logConnectionResult(false); - this._errorHandler?.( - new LDStreamingError(DataSourceErrorKind.ErrorResponse, err.message, err.status), - ); this._logger?.error(httpErrorMessage(err, 'streaming request')); + this._logConnectionResult(false); + statusCallback(subsystemCommon.DataSourceState.Closed, new LDStreamingError(DataSourceErrorKind.ErrorResponse, err.message, err.status)) return false; } this._logger?.warn(httpErrorMessage(err, 'streaming request', 'will retry')); this._logConnectionResult(false); - this._logConnectionStarted(); + this._logConnectionAttempt(); + statusCallback(subsystemCommon.DataSourceState.Interrupted) return true; } - start() { - this._logConnectionStarted(); + start( + dataCallback: (basis: boolean, data: any) => void, + statusCallback: (status: subsystemCommon.DataSourceState, err?: any) => void, + ) { + this._logConnectionAttempt(); + statusCallback(subsystemCommon.DataSourceState.Initializing); const eventSource = this._requests.createEventSource(this._streamUri, { headers: this._headers, - errorFilter: (error: HttpErrorResponse) => this._retryAndHandleError(error), + errorFilter: (error: HttpErrorResponse) => this._retryAndHandleError(error, statusCallback), initialRetryDelayMillis: 1000 * this._streamInitialReconnectDelay, readTimeoutMillis: 5 * 60 * 1000, retryResetIntervalMillis: 60 * 1000, @@ -119,7 +121,7 @@ export default class StreamingProcessorFDv2 implements subsystem.LDStreamProcess }, }, (errorKind: DataSourceErrorKind, message: string) => { - this._errorHandler?.(new LDStreamingError(errorKind, message)); + statusCallback(subsystemCommon.DataSourceState.Interrupted, new LDStreamingError(errorKind, message)); }, this._logger, ); @@ -128,10 +130,13 @@ export default class StreamingProcessorFDv2 implements subsystem.LDStreamProcess // to double check the handling in the ServerIntent:none case. That may not trigger these payload listeners. this._logConnectionResult(true); }); - payloadReader.addPayloadListener(this._payloadListener); + payloadReader.addPayloadListener((payload) => { + dataCallback(payload.basis, payload); + }); eventSource.onclose = () => { this._logger?.info('Closed LaunchDarkly stream connection'); + statusCallback(subsystemCommon.DataSourceState.Closed); }; eventSource.onerror = () => { @@ -140,6 +145,7 @@ export default class StreamingProcessorFDv2 implements subsystem.LDStreamProcess eventSource.onopen = () => { this._logger?.info('Opened LaunchDarkly stream connection'); + statusCallback(subsystemCommon.DataSourceState.Valid); }; eventSource.onretrying = (e) => { From a0f5398d00e254dc36512f3711355f101110cc01 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Wed, 2 Apr 2025 16:26:09 -0500 Subject: [PATCH 14/20] feat: adds polling synchronizer support --- contract-tests/testharness-suppressions.txt | 5 - ...er.test.ts => PayloadStreamReader.test.ts} | 24 +-- .../shared/common/src/internal/fdv2/index.ts | 11 +- .../{payloadReader.ts => payloadProcessor.ts} | 79 ++++++---- .../src/internal/fdv2/payloadStreamReader.ts | 69 +++++++++ .../data_sources/PollingProcessor.test.ts | 33 +--- .../StreamingProcessorFDv2.test.ts | 46 +++--- .../shared/sdk-server/src/LDClientImpl.ts | 34 +++-- .../src/data_sources/PollingProcessorFDv2.ts | 144 ++++++++++++++++++ .../data_sources/StreamingProcessorFDv2.ts | 29 ++-- .../createDiagnosticsInitConfig.ts | 8 +- 11 files changed, 362 insertions(+), 120 deletions(-) rename packages/shared/common/__tests__/internal/fdv2/{PayloadReader.test.ts => PayloadStreamReader.test.ts} (94%) rename packages/shared/common/src/internal/fdv2/{payloadReader.ts => payloadProcessor.ts} (76%) create mode 100644 packages/shared/common/src/internal/fdv2/payloadStreamReader.ts create mode 100644 packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts diff --git a/contract-tests/testharness-suppressions.txt b/contract-tests/testharness-suppressions.txt index e6fdd9ffc9..c7c8985d79 100644 --- a/contract-tests/testharness-suppressions.txt +++ b/contract-tests/testharness-suppressions.txt @@ -1,11 +1,6 @@ -polling/requests -polling/payload/large payloads - streaming/validation/drop and reconnect if stream event has malformed JSON streaming/validation/drop and reconnect if stream event has well-formed JSON not matching schema -wrapper/poll requests/wrapper name and version - streaming/fdv2/reconnection state management/initializes from polling initializer streaming/fdv2/reconnection state management/initializes from 2 polling initializers diff --git a/packages/shared/common/__tests__/internal/fdv2/PayloadReader.test.ts b/packages/shared/common/__tests__/internal/fdv2/PayloadStreamReader.test.ts similarity index 94% rename from packages/shared/common/__tests__/internal/fdv2/PayloadReader.test.ts rename to packages/shared/common/__tests__/internal/fdv2/PayloadStreamReader.test.ts index 33f22bd096..5b782d9a3f 100644 --- a/packages/shared/common/__tests__/internal/fdv2/PayloadReader.test.ts +++ b/packages/shared/common/__tests__/internal/fdv2/PayloadStreamReader.test.ts @@ -1,5 +1,7 @@ import { EventListener, EventName, LDLogger } from '../../../src/api'; -import { EventStream, Payload, PayloadReader } from '../../../src/internal/fdv2/payloadReader'; +import { Payload } from '../../../src/internal/fdv2/payloadProcessor'; +import { EventStream, PayloadStreamReader } from '../../../src/internal/fdv2/payloadStreamReader'; + class MockEventStream implements EventStream { private _listeners: Record = {}; @@ -16,7 +18,7 @@ class MockEventStream implements EventStream { it('it sets basis to true when intent code is xfer-full', () => { const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader(mockStream, { + const readerUnderTest = new PayloadStreamReader(mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj }); readerUnderTest.addPayloadListener((it) => { @@ -38,7 +40,7 @@ it('it sets basis to true when intent code is xfer-full', () => { it('it sets basis to false when intent code is xfer-changes', () => { const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader(mockStream, { + const readerUnderTest = new PayloadStreamReader(mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj }); readerUnderTest.addPayloadListener((it) => { @@ -60,7 +62,7 @@ it('it sets basis to false when intent code is xfer-changes', () => { it('it handles xfer-full then xfer-changes', () => { const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader(mockStream, { + const readerUnderTest = new PayloadStreamReader(mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj }); readerUnderTest.addPayloadListener((it) => { @@ -102,7 +104,7 @@ it('it handles xfer-full then xfer-changes', () => { it('it includes multiple types of updates in payload', () => { const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader(mockStream, { + const readerUnderTest = new PayloadStreamReader(mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj }); readerUnderTest.addPayloadListener((it) => { @@ -140,7 +142,7 @@ it('it includes multiple types of updates in payload', () => { it('it does not include messages thats are not between server-intent and payloader-transferred', () => { const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader(mockStream, { + const readerUnderTest = new PayloadStreamReader(mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj }); readerUnderTest.addPayloadListener((it) => { @@ -173,7 +175,7 @@ it('logs prescribed message when goodbye event is encountered', () => { }; const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader( + const readerUnderTest = new PayloadStreamReader( mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj @@ -204,7 +206,7 @@ it('logs prescribed message when error event is encountered', () => { }; const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader( + const readerUnderTest = new PayloadStreamReader( mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj @@ -246,7 +248,7 @@ it('discards partially transferred data when an error is encountered', () => { }; const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader( + const readerUnderTest = new PayloadStreamReader( mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj @@ -310,7 +312,7 @@ it('discards partially transferred data when an error is encountered', () => { it('silently ignores unrecognized kinds', () => { const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader(mockStream, { + const readerUnderTest = new PayloadStreamReader(mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj }); readerUnderTest.addPayloadListener((it) => { @@ -340,7 +342,7 @@ it('silently ignores unrecognized kinds', () => { it('ignores additional payloads beyond the first payload in the server-intent message', () => { const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; - const readerUnderTest = new PayloadReader(mockStream, { + const readerUnderTest = new PayloadStreamReader(mockStream, { mockKind: (it) => it, // obj processor that just returns the same obj }); readerUnderTest.addPayloadListener((it) => { diff --git a/packages/shared/common/src/internal/fdv2/index.ts b/packages/shared/common/src/internal/fdv2/index.ts index 4c4a88773e..fba82ccdb6 100644 --- a/packages/shared/common/src/internal/fdv2/index.ts +++ b/packages/shared/common/src/internal/fdv2/index.ts @@ -1,3 +1,10 @@ -import { Payload, PayloadListener, PayloadReader, Update } from './payloadReader'; +import { + EventsSummary, + Payload, + PayloadListener, + PayloadProcessor, + Update, +} from './payloadProcessor'; +import { PayloadStreamReader } from './payloadStreamReader'; -export { Payload, PayloadListener, PayloadReader, Update }; +export { EventsSummary, Payload, PayloadListener, PayloadProcessor, PayloadStreamReader, Update }; diff --git a/packages/shared/common/src/internal/fdv2/payloadReader.ts b/packages/shared/common/src/internal/fdv2/payloadProcessor.ts similarity index 76% rename from packages/shared/common/src/internal/fdv2/payloadReader.ts rename to packages/shared/common/src/internal/fdv2/payloadProcessor.ts index e1f4fde0bf..25dff1ab9e 100644 --- a/packages/shared/common/src/internal/fdv2/payloadReader.ts +++ b/packages/shared/common/src/internal/fdv2/payloadProcessor.ts @@ -1,19 +1,23 @@ /* eslint-disable no-underscore-dangle */ -import { EventListener, EventName, LDLogger } from '../../api'; +import { LDLogger } from '../../api'; import { DataSourceErrorKind } from '../../datasource'; import { DeleteObject, PayloadTransferred, PutObject, ServerIntentData } from './proto'; -// Facade interface to contain only ability to add event listeners -export interface EventStream { - addEventListener(type: EventName, listener: EventListener): void; -} - // Used to define object processing between deserialization and payload listener invocation. This can be // used provide object sanitization logic. export interface ObjProcessors { [kind: string]: (object: any) => any; } +export interface EventsSummary { + events: Event[]; +} + +export interface Event { + event: string; + data: any; +} + // Represents information for one keyed object. export interface Update { kind: string; @@ -40,7 +44,7 @@ export type PayloadListener = (payload: Payload) => void; * to the PayloadListeners as the payloads are received. Invalid series of events may be dropped silently, * but the payload reader will continue to operate. */ -export class PayloadReader { +export class PayloadProcessor { private _listeners: PayloadListener[] = []; private _tempId?: string = undefined; @@ -50,24 +54,15 @@ export class PayloadReader { /** * Creates a PayloadReader * - * @param eventStream event stream of FDv2 events * @param _objProcessors defines object processors for each object kind. - * @param _errorHandler that will be called with errors as they are encountered + * @param _errorHandler that will be called with parsing errors as they are encountered * @param _logger for logging */ constructor( - eventStream: EventStream, private readonly _objProcessors: ObjProcessors, private readonly _errorHandler?: (errorKind: DataSourceErrorKind, message: string) => void, private readonly _logger?: LDLogger, - ) { - this._attachHandler(eventStream, 'server-intent', this._processServerIntent); - this._attachHandler(eventStream, 'put-object', this._processPutObject); - this._attachHandler(eventStream, 'delete-object', this._processDeleteObject); - this._attachHandler(eventStream, 'payload-transferred', this._processPayloadTransferred); - this._attachHandler(eventStream, 'goodbye', this._processGoodbye); - this._attachHandler(eventStream, 'error', this._processError); - } + ) {} addPayloadListener(listener: PayloadListener) { this._listeners.push(listener); @@ -80,21 +75,41 @@ export class PayloadReader { } } - private _attachHandler(stream: EventStream, eventName: string, processor: (obj: any) => void) { - stream.addEventListener(eventName, async (event?: { data?: string }) => { - if (event?.data) { - this._logger?.debug(`Received ${eventName} event. Data is ${event.data}`); - try { - processor(JSON.parse(event.data)); - } catch { - this._logger?.error( - `Stream received data that was unable to be processed in "${eventName}" message`, - ); - this._logger?.debug(`Data follows: ${event.data}`); - this._errorHandler?.(DataSourceErrorKind.InvalidData, 'Malformed data in event stream'); + /** + * Gives the {@link PayloadProcessor} a series of events that it will statefully, incrementally processed. + * This may lead to listeners being invoked as necessary. + * @param events to be processed (can be a single element) + */ + processEvents(events: Event[]) { + events.forEach((event) => { + switch (event.event) { + case 'server-intent': { + this._processServerIntent(event.data); + break; + } + case 'put-object': { + this._processPutObject(event.data); + break; + } + case 'delete-object': { + this._processDeleteObject(event.data); + break; + } + case 'payload-transferred': { + this._processPayloadTransferred(event.data); + break; + } + case 'goodbye': { + this._processGoodbye(event.data); + break; + } + case 'error': { + this._processError(event.data); + break; + } + default: { + // no-op, unrecognized } - } else { - this._errorHandler?.(DataSourceErrorKind.Unknown, 'Unexpected message from event stream'); } }); } diff --git a/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts b/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts new file mode 100644 index 0000000000..2d0b13f85e --- /dev/null +++ b/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts @@ -0,0 +1,69 @@ +/* eslint-disable no-underscore-dangle */ +import { EventListener, EventName, LDLogger } from '../../api'; +import { DataSourceErrorKind } from '../../datasource'; +import { ObjProcessors, PayloadListener, PayloadProcessor } from './payloadProcessor'; + +// Facade interface to contain only ability to add event listeners +export interface EventStream { + addEventListener(type: EventName, listener: EventListener): void; +} + +/** + * A FDv2 PayloadStreamReader can be used to parse payloads from a stream of FDv2 events. See {@link PayloadProcessor} + * for more details. + */ +export class PayloadStreamReader { + private _payloadProcessor: PayloadProcessor; + + /** + * Creates a PayloadReader + * + * @param eventStream event stream of FDv2 events + * @param _objProcessors defines object processors for each object kind. + * @param _errorHandler that will be called with parsing errors as they are encountered + * @param _logger for logging + */ + constructor( + eventStream: EventStream, + _objProcessors: ObjProcessors, + private readonly _errorHandler?: (errorKind: DataSourceErrorKind, message: string) => void, + private readonly _logger?: LDLogger, + ) { + this._attachHandler(eventStream, 'server-intent'); + this._attachHandler(eventStream, 'put-object'); + this._attachHandler(eventStream, 'delete-object'); + this._attachHandler(eventStream, 'payload-transferred'); + this._attachHandler(eventStream, 'goodbye'); + this._attachHandler(eventStream, 'error'); + this._payloadProcessor = new PayloadProcessor(_objProcessors, _errorHandler, _logger); + } + + addPayloadListener(listener: PayloadListener) { + this._payloadProcessor.addPayloadListener(listener); + } + + removePayloadListener(listener: PayloadListener) { + this._payloadProcessor.removePayloadListener(listener); + } + + private _attachHandler(stream: EventStream, eventName: string) { + stream.addEventListener(eventName, async (event?: { data?: string }) => { + if (event?.data) { + this._logger?.debug(`Received ${eventName} event. Data is ${event.data}`); + try { + this._payloadProcessor.processEvents([ + { event: eventName, data: JSON.parse(event.data) }, + ]); + } catch { + this._logger?.error( + `Stream received data that was unable to be processed in "${eventName}" message`, + ); + this._logger?.debug(`Data follows: ${event.data}`); + this._errorHandler?.(DataSourceErrorKind.InvalidData, 'Malformed data in EventStream.'); + } + } else { + this._errorHandler?.(DataSourceErrorKind.Unknown, 'Event from EventStream missing data.'); + } + }); + } +} diff --git a/packages/shared/sdk-server/__tests__/data_sources/PollingProcessor.test.ts b/packages/shared/sdk-server/__tests__/data_sources/PollingProcessor.test.ts index 05ae9ff282..9a38a55d4c 100644 --- a/packages/shared/sdk-server/__tests__/data_sources/PollingProcessor.test.ts +++ b/packages/shared/sdk-server/__tests__/data_sources/PollingProcessor.test.ts @@ -1,13 +1,9 @@ -import { ClientContext } from '@launchdarkly/js-sdk-common'; - import { LDFeatureStore } from '../../src'; import PollingProcessor from '../../src/data_sources/PollingProcessor'; import Requestor from '../../src/data_sources/Requestor'; -import Configuration from '../../src/options/Configuration'; import AsyncStoreFacade from '../../src/store/AsyncStoreFacade'; import InMemoryFeatureStore from '../../src/store/InMemoryFeatureStore'; import VersionedDataKinds from '../../src/store/VersionedDataKinds'; -import { createBasicPlatform } from '../createBasicPlatform'; import TestLogger, { LogLevel } from '../Logger'; describe('given an event processor', () => { @@ -23,24 +19,19 @@ describe('given an event processor', () => { let store: LDFeatureStore; let storeFacade: AsyncStoreFacade; - let config: Configuration; let processor: PollingProcessor; let initSuccessHandler: jest.Mock; beforeEach(() => { store = new InMemoryFeatureStore(); storeFacade = new AsyncStoreFacade(store); - config = new Configuration({ - featureStore: store, - pollInterval: longInterval, - logger: new TestLogger(), - }); initSuccessHandler = jest.fn(); processor = new PollingProcessor( - config, requestor as unknown as Requestor, - config.featureStoreFactory(new ClientContext('', config, createBasicPlatform())), + longInterval, + store, + new TestLogger(), initSuccessHandler, ); }); @@ -87,27 +78,22 @@ describe('given a polling processor with a short poll duration', () => { const jsonData = JSON.stringify(allData); let store: LDFeatureStore; - let config: Configuration; + let testLogger: TestLogger; let processor: PollingProcessor; let initSuccessHandler: jest.Mock; let errorHandler: jest.Mock; beforeEach(() => { store = new InMemoryFeatureStore(); - config = new Configuration({ - featureStore: store, - pollInterval: shortInterval, - logger: new TestLogger(), - }); + testLogger = new TestLogger(); initSuccessHandler = jest.fn(); errorHandler = jest.fn(); - // Configuration will not let us set this as low as needed for the test. - Object.defineProperty(config, 'pollInterval', { value: 0.1 }); processor = new PollingProcessor( - config, requestor as unknown as Requestor, - config.featureStoreFactory(new ClientContext('', config, createBasicPlatform())), + shortInterval, + store, + testLogger, initSuccessHandler, errorHandler, ); @@ -146,7 +132,6 @@ describe('given a polling processor with a short poll duration', () => { expect(errorHandler).not.toBeCalled(); setTimeout(() => { expect(requestor.requestAllData.mock.calls.length).toBeGreaterThanOrEqual(2); - const testLogger = config.logger as TestLogger; expect(testLogger.getCount(LogLevel.Error)).toBe(0); expect(testLogger.getCount(LogLevel.Warn)).toBeGreaterThan(2); (done as jest.DoneCallback)(); @@ -164,7 +149,6 @@ describe('given a polling processor with a short poll duration', () => { setTimeout(() => { expect(requestor.requestAllData.mock.calls.length).toBeGreaterThanOrEqual(2); - const testLogger = config.logger as TestLogger; expect(testLogger.getCount(LogLevel.Error)).toBeGreaterThan(2); (done as jest.DoneCallback)(); }, 300); @@ -187,7 +171,6 @@ describe('given a polling processor with a short poll duration', () => { setTimeout(() => { expect(requestor.requestAllData.mock.calls.length).toBe(1); - const testLogger = config.logger as TestLogger; expect(testLogger.getCount(LogLevel.Error)).toBe(1); (done as jest.DoneCallback)(); }, 300); diff --git a/packages/shared/sdk-server/__tests__/data_sources/StreamingProcessorFDv2.test.ts b/packages/shared/sdk-server/__tests__/data_sources/StreamingProcessorFDv2.test.ts index c30a91c446..2123b7ad4f 100644 --- a/packages/shared/sdk-server/__tests__/data_sources/StreamingProcessorFDv2.test.ts +++ b/packages/shared/sdk-server/__tests__/data_sources/StreamingProcessorFDv2.test.ts @@ -66,11 +66,11 @@ const createMockEventSource = (streamUri: string = '', options: any = {}) => ({ describe('given a stream processor with mock event source', () => { let info: Info; - let streamingProcessor: subsystem.LDStreamProcessor; + let streamingProcessor: StreamingProcessorFDv2; let diagnosticsManager: internal.DiagnosticsManager; - let listener: internal.PayloadListener; let mockEventSource: any; - let mockErrorHandler: jest.Mock; + let mockDataCallback: jest.Mock; + let mockStatusCallback: jest.Mock; let simulateEvents: (e?: any) => void; let simulateError: (e: { status: number; message: string }) => boolean; @@ -84,7 +84,6 @@ describe('given a stream processor with mock event source', () => { }); beforeEach(() => { - mockErrorHandler = jest.fn(); info = basicPlatform.info; @@ -95,6 +94,7 @@ describe('given a stream processor with mock event source', () => { }), } as any; simulateEvents = (e: any = events) => { + // positions in these call arrays match order of handler attachment in payloadStreamReader mockEventSource.addEventListener.mock.calls[0][1](e['server-intent']); // server intent listener mockEventSource.addEventListener.mock.calls[1][1](e['put-object']); // put listener mockEventSource.addEventListener.mock.calls[3][1](e['payload-transferred']); // payload transferred listener @@ -102,8 +102,6 @@ describe('given a stream processor with mock event source', () => { simulateError = (e: { status: number; message: string }): boolean => mockEventSource.options.errorFilter(e); - listener = jest.fn(); - diagnosticsManager = new internal.DiagnosticsManager(sdkKey, basicPlatform, {}); streamingProcessor = new StreamingProcessorFDv2( { @@ -112,18 +110,19 @@ describe('given a stream processor with mock event source', () => { }, '/all', [], - listener, { authorization: 'my-sdk-key', 'user-agent': 'TestUserAgent/2.0.2', 'x-launchdarkly-wrapper': 'Rapper/1.2.3', }, diagnosticsManager, - mockErrorHandler, ); jest.spyOn(streamingProcessor, 'stop'); - streamingProcessor.start(); + + mockDataCallback = jest.fn(); + mockStatusCallback = jest.fn(); + streamingProcessor.start(mockDataCallback, mockStatusCallback); }); afterEach(() => { @@ -152,17 +151,15 @@ describe('given a stream processor with mock event source', () => { }, '/all', [], - listener, { authorization: 'my-sdk-key', 'user-agent': 'TestUserAgent/2.0.2', 'x-launchdarkly-wrapper': 'Rapper/1.2.3', }, diagnosticsManager, - mockErrorHandler, 22, ); - streamingProcessor.start(); + streamingProcessor.start(jest.fn(), jest.fn()); expect(basicPlatform.requests.createEventSource).toHaveBeenLastCalledWith( `${serviceEndpoints.streaming}/all`, @@ -211,7 +208,7 @@ describe('given a stream processor with mock event source', () => { it('executes payload listener', () => { simulateEvents(); - expect(listener).toHaveBeenCalled(); + expect(mockDataCallback).toHaveBeenCalled(); }); it('passes error to callback if json data is malformed', async () => { @@ -221,8 +218,11 @@ describe('given a stream processor with mock event source', () => { }, }); - expect(mockErrorHandler.mock.calls[0][0].kind).toEqual(DataSourceErrorKind.InvalidData); - expect(mockErrorHandler.mock.calls[0][0].message).toEqual('Malformed data in event stream'); + expect(mockStatusCallback).toHaveBeenNthCalledWith( + 2, + subsystem.DataSourceState.Interrupted, + new LDStreamingError(DataSourceErrorKind.InvalidData, 'Malformed data in EventStream.'), + ); }); it('calls error handler if event.data prop is missing', async () => { @@ -238,9 +238,12 @@ describe('given a stream processor with mock event source', () => { notData: '{"state": "mockState", "version": 1}', }, }); - expect(listener).not.toHaveBeenCalled(); - expect(mockErrorHandler.mock.calls[0][0].kind).toEqual(DataSourceErrorKind.Unknown); - expect(mockErrorHandler.mock.calls[0][0].message).toMatch(/unexpected message/i); + expect(mockDataCallback).not.toHaveBeenCalled(); + expect(mockStatusCallback).toHaveBeenNthCalledWith( + 2, + subsystem.DataSourceState.Interrupted, + new LDStreamingError(DataSourceErrorKind.Unknown, 'Event from EventStream missing data.'), + ); }); it('closes and stops', async () => { @@ -271,7 +274,8 @@ describe('given a stream processor with mock event source', () => { const willRetry = simulateError(testError); expect(willRetry).toBeTruthy(); - expect(mockErrorHandler).not.toBeCalled(); + expect(mockStatusCallback).toHaveBeenNthCalledWith(1, subsystem.DataSourceState.Initializing); + expect(mockStatusCallback).toHaveBeenNthCalledWith(2, subsystem.DataSourceState.Interrupted); expect(logger.warn).toBeCalledWith( expect.stringMatching(new RegExp(`${status}.*will retry`)), ); @@ -292,7 +296,9 @@ describe('given a stream processor with mock event source', () => { const willRetry = simulateError(testError); expect(willRetry).toBeFalsy(); - expect(mockErrorHandler).toBeCalledWith( + expect(mockStatusCallback).toHaveBeenNthCalledWith( + 2, + subsystem.DataSourceState.Closed, new LDStreamingError(DataSourceErrorKind.Unknown, testError.message, testError.status), ); expect(logger.error).toBeCalledWith( diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 74f4df9077..7b3fc87fa7 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -39,10 +39,9 @@ import { import BigSegmentsManager from './BigSegmentsManager'; import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; import { createPayloadListener } from './data_sources/createPayloadListenerFDv2'; -import { createStreamListeners } from './data_sources/createStreamListeners'; import DataSourceUpdates from './data_sources/DataSourceUpdates'; import OneShotInitializer from './data_sources/OneShotInitializer'; -import PollingProcessor from './data_sources/PollingProcessor'; +import PollingProcessorFDv2 from './data_sources/PollingProcessorFDv2'; import Requestor from './data_sources/Requestor'; import StreamingProcessorFDv2 from './data_sources/StreamingProcessorFDv2'; import createDiagnosticsInitConfig from './diagnostics/createDiagnosticsInitConfig'; @@ -252,16 +251,14 @@ export default class LDClientImpl implements LDClient { initializers = []; } - let synchronizers: subsystem.LDSynchronizerFactory[] = []; - if (isPollingOnlyOptions(config.dataSystem.dataSource)) { - // TODO: SDK-851 - Make polling synchronizer - synchronizers = []; - } else if ( + const synchronizers: subsystem.LDSynchronizerFactory[] = []; + // if streaming is configured, add streaming synchronizer + if ( isStandardOptions(config.dataSystem.dataSource) || isStreamingOnlyOptions(config.dataSystem.dataSource) ) { const reconnectDelay = config.dataSystem.dataSource.streamInitialReconnectDelay; - synchronizers = [ + synchronizers.push( () => new StreamingProcessorFDv2( clientContext, @@ -271,10 +268,23 @@ export default class LDClientImpl implements LDClient { this._diagnosticsManager, reconnectDelay, ), - ]; - } else { - // TODO: this is an interesting case to be figured out later - synchronizers = []; + ); + } + + // if polling is configured, add polling synchronizer + if ( + isStandardOptions(config.dataSystem.dataSource) || + isPollingOnlyOptions(config.dataSystem.dataSource) + ) { + const pollingInterval = config.dataSystem.dataSource.pollInterval; + synchronizers.push( + () => + new PollingProcessorFDv2( + new Requestor(config, this._platform.requests, baseHeaders), + pollingInterval, + config.logger, + ), + ); } this._dataSource = new CompositeDataSource(initializers, synchronizers, this.logger); diff --git a/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts new file mode 100644 index 0000000000..0b50b1ba6e --- /dev/null +++ b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts @@ -0,0 +1,144 @@ +import { + DataSourceErrorKind, + httpErrorMessage, + internal, + isHttpRecoverable, + LDLogger, + LDPollingError, + subsystem as subsystemCommon, +} from '@launchdarkly/js-sdk-common'; + +import { Flag } from '../evaluation/data/Flag'; +import { Segment } from '../evaluation/data/Segment'; +import { processFlag, processSegment } from '../store/serialization'; +import Requestor from './Requestor'; + +export type PollingErrorHandler = (err: LDPollingError) => void; + +/** + * @internal + */ +export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemSynchronizer { + private _stopped = false; + + private _timeoutHandle: any; + + constructor( + private readonly _requestor: Requestor, + private readonly _pollInterval: number = 30, + private readonly _logger?: LDLogger, + ) {} + + private _poll( + dataCallback: (basis: boolean, data: any) => void, + statusCallback: (status: subsystemCommon.DataSourceState, err?: any) => void, + ) { + if (this._stopped) { + return; + } + + const reportJsonError = (data: string) => { + this._logger?.error('Polling received invalid data'); + this._logger?.debug(`Invalid JSON follows: ${data}`); + statusCallback( + subsystemCommon.DataSourceState.Interrupted, + new LDPollingError( + DataSourceErrorKind.InvalidData, + 'Malformed JSON data in polling response', + ), + ); + }; + + const startTime = Date.now(); + this._logger?.debug('Polling LaunchDarkly for feature flag updates'); + this._requestor.requestAllData((err, body) => { + const elapsed = Date.now() - startTime; + const sleepFor = Math.max(this._pollInterval * 1000 - elapsed, 0); + + this._logger?.debug('Elapsed: %d ms, sleeping for %d ms', elapsed, sleepFor); + if (err) { + const { status } = err; + if (status && !isHttpRecoverable(status)) { + const message = httpErrorMessage(err, 'polling request'); + this._logger?.error(message); + statusCallback( + subsystemCommon.DataSourceState.Interrupted, + new LDPollingError(DataSourceErrorKind.ErrorResponse, message, status), + ); + // It is not recoverable, return and do not trigger another + // poll. + return; + } + this._logger?.warn(httpErrorMessage(err, 'polling request', 'will retry')); + } else if (body) { + try { + const parsed = JSON.parse(body) as internal.EventsSummary; + + const payloadProcessor = new internal.PayloadProcessor( + { + flag: (flag: Flag) => { + processFlag(flag); + return flag; + }, + segment: (segment: Segment) => { + processSegment(segment); + return segment; + }, + }, + (errorKind: DataSourceErrorKind, message: string) => { + statusCallback( + subsystemCommon.DataSourceState.Interrupted, + new LDPollingError(errorKind, message), + ); + }, + this._logger, + ); + + payloadProcessor.addPayloadListener((payload) => { + dataCallback(payload.basis, payload); + }); + + payloadProcessor.processEvents(parsed.events); + + // TODO: verify it is ok to schedule poll even if we have no knowledge + // of init succeeding since this layer no longer knows about the + // feature store. FDv1 code would wait for init success callback before + // scheduling next poll + this._timeoutHandle = setTimeout(() => { + this._poll(dataCallback, statusCallback); + }, sleepFor); + return; + } catch { + // We could not parse this JSON. Report the problem and fallthrough to + // start another poll. + reportJsonError(body); + } + } + + // Falling through, there was some type of error and we need to trigger + // a new poll. + this._timeoutHandle = setTimeout(() => { + this._poll(dataCallback, statusCallback); + }, sleepFor); + }); + } + + start( + dataCallback: (basis: boolean, data: any) => void, + statusCallback: (status: subsystemCommon.DataSourceState, err?: any) => void, + ) { + this._poll(dataCallback, statusCallback); + } + + stop() { + if (this._timeoutHandle) { + clearTimeout(this._timeoutHandle); + this._timeoutHandle = undefined; + } + this._stopped = true; + } + + close() { + this.stop(); + } +} diff --git a/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts b/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts index d7715f9a7a..f496dbee5a 100644 --- a/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts +++ b/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts @@ -11,10 +11,8 @@ import { LDStreamingError, Requests, shouldRetry, - StreamingErrorHandler, subsystem as subsystemCommon, } from '@launchdarkly/js-sdk-common'; -import { PayloadListener } from '@launchdarkly/js-sdk-common/dist/esm/internal'; import { Flag } from '../evaluation/data/Flag'; import { Segment } from '../evaluation/data/Segment'; @@ -77,19 +75,24 @@ export default class StreamingProcessorFDv2 implements subsystemCommon.DataSyste * * @private */ - private _retryAndHandleError(err: HttpErrorResponse, statusCallback: (status: subsystemCommon.DataSourceState, err?: any) => void) { - + private _retryAndHandleError( + err: HttpErrorResponse, + statusCallback: (status: subsystemCommon.DataSourceState, err?: any) => void, + ) { if (!shouldRetry(err)) { this._logger?.error(httpErrorMessage(err, 'streaming request')); this._logConnectionResult(false); - statusCallback(subsystemCommon.DataSourceState.Closed, new LDStreamingError(DataSourceErrorKind.ErrorResponse, err.message, err.status)) + statusCallback( + subsystemCommon.DataSourceState.Closed, + new LDStreamingError(DataSourceErrorKind.ErrorResponse, err.message, err.status), + ); return false; } this._logger?.warn(httpErrorMessage(err, 'streaming request', 'will retry')); this._logConnectionResult(false); this._logConnectionAttempt(); - statusCallback(subsystemCommon.DataSourceState.Interrupted) + statusCallback(subsystemCommon.DataSourceState.Interrupted); return true; } @@ -108,7 +111,7 @@ export default class StreamingProcessorFDv2 implements subsystemCommon.DataSyste retryResetIntervalMillis: 60 * 1000, }); this._eventSource = eventSource; - const payloadReader = new internal.PayloadReader( + const payloadReader = new internal.PayloadStreamReader( eventSource, { flag: (flag: Flag) => { @@ -121,16 +124,20 @@ export default class StreamingProcessorFDv2 implements subsystemCommon.DataSyste }, }, (errorKind: DataSourceErrorKind, message: string) => { - statusCallback(subsystemCommon.DataSourceState.Interrupted, new LDStreamingError(errorKind, message)); + statusCallback( + subsystemCommon.DataSourceState.Interrupted, + new LDStreamingError(errorKind, message), + ); + + // parsing error was encountered, defensively close the data source + this.stop(); }, this._logger, ); - payloadReader.addPayloadListener(() => { + payloadReader.addPayloadListener((payload) => { // TODO: discuss if it is satisfactory to switch from setting connection result on single event to getting a payload. Need // to double check the handling in the ServerIntent:none case. That may not trigger these payload listeners. this._logConnectionResult(true); - }); - payloadReader.addPayloadListener((payload) => { dataCallback(payload.basis, payload); }); diff --git a/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts b/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts index e580a8871f..008ae2e31f 100644 --- a/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts +++ b/packages/shared/sdk-server/src/diagnostics/createDiagnosticsInitConfig.ts @@ -27,13 +27,17 @@ const createDiagnosticsInitConfig = ( ...((isStandardOptions(config.dataSystem.dataSource) || isPollingOnlyOptions(config.dataSystem.dataSource)) && config.dataSystem.dataSource.pollInterval - ? { pollingIntervalMillis: config.dataSystem.dataSource.pollInterval } + ? { pollingIntervalMillis: secondsToMillis(config.dataSystem.dataSource.pollInterval) } : null), // include reconnect delay if data source config has it ...((isStandardOptions(config.dataSystem.dataSource) || isStreamingOnlyOptions(config.dataSystem.dataSource)) && config.dataSystem.dataSource.streamInitialReconnectDelay - ? { reconnectTimeMillis: config.dataSystem.dataSource.streamInitialReconnectDelay } + ? { + reconnectTimeMillis: secondsToMillis( + config.dataSystem.dataSource.streamInitialReconnectDelay, + ), + } : null), contextKeysFlushIntervalMillis: secondsToMillis(config.contextKeysFlushInterval), diagnosticRecordingIntervalMillis: secondsToMillis(config.diagnosticRecordingInterval), From 9b9abf3e32abc52259ed43ca32869201a4f29df6 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Mon, 7 Apr 2025 10:32:18 -0500 Subject: [PATCH 15/20] wip --- contract-tests/index.js | 2 ++ contract-tests/package.json | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/contract-tests/index.js b/contract-tests/index.js index 761a431ddc..258e8d8be3 100644 --- a/contract-tests/index.js +++ b/contract-tests/index.js @@ -39,6 +39,8 @@ app.get('/', (req, res) => { 'evaluation-hooks', 'wrapper', 'client-prereq-events', + // TODO: look into persistent layer test capabilities + // TODO: look into daemon mode testing capabilities ], }); }); diff --git a/contract-tests/package.json b/contract-tests/package.json index ef4975c946..f53e40107f 100644 --- a/contract-tests/package.json +++ b/contract-tests/package.json @@ -11,7 +11,11 @@ "dependencies": { "body-parser": "^1.19.0", "express": "^4.17.1", - "node-server-sdk": "file:../packages/sdk/server-node", - "got": "13.0.0" + "got": "13.0.0", + "node-server-sdk": "file:../packages/sdk/server-node" + }, + "resolutions": { + "@launchdarkly/js-server-sdk-common": "file:../packages/shared/sdk-server", + "@launchdarkly/js-sdk-common": "file:../packages/shared/common" } } From 2a0fdd2daa91554557bc377ff7fa07f633db34af Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 8 Apr 2025 12:13:20 -0500 Subject: [PATCH 16/20] Self review comments and fixing intent none handling --- contract-tests/index.js | 2 - contract-tests/package.json | 8 +- .../internal/fdv2/PayloadStreamReader.test.ts | 19 +++++ .../src/internal/fdv2/payloadProcessor.ts | 33 ++++++-- .../src/internal/fdv2/payloadStreamReader.ts | 2 +- .../shared/sdk-server/src/LDClientImpl.ts | 16 ++-- .../src/data_sources/OneShotInitializer.ts | 79 ++++++++++--------- .../src/data_sources/PollingProcessorFDv2.ts | 7 +- .../data_sources/StreamingProcessorFDv2.ts | 3 - 9 files changed, 104 insertions(+), 65 deletions(-) diff --git a/contract-tests/index.js b/contract-tests/index.js index 258e8d8be3..761a431ddc 100644 --- a/contract-tests/index.js +++ b/contract-tests/index.js @@ -39,8 +39,6 @@ app.get('/', (req, res) => { 'evaluation-hooks', 'wrapper', 'client-prereq-events', - // TODO: look into persistent layer test capabilities - // TODO: look into daemon mode testing capabilities ], }); }); diff --git a/contract-tests/package.json b/contract-tests/package.json index f53e40107f..ef4975c946 100644 --- a/contract-tests/package.json +++ b/contract-tests/package.json @@ -11,11 +11,7 @@ "dependencies": { "body-parser": "^1.19.0", "express": "^4.17.1", - "got": "13.0.0", - "node-server-sdk": "file:../packages/sdk/server-node" - }, - "resolutions": { - "@launchdarkly/js-server-sdk-common": "file:../packages/shared/sdk-server", - "@launchdarkly/js-sdk-common": "file:../packages/shared/common" + "node-server-sdk": "file:../packages/sdk/server-node", + "got": "13.0.0" } } diff --git a/packages/shared/common/__tests__/internal/fdv2/PayloadStreamReader.test.ts b/packages/shared/common/__tests__/internal/fdv2/PayloadStreamReader.test.ts index 5b782d9a3f..ba7a951292 100644 --- a/packages/shared/common/__tests__/internal/fdv2/PayloadStreamReader.test.ts +++ b/packages/shared/common/__tests__/internal/fdv2/PayloadStreamReader.test.ts @@ -59,6 +59,25 @@ it('it sets basis to false when intent code is xfer-changes', () => { expect(receivedPayloads[0].basis).toEqual(false); }); +it('it sets basis to false and emits empty payload when intent code is none', () => { + const mockStream = new MockEventStream(); + const receivedPayloads: Payload[] = []; + const readerUnderTest = new PayloadStreamReader(mockStream, { + mockKind: (it) => it, // obj processor that just returns the same obj + }); + readerUnderTest.addPayloadListener((it) => { + receivedPayloads.push(it); + }); + + mockStream.simulateEvent('server-intent', { + data: '{"payloads": [{"code": "none", "id": "mockId", "target": 42}]}', + }); + expect(receivedPayloads.length).toEqual(1); + expect(receivedPayloads[0].id).toEqual('mockId'); + expect(receivedPayloads[0].version).toEqual(42); + expect(receivedPayloads[0].basis).toEqual(false); +}); + it('it handles xfer-full then xfer-changes', () => { const mockStream = new MockEventStream(); const receivedPayloads: Payload[] = []; diff --git a/packages/shared/common/src/internal/fdv2/payloadProcessor.ts b/packages/shared/common/src/internal/fdv2/payloadProcessor.ts index 25dff1ab9e..815457ad14 100644 --- a/packages/shared/common/src/internal/fdv2/payloadProcessor.ts +++ b/packages/shared/common/src/internal/fdv2/payloadProcessor.ts @@ -1,7 +1,7 @@ /* eslint-disable no-underscore-dangle */ import { LDLogger } from '../../api'; import { DataSourceErrorKind } from '../../datasource'; -import { DeleteObject, PayloadTransferred, PutObject, ServerIntentData } from './proto'; +import { DeleteObject, PayloadIntent, PayloadTransferred, PutObject, ServerIntentData } from './proto'; // Used to define object processing between deserialization and payload listener invocation. This can be // used provide object sanitization logic. @@ -9,10 +9,12 @@ export interface ObjProcessors { [kind: string]: (object: any) => any; } +// Represents a collection of events (one case where this is seen is in the polling response) export interface EventsSummary { events: Event[]; } +// Represents a single event export interface Event { event: string; data: any; @@ -32,7 +34,7 @@ export interface Update { export interface Payload { id: string; version: number; - state: string; + state?: string; basis: boolean; updates: Update[]; } @@ -40,9 +42,9 @@ export interface Payload { export type PayloadListener = (payload: Payload) => void; /** - * A FDv2 PayloadReader can be used to parse payloads from a stream of FDv2 events. It will send payloads + * A FDv2 PayloadProcessor can be used to parse payloads from a stream of FDv2 events. It will send payloads * to the PayloadListeners as the payloads are received. Invalid series of events may be dropped silently, - * but the payload reader will continue to operate. + * but the payload processor will continue to operate. */ export class PayloadProcessor { private _listeners: PayloadListener[] = []; @@ -52,7 +54,7 @@ export class PayloadProcessor { private _tempUpdates: Update[] = []; /** - * Creates a PayloadReader + * Creates a PayloadProcessor * * @param _objProcessors defines object processors for each object kind. * @param _errorHandler that will be called with parsing errors as they are encountered @@ -134,8 +136,11 @@ export class PayloadProcessor { this._tempBasis = true; break; case 'xfer-changes': + this._tempBasis = false; + break; case 'none': this._tempBasis = false; + this._processIntentNone(payload); break; default: // unrecognized intent code, return @@ -188,6 +193,24 @@ export class PayloadProcessor { }); }; + private _processIntentNone = (intent: PayloadIntent) => { + // if the following properties aren't present ignore the event + if (!intent.id || !intent.target) { + return; + } + + const payload: Payload = { + id: intent.id, + version: intent.target, + basis: false, // intent none is always not a basis + updates: [], // payload with no updates to hide the intent none concept from the consumer + // note: state is absent here as that only appears in payload transferred events + }; + + this._listeners.forEach((it) => it(payload)); + this._resetAfterEmission(); + }; + private _processPayloadTransferred = (data: PayloadTransferred) => { // if the following properties haven't been provided by now, we should reset if ( diff --git a/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts b/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts index 2d0b13f85e..2ffcafa781 100644 --- a/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts +++ b/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts @@ -16,7 +16,7 @@ export class PayloadStreamReader { private _payloadProcessor: PayloadProcessor; /** - * Creates a PayloadReader + * Creates a PayloadStreamReader * * @param eventStream event stream of FDv2 events * @param _objProcessors defines object processors for each object kind. diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 7b3fc87fa7..f32e500152 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -238,17 +238,21 @@ export default class LDClientImpl implements LDClient { this._updateProcessor?.start(); } else { // make the FDv2 composite datasource with initializers/synchronizers - let initializers: subsystem.LDSynchronizerFactory[] = []; - if (isStandardOptions(config.dataSystem.dataSource)) { - initializers = [ + const initializers: subsystem.LDSynchronizerFactory[] = []; + + // if polling is allowed, use one shot initializer for performance and cost + if ( + isStandardOptions(config.dataSystem.dataSource) || + isPollingOnlyOptions(config.dataSystem.dataSource) + ) { + this._logger?.debug(`Todd was here`); + initializers.push( () => new OneShotInitializer( new Requestor(config, this._platform.requests, baseHeaders), config.logger, ), - ]; - } else { - initializers = []; + ); } const synchronizers: subsystem.LDSynchronizerFactory[] = []; diff --git a/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts b/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts index c1fd78de8c..4cce11408b 100644 --- a/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts +++ b/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts @@ -1,13 +1,15 @@ import { DataSourceErrorKind, httpErrorMessage, + internal, LDLogger, LDPollingError, subsystem as subsystemCommon, } from '@launchdarkly/js-sdk-common'; -import { deserializePoll } from '../store'; -import VersionedDataKinds from '../store/VersionedDataKinds'; +import { Flag } from '../evaluation/data/Flag'; +import { Segment } from '../evaluation/data/Segment'; +import { processFlag, processSegment } from '../store/serialization'; import Requestor from './Requestor'; /** @@ -19,22 +21,12 @@ export default class OneShotInitializer implements subsystemCommon.DataSystemIni private readonly _logger?: LDLogger, ) {} - /** - * May be called any number of times, if already started, has no effect - * @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. - */ start( dataCallback: (basis: boolean, data: any) => void, statusCallback: (status: subsystemCommon.DataSourceState, err?: any) => void, ) { statusCallback(subsystemCommon.DataSourceState.Initializing); - // @ts-ignore - // eslint-disable-next-line no-underscore-dangle - console.log(this._requestor._headers); - this._logger?.debug('Performing initialization request to LaunchDarkly for feature flag data.'); this._requestor.requestAllData((err, body) => { if (err) { @@ -54,37 +46,52 @@ export default class OneShotInitializer implements subsystemCommon.DataSystemIni subsystemCommon.DataSourceState.Closed, new LDPollingError(DataSourceErrorKind.InvalidData, 'Polling response missing body'), ); - } - - const parsed = deserializePoll(body); - if (!parsed) { - // We could not parse this JSON. Report the problem and fallthrough to - // start another poll. - this._logger?.error('Initialization response contained invalid data'); - this._logger?.debug(`Invalid JSON follows: ${body}`); - statusCallback( - subsystemCommon.DataSourceState.Closed, - new LDPollingError( - DataSourceErrorKind.InvalidData, - 'Malformed JSON data in polling response', - ), - ); } else { - const initData = { - [VersionedDataKinds.Features.namespace]: parsed.flags, - [VersionedDataKinds.Segments.namespace]: parsed.segments, - }; + try { + const parsed = JSON.parse(body) as internal.EventsSummary; + + const payloadProcessor = new internal.PayloadProcessor( + { + flag: (flag: Flag) => { + processFlag(flag); + return flag; + }, + segment: (segment: Segment) => { + processSegment(segment); + return segment; + }, + }, + (errorKind: DataSourceErrorKind, message: string) => { + statusCallback( + subsystemCommon.DataSourceState.Interrupted, + new LDPollingError(errorKind, message), + ); + }, + this._logger, + ); - // TODO: need to transform this into a payload + payloadProcessor.addPayloadListener((payload) => { + dataCallback(payload.basis, payload); + }); - dataCallback(true, initData); - statusCallback(subsystemCommon.DataSourceState.Closed); + payloadProcessor.processEvents(parsed.events); + } catch { + // We could not parse this JSON. Report the problem. + this._logger?.error('Initialization response contained invalid data'); + this._logger?.debug(`Invalid JSON follows: ${body}`); + statusCallback( + subsystemCommon.DataSourceState.Closed, + new LDPollingError( + DataSourceErrorKind.InvalidData, + 'Malformed JSON data in polling response', + ), + ); + } } }); } stop() { - // TODO: at the moment no way to cancel the inflight request via the requester API, but could - // be added in the future. + // no-op since requestor has no cancellation support } } diff --git a/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts index 0b50b1ba6e..f095b9fe26 100644 --- a/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts +++ b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts @@ -65,8 +65,7 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS subsystemCommon.DataSourceState.Interrupted, new LDPollingError(DataSourceErrorKind.ErrorResponse, message, status), ); - // It is not recoverable, return and do not trigger another - // poll. + // It is not recoverable, return and do not trigger another poll. return; } this._logger?.warn(httpErrorMessage(err, 'polling request', 'will retry')); @@ -100,10 +99,6 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS payloadProcessor.processEvents(parsed.events); - // TODO: verify it is ok to schedule poll even if we have no knowledge - // of init succeeding since this layer no longer knows about the - // feature store. FDv1 code would wait for init success callback before - // scheduling next poll this._timeoutHandle = setTimeout(() => { this._poll(dataCallback, statusCallback); }, sleepFor); diff --git a/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts b/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts index f496dbee5a..7378f79562 100644 --- a/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts +++ b/packages/shared/sdk-server/src/data_sources/StreamingProcessorFDv2.ts @@ -18,7 +18,6 @@ import { Flag } from '../evaluation/data/Flag'; import { Segment } from '../evaluation/data/Segment'; import { processFlag, processSegment } from '../store/serialization'; -// TODO: consider naming this StreamingDatasource export default class StreamingProcessorFDv2 implements subsystemCommon.DataSystemSynchronizer { private readonly _headers: { [key: string]: string | string[] }; private readonly _streamUri: string; @@ -135,8 +134,6 @@ export default class StreamingProcessorFDv2 implements subsystemCommon.DataSyste this._logger, ); payloadReader.addPayloadListener((payload) => { - // TODO: discuss if it is satisfactory to switch from setting connection result on single event to getting a payload. Need - // to double check the handling in the ServerIntent:none case. That may not trigger these payload listeners. this._logConnectionResult(true); dataCallback(payload.basis, payload); }); From 893f09fa9eee2f002202303f6c259d93e67a44e6 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 8 Apr 2025 12:29:00 -0500 Subject: [PATCH 17/20] Removing deprecations that didn't merge corrrectly --- .../sdk-server/src/api/options/LDOptions.ts | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/packages/shared/sdk-server/src/api/options/LDOptions.ts b/packages/shared/sdk-server/src/api/options/LDOptions.ts index a261cd2758..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); @@ -166,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; @@ -184,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; @@ -201,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; From eebf115076e0f2c99eedb3354fe8c07760273420 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Thu, 10 Apr 2025 15:24:52 -0500 Subject: [PATCH 18/20] addressing review comments --- .../shared/common/src/internal/fdv2/index.ts | 11 ++- .../src/internal/fdv2/payloadProcessor.ts | 20 +++-- .../src/internal/fdv2/payloadStreamReader.ts | 4 +- .../shared/sdk-server/src/LDClientImpl.ts | 22 ++--- .../src/data_sources/OneShotInitializer.ts | 72 ++++++++-------- .../src/data_sources/PollingProcessorFDv2.ts | 85 ++++++++++--------- 6 files changed, 115 insertions(+), 99 deletions(-) diff --git a/packages/shared/common/src/internal/fdv2/index.ts b/packages/shared/common/src/internal/fdv2/index.ts index fba82ccdb6..b07537ddd9 100644 --- a/packages/shared/common/src/internal/fdv2/index.ts +++ b/packages/shared/common/src/internal/fdv2/index.ts @@ -1,5 +1,5 @@ import { - EventsSummary, + FDv2EventsCollection, Payload, PayloadListener, PayloadProcessor, @@ -7,4 +7,11 @@ import { } from './payloadProcessor'; import { PayloadStreamReader } from './payloadStreamReader'; -export { EventsSummary, Payload, PayloadListener, PayloadProcessor, PayloadStreamReader, Update }; +export { + FDv2EventsCollection, + Payload, + PayloadListener, + PayloadProcessor, + PayloadStreamReader, + Update, +}; diff --git a/packages/shared/common/src/internal/fdv2/payloadProcessor.ts b/packages/shared/common/src/internal/fdv2/payloadProcessor.ts index 815457ad14..0273ccacdb 100644 --- a/packages/shared/common/src/internal/fdv2/payloadProcessor.ts +++ b/packages/shared/common/src/internal/fdv2/payloadProcessor.ts @@ -1,7 +1,13 @@ /* eslint-disable no-underscore-dangle */ import { LDLogger } from '../../api'; import { DataSourceErrorKind } from '../../datasource'; -import { DeleteObject, PayloadIntent, PayloadTransferred, PutObject, ServerIntentData } from './proto'; +import { + DeleteObject, + PayloadIntent, + PayloadTransferred, + PutObject, + ServerIntentData, +} from './proto'; // Used to define object processing between deserialization and payload listener invocation. This can be // used provide object sanitization logic. @@ -10,12 +16,12 @@ export interface ObjProcessors { } // Represents a collection of events (one case where this is seen is in the polling response) -export interface EventsSummary { - events: Event[]; +export interface FDv2EventsCollection { + events: FDv2Event[]; } // Represents a single event -export interface Event { +export interface FDv2Event { event: string; data: any; } @@ -78,11 +84,11 @@ export class PayloadProcessor { } /** - * Gives the {@link PayloadProcessor} a series of events that it will statefully, incrementally processed. + * Gives the {@link PayloadProcessor} a series of events that it will statefully, incrementally process. * This may lead to listeners being invoked as necessary. * @param events to be processed (can be a single element) */ - processEvents(events: Event[]) { + processEvents(events: FDv2Event[]) { events.forEach((event) => { switch (event.event) { case 'server-intent': { @@ -214,7 +220,7 @@ export class PayloadProcessor { private _processPayloadTransferred = (data: PayloadTransferred) => { // if the following properties haven't been provided by now, we should reset if ( - !this._tempId || // server intent hasn't been recieved yet. + !this._tempId || // server intent hasn't been received yet. !data.state || !data.version ) { diff --git a/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts b/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts index 2ffcafa781..6548a6ce60 100644 --- a/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts +++ b/packages/shared/common/src/internal/fdv2/payloadStreamReader.ts @@ -3,7 +3,9 @@ import { EventListener, EventName, LDLogger } from '../../api'; import { DataSourceErrorKind } from '../../datasource'; import { ObjProcessors, PayloadListener, PayloadProcessor } from './payloadProcessor'; -// Facade interface to contain only ability to add event listeners +/** + * Interface for an event stream. Only allows listening to events. + */ export interface EventStream { addEventListener(type: EventName, listener: EventListener): void; } diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index f32e500152..599dc291b7 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -240,20 +240,14 @@ export default class LDClientImpl implements LDClient { // make the FDv2 composite datasource with initializers/synchronizers const initializers: subsystem.LDSynchronizerFactory[] = []; - // if polling is allowed, use one shot initializer for performance and cost - if ( - isStandardOptions(config.dataSystem.dataSource) || - isPollingOnlyOptions(config.dataSystem.dataSource) - ) { - this._logger?.debug(`Todd was here`); - initializers.push( - () => - new OneShotInitializer( - new Requestor(config, this._platform.requests, baseHeaders), - config.logger, - ), - ); - } + // use one shot initializer for performance and cost + initializers.push( + () => + new OneShotInitializer( + new Requestor(config, this._platform.requests, baseHeaders), + config.logger, + ), + ); const synchronizers: subsystem.LDSynchronizerFactory[] = []; // if streaming is configured, add streaming synchronizer diff --git a/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts b/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts index 4cce11408b..19d1f486e2 100644 --- a/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts +++ b/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts @@ -46,47 +46,47 @@ export default class OneShotInitializer implements subsystemCommon.DataSystemIni subsystemCommon.DataSourceState.Closed, new LDPollingError(DataSourceErrorKind.InvalidData, 'Polling response missing body'), ); - } else { - try { - const parsed = JSON.parse(body) as internal.EventsSummary; + return; + } - const payloadProcessor = new internal.PayloadProcessor( - { - flag: (flag: Flag) => { - processFlag(flag); - return flag; - }, - segment: (segment: Segment) => { - processSegment(segment); - return segment; - }, + try { + const parsed = JSON.parse(body) as internal.FDv2EventsCollection; + const payloadProcessor = new internal.PayloadProcessor( + { + flag: (flag: Flag) => { + processFlag(flag); + return flag; }, - (errorKind: DataSourceErrorKind, message: string) => { - statusCallback( - subsystemCommon.DataSourceState.Interrupted, - new LDPollingError(errorKind, message), - ); + segment: (segment: Segment) => { + processSegment(segment); + return segment; }, - this._logger, - ); + }, + (errorKind: DataSourceErrorKind, message: string) => { + statusCallback( + subsystemCommon.DataSourceState.Interrupted, + new LDPollingError(errorKind, message), + ); + }, + this._logger, + ); - payloadProcessor.addPayloadListener((payload) => { - dataCallback(payload.basis, payload); - }); + payloadProcessor.addPayloadListener((payload) => { + dataCallback(payload.basis, payload); + }); - payloadProcessor.processEvents(parsed.events); - } catch { - // We could not parse this JSON. Report the problem. - this._logger?.error('Initialization response contained invalid data'); - this._logger?.debug(`Invalid JSON follows: ${body}`); - statusCallback( - subsystemCommon.DataSourceState.Closed, - new LDPollingError( - DataSourceErrorKind.InvalidData, - 'Malformed JSON data in polling response', - ), - ); - } + payloadProcessor.processEvents(parsed.events); + } catch { + // We could not parse this JSON. Report the problem. + this._logger?.error('Initialization response contained invalid data'); + this._logger?.debug(`Invalid JSON follows: ${body}`); + statusCallback( + subsystemCommon.DataSourceState.Closed, + new LDPollingError( + DataSourceErrorKind.InvalidData, + 'Malformed JSON data in polling response', + ), + ); } }); } diff --git a/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts index f095b9fe26..5cd63213a2 100644 --- a/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts +++ b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts @@ -69,49 +69,56 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS return; } this._logger?.warn(httpErrorMessage(err, 'polling request', 'will retry')); - } else if (body) { - try { - const parsed = JSON.parse(body) as internal.EventsSummary; - - const payloadProcessor = new internal.PayloadProcessor( - { - flag: (flag: Flag) => { - processFlag(flag); - return flag; - }, - segment: (segment: Segment) => { - processSegment(segment); - return segment; - }, - }, - (errorKind: DataSourceErrorKind, message: string) => { - statusCallback( - subsystemCommon.DataSourceState.Interrupted, - new LDPollingError(errorKind, message), - ); - }, - this._logger, - ); - - payloadProcessor.addPayloadListener((payload) => { - dataCallback(payload.basis, payload); - }); + // schedule poll + this._timeoutHandle = setTimeout(() => { + this._poll(dataCallback, statusCallback); + }, sleepFor); + return; + } - payloadProcessor.processEvents(parsed.events); + if (!body) { + this._logger?.warn('Response missing body, will retry.'); + // schedule poll + this._timeoutHandle = setTimeout(() => { + this._poll(dataCallback, statusCallback); + }, sleepFor); + return; + } - this._timeoutHandle = setTimeout(() => { - this._poll(dataCallback, statusCallback); - }, sleepFor); - return; - } catch { - // We could not parse this JSON. Report the problem and fallthrough to - // start another poll. - reportJsonError(body); - } + try { + const parsed = JSON.parse(body) as internal.FDv2EventsCollection; + const payloadProcessor = new internal.PayloadProcessor( + { + flag: (flag: Flag) => { + processFlag(flag); + return flag; + }, + segment: (segment: Segment) => { + processSegment(segment); + return segment; + }, + }, + (errorKind: DataSourceErrorKind, message: string) => { + statusCallback( + subsystemCommon.DataSourceState.Interrupted, + new LDPollingError(errorKind, message), + ); + }, + this._logger, + ); + + payloadProcessor.addPayloadListener((payload) => { + dataCallback(payload.basis, payload); + }); + + payloadProcessor.processEvents(parsed.events); + } catch { + // We could not parse this JSON. Report the problem and fallthrough to + // start another poll. + reportJsonError(body); } - // Falling through, there was some type of error and we need to trigger - // a new poll. + // schedule poll this._timeoutHandle = setTimeout(() => { this._poll(dataCallback, statusCallback); }, sleepFor); From 87bcec4a8a5fbd2d5631e5df0f11df4cf69bdc08 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Thu, 10 Apr 2025 17:00:36 -0500 Subject: [PATCH 19/20] Adding unit tests --- .../data_sources/PollingProcessorFDv2.test.ts | 207 ++++++++++++++++++ .../StreamingProcessorFDv2.test.ts | 17 +- .../src/data_sources/PollingProcessorFDv2.ts | 18 +- 3 files changed, 238 insertions(+), 4 deletions(-) create mode 100644 packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts diff --git a/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts b/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts new file mode 100644 index 0000000000..0165f67d0a --- /dev/null +++ b/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts @@ -0,0 +1,207 @@ +import { DataSourceErrorKind, LDPollingError, subsystem } from '../../src'; +import PollingProcessorFDv2 from '../../src/data_sources/PollingProcessorFDv2'; +import Requestor from '../../src/data_sources/Requestor'; +import TestLogger, { LogLevel } from '../Logger'; + +describe('given an event processor', () => { + const requestor = { + requestAllData: jest.fn(), + }; + const longInterval = 100000; + const allEvents = { + events: [ + { + event: 'server-intent', + data: { payloads: [{ code: 'xfer-full', id: 'mockId' }] }, + }, + { + event: 'put-object', + data: { + kind: 'flag', + key: 'flagA', + version: 123, + object: { objectFieldA: 'objectValueA' }, + }, + }, + { + event: 'payload-transferred', + data: { state: 'mockState', version: 1 }, + }, + ], + }; + const jsonData = JSON.stringify(allEvents); + + let processor: PollingProcessorFDv2; + const mockDataCallback = jest.fn(); + const mockStatusCallback = jest.fn(); + + beforeEach(() => { + processor = new PollingProcessorFDv2( + requestor as unknown as Requestor, + longInterval, + new TestLogger(), + ); + }); + + afterEach(() => { + processor.stop(); + jest.restoreAllMocks(); + }); + + it('makes no requests before being started', () => { + expect(requestor.requestAllData).not.toHaveBeenCalled(); + }); + + it('polls immediately on start', () => { + processor.start(mockDataCallback, mockStatusCallback); + expect(requestor.requestAllData).toHaveBeenCalledTimes(1); + expect(mockDataCallback).not.toHaveBeenCalled(); + expect(mockStatusCallback).not.toHaveBeenCalled(); + }); + + it('calls callback on success', () => { + requestor.requestAllData = jest.fn((cb) => cb(undefined, jsonData)); + processor.start(mockDataCallback, mockStatusCallback); + expect(mockDataCallback).toHaveBeenNthCalledWith(1, true, { + basis: true, + id: `mockId`, + state: `mockState`, + updates: [ + { + kind: `flag`, + key: `flagA`, + version: 123, + object: { objectFieldA: 'objectValueA' }, + }, + ], + version: 1, + }); + }); +}); + +describe('given a polling processor with a short poll duration', () => { + const requestor = { + requestAllData: jest.fn(), + }; + const shortInterval = 0.1; + const allData = { flags: { flag: { version: 1 } }, segments: { segment: { version: 1 } } }; + const jsonData = JSON.stringify(allData); + + let testLogger: TestLogger; + let processor: PollingProcessorFDv2; + const mockDataCallback = jest.fn(); + const mockStatusCallback = jest.fn(); + + beforeEach(() => { + testLogger = new TestLogger(); + + processor = new PollingProcessorFDv2( + requestor as unknown as Requestor, + shortInterval, + testLogger, + ); + }); + + afterEach(() => { + processor.stop(); + jest.resetAllMocks(); + }); + + it('polls repeatedly', (done) => { + requestor.requestAllData = jest.fn((cb) => cb(undefined, jsonData)); + + processor.start(mockDataCallback, mockStatusCallback); + setTimeout(() => { + expect(requestor.requestAllData.mock.calls.length).toBeGreaterThanOrEqual(4); + done(); + }, 500); + }); + + it.each([400, 408, 429, 500, 503])( + 'continues polling after recoverable error', + (status, done) => { + requestor.requestAllData = jest.fn((cb) => + cb( + { + status, + }, + undefined, + ), + ); + + processor.start(mockDataCallback, mockStatusCallback); + expect(mockDataCallback).not.toHaveBeenCalled(); + expect(mockStatusCallback).toHaveBeenNthCalledWith(1, subsystem.DataSourceState.Initializing); + expect(mockStatusCallback).toHaveBeenNthCalledWith( + 2, + subsystem.DataSourceState.Interrupted, + new LDPollingError( + DataSourceErrorKind.ErrorResponse, + `Received error ${status} for polling request - will retry`, + status as number, + ), + ); + setTimeout(() => { + expect(requestor.requestAllData.mock.calls.length).toBeGreaterThanOrEqual(2); + expect(testLogger.getCount(LogLevel.Error)).toBe(0); + expect(testLogger.getCount(LogLevel.Warn)).toBeGreaterThan(2); + (done as jest.DoneCallback)(); + }, 300); + }, + ); + + it('continues polling after receiving invalid JSON', (done) => { + requestor.requestAllData = jest.fn((cb) => cb(undefined, '{sad')); + + processor.start(mockDataCallback, mockStatusCallback); + expect(mockDataCallback).not.toHaveBeenCalled(); + expect(mockStatusCallback).toHaveBeenNthCalledWith(1, subsystem.DataSourceState.Initializing); + expect(mockStatusCallback).toHaveBeenNthCalledWith( + 2, + subsystem.DataSourceState.Interrupted, + new LDPollingError( + DataSourceErrorKind.ErrorResponse, + `Malformed JSON data in polling response`, + ), + ); + + setTimeout(() => { + expect(requestor.requestAllData.mock.calls.length).toBeGreaterThanOrEqual(2); + expect(testLogger.getCount(LogLevel.Error)).toBeGreaterThan(2); + (done as jest.DoneCallback)(); + }, 300); + }); + + it.each([401, 403])( + 'does not continue after non-recoverable error', + (status, done) => { + requestor.requestAllData = jest.fn((cb) => + cb( + { + status, + }, + undefined, + ), + ); + processor.start(mockDataCallback, mockStatusCallback); + expect(mockDataCallback).not.toHaveBeenCalled(); + expect(mockStatusCallback).toHaveBeenNthCalledWith(1, subsystem.DataSourceState.Initializing); + expect(mockStatusCallback).toHaveBeenNthCalledWith( + 2, + subsystem.DataSourceState.Interrupted, + new LDPollingError( + DataSourceErrorKind.ErrorResponse, + status === 401 + ? `Received error ${status} (invalid SDK key) for polling request - giving up permanently` + : `Received error ${status} for polling request - giving up permanently`, + status as number, + ), + ); + setTimeout(() => { + expect(requestor.requestAllData.mock.calls.length).toBe(1); + expect(testLogger.getCount(LogLevel.Error)).toBe(1); + (done as jest.DoneCallback)(); + }, 300); + }, + ); +}); diff --git a/packages/shared/sdk-server/__tests__/data_sources/StreamingProcessorFDv2.test.ts b/packages/shared/sdk-server/__tests__/data_sources/StreamingProcessorFDv2.test.ts index 2123b7ad4f..4f864bc51b 100644 --- a/packages/shared/sdk-server/__tests__/data_sources/StreamingProcessorFDv2.test.ts +++ b/packages/shared/sdk-server/__tests__/data_sources/StreamingProcessorFDv2.test.ts @@ -37,7 +37,7 @@ const events = { data: '{"payloads": [{"code": "xfer-full", "id": "mockId"}]}', }, 'put-object': { - data: '{"kind": "mockKind", "key": "flagA", "version": 123, "object": {"objectFieldA": "objectValueA"}}', + data: '{"kind": "flag", "key": "flagA", "version": 123, "object": {"objectFieldA": "objectValueA"}}', }, 'payload-transferred': { data: '{"state": "mockState", "version": 1}', @@ -208,7 +208,20 @@ describe('given a stream processor with mock event source', () => { it('executes payload listener', () => { simulateEvents(); - expect(mockDataCallback).toHaveBeenCalled(); + expect(mockDataCallback).toHaveBeenNthCalledWith(1, true, { + basis: true, + id: `mockId`, + state: `mockState`, + updates: [ + { + kind: `flag`, + key: `flagA`, + version: 123, + object: { objectFieldA: 'objectValueA' }, + }, + ], + version: 1, + }); }); it('passes error to callback if json data is malformed', async () => { diff --git a/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts index 5cd63213a2..9e8965f727 100644 --- a/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts +++ b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts @@ -20,9 +20,10 @@ export type PollingErrorHandler = (err: LDPollingError) => void; */ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemSynchronizer { private _stopped = false; - private _timeoutHandle: any; + private _statusCallback?: (status: subsystemCommon.DataSourceState, err?: any) => void; + constructor( private readonly _requestor: Requestor, private readonly _pollInterval: number = 30, @@ -68,7 +69,13 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS // It is not recoverable, return and do not trigger another poll. return; } - this._logger?.warn(httpErrorMessage(err, 'polling request', 'will retry')); + + const message = httpErrorMessage(err, 'polling request', 'will retry'); + statusCallback( + subsystemCommon.DataSourceState.Interrupted, + new LDPollingError(DataSourceErrorKind.ErrorResponse, message, status), + ); + this._logger?.warn(message); // schedule poll this._timeoutHandle = setTimeout(() => { this._poll(dataCallback, statusCallback); @@ -112,6 +119,9 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS }); payloadProcessor.processEvents(parsed.events); + + // TODO: SDK-855 implement blocking duplicate data source state events in DataAvailability API + statusCallback(subsystemCommon.DataSourceState.Valid); } catch { // We could not parse this JSON. Report the problem and fallthrough to // start another poll. @@ -129,6 +139,8 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS dataCallback: (basis: boolean, data: any) => void, statusCallback: (status: subsystemCommon.DataSourceState, err?: any) => void, ) { + this._statusCallback = statusCallback; // hold reference for usage in stop() + statusCallback(subsystemCommon.DataSourceState.Initializing); this._poll(dataCallback, statusCallback); } @@ -137,7 +149,9 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS clearTimeout(this._timeoutHandle); this._timeoutHandle = undefined; } + this._statusCallback?.(subsystemCommon.DataSourceState.Closed); this._stopped = true; + this._statusCallback = undefined; } close() { From 80f8d673f4fb624cedcf3dc5d271bf0427b6b186 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 11 Apr 2025 12:22:18 -0500 Subject: [PATCH 20/20] Adding one shot initializer tests --- .../OneShotInitializerFDv2.test.ts | 81 +++++++++++++++++++ .../data_sources/PollingProcessorFDv2.test.ts | 26 +++++- .../shared/sdk-server/src/LDClientImpl.ts | 4 +- ...itializer.ts => OneShotInitializerFDv2.ts} | 16 ++-- .../src/data_sources/PollingProcessorFDv2.ts | 26 +++--- 5 files changed, 126 insertions(+), 27 deletions(-) create mode 100644 packages/shared/sdk-server/__tests__/data_sources/OneShotInitializerFDv2.test.ts rename packages/shared/sdk-server/src/data_sources/{OneShotInitializer.ts => OneShotInitializerFDv2.ts} (80%) diff --git a/packages/shared/sdk-server/__tests__/data_sources/OneShotInitializerFDv2.test.ts b/packages/shared/sdk-server/__tests__/data_sources/OneShotInitializerFDv2.test.ts new file mode 100644 index 0000000000..a590d6f03a --- /dev/null +++ b/packages/shared/sdk-server/__tests__/data_sources/OneShotInitializerFDv2.test.ts @@ -0,0 +1,81 @@ +import { DataSourceErrorKind, LDPollingError, subsystem } from '../../src'; +import OneShotInitializerFDv2 from '../../src/data_sources/OneShotInitializerFDv2'; +import PollingProcessorFDv2 from '../../src/data_sources/PollingProcessorFDv2'; +import Requestor from '../../src/data_sources/Requestor'; +import TestLogger, { LogLevel } from '../Logger'; + +describe('given a one shot initializer', () => { + const requestor = { + requestAllData: jest.fn(), + }; + const allEvents = { + events: [ + { + event: 'server-intent', + data: { payloads: [{ code: 'xfer-full', id: 'mockId' }] }, + }, + { + event: 'put-object', + data: { + kind: 'flag', + key: 'flagA', + version: 123, + object: { objectFieldA: 'objectValueA' }, + }, + }, + { + event: 'payload-transferred', + data: { state: 'mockState', version: 1 }, + }, + ], + }; + const jsonData = JSON.stringify(allEvents); + + let initializer: OneShotInitializerFDv2; + const mockDataCallback = jest.fn(); + const mockStatusCallback = jest.fn(); + let testLogger: TestLogger; + + beforeEach(() => { + testLogger = new TestLogger(); + initializer = new OneShotInitializerFDv2( + requestor as unknown as Requestor, + testLogger, + ); + }); + + afterEach(() => { + initializer.stop(); + jest.restoreAllMocks(); + }); + + it('makes no requests before being started', () => { + expect(requestor.requestAllData).not.toHaveBeenCalled(); + }); + + it('polls immediately on start', () => { + initializer.start(mockDataCallback, mockStatusCallback); + expect(requestor.requestAllData).toHaveBeenCalledTimes(1); + expect(mockDataCallback).not.toHaveBeenCalled(); + expect(mockStatusCallback).toHaveBeenNthCalledWith(1, subsystem.DataSourceState.Initializing); + }); + + it('calls callback on success', () => { + requestor.requestAllData = jest.fn((cb) => cb(undefined, jsonData)); + initializer.start(mockDataCallback, mockStatusCallback); + expect(mockDataCallback).toHaveBeenNthCalledWith(1, true, { + basis: true, + id: `mockId`, + state: `mockState`, + updates: [ + { + kind: `flag`, + key: `flagA`, + version: 123, + object: { objectFieldA: 'objectValueA' }, + }, + ], + version: 1, + }); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts b/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts index 0165f67d0a..e0d77e5fd2 100644 --- a/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts +++ b/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts @@ -56,7 +56,7 @@ describe('given an event processor', () => { processor.start(mockDataCallback, mockStatusCallback); expect(requestor.requestAllData).toHaveBeenCalledTimes(1); expect(mockDataCallback).not.toHaveBeenCalled(); - expect(mockStatusCallback).not.toHaveBeenCalled(); + expect(mockStatusCallback).toHaveBeenNthCalledWith(1, subsystem.DataSourceState.Initializing); }); it('calls callback on success', () => { @@ -84,8 +84,28 @@ describe('given a polling processor with a short poll duration', () => { requestAllData: jest.fn(), }; const shortInterval = 0.1; - const allData = { flags: { flag: { version: 1 } }, segments: { segment: { version: 1 } } }; - const jsonData = JSON.stringify(allData); + const allEvents = { + events: [ + { + event: 'server-intent', + data: { payloads: [{ code: 'xfer-full', id: 'mockId' }] }, + }, + { + event: 'put-object', + data: { + kind: 'flag', + key: 'flagA', + version: 123, + object: { objectFieldA: 'objectValueA' }, + }, + }, + { + event: 'payload-transferred', + data: { state: 'mockState', version: 1 }, + }, + ], + }; + const jsonData = JSON.stringify(allEvents); let testLogger: TestLogger; let processor: PollingProcessorFDv2; diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 599dc291b7..e8afbf53f4 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -40,7 +40,7 @@ import BigSegmentsManager from './BigSegmentsManager'; import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; import { createPayloadListener } from './data_sources/createPayloadListenerFDv2'; import DataSourceUpdates from './data_sources/DataSourceUpdates'; -import OneShotInitializer from './data_sources/OneShotInitializer'; +import OneShotInitializerFDv2 from './data_sources/OneShotInitializerFDv2'; import PollingProcessorFDv2 from './data_sources/PollingProcessorFDv2'; import Requestor from './data_sources/Requestor'; import StreamingProcessorFDv2 from './data_sources/StreamingProcessorFDv2'; @@ -243,7 +243,7 @@ export default class LDClientImpl implements LDClient { // use one shot initializer for performance and cost initializers.push( () => - new OneShotInitializer( + new OneShotInitializerFDv2( new Requestor(config, this._platform.requests, baseHeaders), config.logger, ), diff --git a/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts b/packages/shared/sdk-server/src/data_sources/OneShotInitializerFDv2.ts similarity index 80% rename from packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts rename to packages/shared/sdk-server/src/data_sources/OneShotInitializerFDv2.ts index 19d1f486e2..8049511e8d 100644 --- a/packages/shared/sdk-server/src/data_sources/OneShotInitializer.ts +++ b/packages/shared/sdk-server/src/data_sources/OneShotInitializerFDv2.ts @@ -15,7 +15,7 @@ import Requestor from './Requestor'; /** * @internal */ -export default class OneShotInitializer implements subsystemCommon.DataSystemInitializer { +export default class OneShotInitializerFDv2 implements subsystemCommon.DataSystemInitializer { constructor( private readonly _requestor: Requestor, private readonly _logger?: LDLogger, @@ -31,7 +31,7 @@ export default class OneShotInitializer implements subsystemCommon.DataSystemIni this._requestor.requestAllData((err, body) => { if (err) { const { status } = err; - const message = httpErrorMessage(err, 'initializer', 'will not retry'); + const message = httpErrorMessage(err, 'initializer', 'initializer does not retry'); this._logger?.error(message); statusCallback( subsystemCommon.DataSourceState.Closed, @@ -41,10 +41,13 @@ export default class OneShotInitializer implements subsystemCommon.DataSystemIni } if (!body) { - this._logger?.error('Initialization response missing body'); + this._logger?.error('One shot initializer response missing body.'); statusCallback( subsystemCommon.DataSourceState.Closed, - new LDPollingError(DataSourceErrorKind.InvalidData, 'Polling response missing body'), + new LDPollingError( + DataSourceErrorKind.InvalidData, + 'One shot initializer response missing body.', + ), ); return; } @@ -76,10 +79,13 @@ export default class OneShotInitializer implements subsystemCommon.DataSystemIni }); payloadProcessor.processEvents(parsed.events); + + // TODO: SDK-855 implement blocking duplicate data source state events in DataAvailability API + statusCallback(subsystemCommon.DataSourceState.Valid); } catch { // We could not parse this JSON. Report the problem. this._logger?.error('Initialization response contained invalid data'); - this._logger?.debug(`Invalid JSON follows: ${body}`); + this._logger?.debug(`Malformed JSON follows: ${body}`); statusCallback( subsystemCommon.DataSourceState.Closed, new LDPollingError( diff --git a/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts index 9e8965f727..2f155cc288 100644 --- a/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts +++ b/packages/shared/sdk-server/src/data_sources/PollingProcessorFDv2.ts @@ -38,18 +38,6 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS return; } - const reportJsonError = (data: string) => { - this._logger?.error('Polling received invalid data'); - this._logger?.debug(`Invalid JSON follows: ${data}`); - statusCallback( - subsystemCommon.DataSourceState.Interrupted, - new LDPollingError( - DataSourceErrorKind.InvalidData, - 'Malformed JSON data in polling response', - ), - ); - }; - const startTime = Date.now(); this._logger?.debug('Polling LaunchDarkly for feature flag updates'); this._requestor.requestAllData((err, body) => { @@ -125,7 +113,15 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS } catch { // We could not parse this JSON. Report the problem and fallthrough to // start another poll. - reportJsonError(body); + this._logger?.error('Polling received malformed data'); + this._logger?.debug(`Malformed JSON follows: ${body}`); + statusCallback( + subsystemCommon.DataSourceState.Interrupted, + new LDPollingError( + DataSourceErrorKind.InvalidData, + 'Malformed JSON data in polling response', + ), + ); } // schedule poll @@ -153,8 +149,4 @@ export default class PollingProcessorFDv2 implements subsystemCommon.DataSystemS this._stopped = true; this._statusCallback = undefined; } - - close() { - this.stop(); - } }