diff --git a/CHANGES.txt b/CHANGES.txt index 711d3ca3..9163d52f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,12 @@ +2.8.0 (October XX, 2025) + - Added `client.whenReady()` and `client.whenReadyFromCache()` methods to replace the deprecated `client.ready()` method, which has an issue causing the returned promise to hang when using async/await syntax if it was rejected. + - Updated the SDK_READY_FROM_CACHE event to be emitted alongside the SDK_READY event if it hasn’t already been emitted. + +2.7.1 (October 8, 2025) + - Bugfix - Update `debug` option to support log levels when `logger` option is used. + 2.7.0 (October 7, 2025) - - Added support for custom loggers: added `logger` configuration option and `LoggerAPI.setLogger` method to allow the SDK to use a custom logger. + - Added support for custom loggers: added `logger` configuration option and `factory.Logger.setLogger` method to allow the SDK to use a custom logger. 2.6.0 (September 18, 2025) - Added `storage.wrapper` configuration option to allow the SDK to use a custom storage wrapper for the storage type `LOCALSTORAGE`. Default value is `window.localStorage`. diff --git a/package-lock.json b/package-lock.json index 7665ca86..356a4e93 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.7.0", + "version": "2.7.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "2.7.0", + "version": "2.7.1", "license": "Apache-2.0", "dependencies": { "@types/ioredis": "^4.28.0", diff --git a/package.json b/package.json index c14387e2..30ea1784 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.7.0", + "version": "2.7.1", "description": "Split JavaScript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/logger/constants.ts b/src/logger/constants.ts index de1ebe58..ca331f82 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -60,7 +60,7 @@ export const SUBMITTERS_PUSH_PAGE_HIDDEN = 125; export const ENGINE_VALUE_INVALID = 200; export const ENGINE_VALUE_NO_ATTRIBUTES = 201; export const CLIENT_NO_LISTENER = 202; -export const CLIENT_NOT_READY = 203; +export const CLIENT_NOT_READY_FROM_CACHE = 203; export const SYNC_MYSEGMENTS_FETCH_RETRY = 204; export const SYNC_SPLITS_FETCH_FAILS = 205; export const STREAMING_PARSING_ERROR_FAILS = 206; diff --git a/src/logger/index.ts b/src/logger/index.ts index 09c3c7a9..903fb2a6 100644 --- a/src/logger/index.ts +++ b/src/logger/index.ts @@ -72,8 +72,6 @@ export class Logger implements ILogger { if (logger) { if (isLogger(logger)) { this.logger = logger; - // If custom logger is set, all logs are either enabled or disabled - if (this.logLevel !== LogLevelIndexes.NONE) this.setLogLevel(LogLevels.DEBUG); return; } else { this.error('Invalid `logger` instance. It must be an object with `debug`, `info`, `warn` and `error` methods. Defaulting to `console.log`'); diff --git a/src/logger/messages/warn.ts b/src/logger/messages/warn.ts index 81cfda1a..a0bf31a9 100644 --- a/src/logger/messages/warn.ts +++ b/src/logger/messages/warn.ts @@ -14,8 +14,8 @@ export const codesWarn: [number, string][] = codesError.concat([ [c.SUBMITTERS_PUSH_FAILS, c.LOG_PREFIX_SYNC_SUBMITTERS + 'Dropping %s after retry. Reason: %s.'], [c.SUBMITTERS_PUSH_RETRY, c.LOG_PREFIX_SYNC_SUBMITTERS + 'Failed to push %s, keeping data to retry on next iteration. Reason: %s.'], // client status - [c.CLIENT_NOT_READY, '%s: the SDK is not ready, results may be incorrect%s. Make sure to wait for SDK readiness before using this method.'], - [c.CLIENT_NO_LISTENER, 'No listeners for SDK Readiness detected. Incorrect control treatments could have been logged if you called getTreatment/s while the SDK was not yet ready.'], + [c.CLIENT_NOT_READY_FROM_CACHE, '%s: the SDK is not ready to evaluate. Results may be incorrect%s. Make sure to wait for SDK readiness before using this method.'], + [c.CLIENT_NO_LISTENER, 'No listeners for SDK_READY event detected. Incorrect control treatments could have been logged if you called getTreatment/s while the SDK was not yet synchronized with the backend.'], // input validation [c.WARN_SETTING_NULL, '%s: Property "%s" is of invalid type. Setting value to null.'], [c.WARN_TRIMMING_PROPERTIES, '%s: more than 300 properties were provided. Some of them will be trimmed when processed.'], diff --git a/src/readiness/__tests__/sdkReadinessManager.spec.ts b/src/readiness/__tests__/sdkReadinessManager.spec.ts index 9044fc72..4d47d12f 100644 --- a/src/readiness/__tests__/sdkReadinessManager.spec.ts +++ b/src/readiness/__tests__/sdkReadinessManager.spec.ts @@ -1,11 +1,12 @@ // @ts-nocheck import { loggerMock } from '../../logger/__tests__/sdkLogger.mock'; import SplitIO from '../../../types/splitio'; -import { SDK_READY, SDK_READY_FROM_CACHE, SDK_READY_TIMED_OUT, SDK_UPDATE } from '../constants'; +import { SDK_READY, SDK_READY_FROM_CACHE, SDK_READY_TIMED_OUT, SDK_UPDATE, SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED, SDK_SPLITS_CACHE_LOADED } from '../constants'; import { sdkReadinessManagerFactory } from '../sdkReadinessManager'; import { IReadinessManager } from '../types'; import { ERROR_CLIENT_LISTENER, CLIENT_READY_FROM_CACHE, CLIENT_READY, CLIENT_NO_LISTENER } from '../../logger/constants'; import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks'; +import { EventEmitter } from '../../utils/MinEvents'; const EventEmitterMock = jest.fn(() => ({ on: jest.fn(), @@ -19,24 +20,37 @@ const EventEmitterMock = jest.fn(() => ({ // Makes readinessManager emit SDK_READY & update isReady flag function emitReadyEvent(readinessManager: IReadinessManager) { + if (readinessManager.gate instanceof EventEmitter) { + readinessManager.splits.emit(SDK_SPLITS_ARRIVED); + readinessManager.segments.emit(SDK_SEGMENTS_ARRIVED); + return; + } + readinessManager.splits.once.mock.calls[0][1](); readinessManager.splits.on.mock.calls[0][1](); readinessManager.segments.once.mock.calls[0][1](); readinessManager.segments.on.mock.calls[0][1](); readinessManager.gate.once.mock.calls[0][1](); + if (readinessManager.gate.once.mock.calls[3]) readinessManager.gate.once.mock.calls[3][1](); // whenReady promise } const timeoutErrorMessage = 'Split SDK emitted SDK_READY_TIMED_OUT event.'; // Makes readinessManager emit SDK_READY_TIMED_OUT & update hasTimedout flag function emitTimeoutEvent(readinessManager: IReadinessManager) { + if (readinessManager.gate instanceof EventEmitter) { + readinessManager.timeout(); + return; + } + readinessManager.gate.once.mock.calls[1][1](timeoutErrorMessage); readinessManager.hasTimedout = () => true; + if (readinessManager.gate.once.mock.calls[4]) readinessManager.gate.once.mock.calls[4][1](timeoutErrorMessage); // whenReady promise } describe('SDK Readiness Manager - Event emitter', () => { - afterEach(() => { loggerMock.mockClear(); }); + beforeEach(() => { loggerMock.mockClear(); }); test('Providing the gate object to get the SDK status interface that manages events', () => { expect(typeof sdkReadinessManagerFactory).toBe('function'); // The module exposes a function. @@ -50,7 +64,8 @@ describe('SDK Readiness Manager - Event emitter', () => { expect(sdkStatus[propName]).toBeTruthy(); // The sdkStatus exposes all minimal EventEmitter functionality. }); - expect(typeof sdkStatus.ready).toBe('function'); // The sdkStatus exposes a .ready() function. + expect(typeof sdkStatus.whenReady).toBe('function'); // The sdkStatus exposes a .whenReady() function. + expect(typeof sdkStatus.whenReadyFromCache).toBe('function'); // The sdkStatus exposes a .whenReadyFromCache() function. expect(typeof sdkStatus.__getStatus).toBe('function'); // The sdkStatus exposes a .__getStatus() function. expect(sdkStatus.__getStatus()).toEqual({ isReady: false, isReadyFromCache: false, isTimedout: false, hasTimedout: false, isDestroyed: false, isOperational: false, lastUpdate: 0 @@ -67,9 +82,9 @@ describe('SDK Readiness Manager - Event emitter', () => { const sdkReadyResolvePromiseCall = gateMock.once.mock.calls[0]; const sdkReadyRejectPromiseCall = gateMock.once.mock.calls[1]; const sdkReadyFromCacheListenersCheckCall = gateMock.once.mock.calls[2]; - expect(sdkReadyResolvePromiseCall[0]).toBe(SDK_READY); // A one time only subscription is on the SDK_READY event, for resolving the full blown ready promise and to check for callbacks warning. - expect(sdkReadyRejectPromiseCall[0]).toBe(SDK_READY_TIMED_OUT); // A one time only subscription is also on the SDK_READY_TIMED_OUT event, for rejecting the full blown ready promise. - expect(sdkReadyFromCacheListenersCheckCall[0]).toBe(SDK_READY_FROM_CACHE); // A one time only subscription is on the SDK_READY_FROM_CACHE event, to log the event and update internal state. + expect(sdkReadyResolvePromiseCall[0]).toBe(SDK_READY); // A one time only subscription is on the SDK_READY event + expect(sdkReadyRejectPromiseCall[0]).toBe(SDK_READY_TIMED_OUT); // A one time only subscription is also on the SDK_READY_TIMED_OUT event + expect(sdkReadyFromCacheListenersCheckCall[0]).toBe(SDK_READY_FROM_CACHE); // A one time only subscription is on the SDK_READY_FROM_CACHE event expect(gateMock.on).toBeCalledTimes(2); // It should also add two persistent listeners @@ -98,7 +113,7 @@ describe('SDK Readiness Manager - Event emitter', () => { emitReadyEvent(sdkReadinessManager.readinessManager); - expect(loggerMock.warn).toBeCalledTimes(1); // If the SDK_READY event fires and we have no callbacks for it (neither event nor ready promise) we get a warning. + expect(loggerMock.warn).toBeCalledTimes(1); // If the SDK_READY event fires and we have no callbacks for it (neither event nor whenReady promise) we get a warning. expect(loggerMock.warn).toBeCalledWith(CLIENT_NO_LISTENER); // Telling us there were no listeners and evaluations before this point may have been incorrect. expect(loggerMock.info).toBeCalledTimes(1); // If the SDK_READY event fires, we get a info message. @@ -199,77 +214,98 @@ describe('SDK Readiness Manager - Event emitter', () => { }); }); -describe('SDK Readiness Manager - Ready promise', () => { +describe('SDK Readiness Manager - Promises', () => { - test('.ready() promise behavior for clients', async () => { - const sdkReadinessManager = sdkReadinessManagerFactory(EventEmitterMock, fullSettings); + test('.whenReady() and .whenReadyFromCache() promises resolves when SDK_READY is emitted', async () => { + const sdkReadinessManager = sdkReadinessManagerFactory(EventEmitter, fullSettings); + + // make the SDK ready from cache + sdkReadinessManager.readinessManager.splits.emit(SDK_SPLITS_CACHE_LOADED); + expect(await sdkReadinessManager.sdkStatus.whenReadyFromCache()).toBe(false); - const ready = sdkReadinessManager.sdkStatus.ready(); - expect(ready instanceof Promise).toBe(true); // It should return a promise. + // validate error log for SDK_READY_FROM_CACHE + expect(loggerMock.error).not.toBeCalled(); + sdkReadinessManager.readinessManager.gate.on(SDK_READY_FROM_CACHE, () => {}); + expect(loggerMock.error).toBeCalledWith(ERROR_CLIENT_LISTENER, ['SDK_READY_FROM_CACHE']); - // make the SDK "ready" + const readyFromCache = sdkReadinessManager.sdkStatus.whenReadyFromCache(); + const ready = sdkReadinessManager.sdkStatus.whenReady(); + + // make the SDK ready emitReadyEvent(sdkReadinessManager.readinessManager); + expect(await sdkReadinessManager.sdkStatus.whenReadyFromCache()).toBe(true); let testPassedCount = 0; - await ready.then( - () => { - expect('It should be a promise that will be resolved when the SDK is ready.'); - testPassedCount++; - }, - () => { throw new Error('It should be resolved on ready event, not rejected.'); } - ); + function incTestPassedCount() { testPassedCount++; } + function throwTestFailed() { throw new Error('It should be resolved, not rejected.'); } - // any subsequent call to .ready() must be a resolved promise - await ready.then( - () => { - expect('A subsequent call should be a resolved promise.'); - testPassedCount++; - }, - () => { throw new Error('It should be resolved on ready event, not rejected.'); } - ); + await readyFromCache.then(incTestPassedCount, throwTestFailed); + await ready.then(incTestPassedCount, throwTestFailed); - // control assertion. stubs already reset. - expect(testPassedCount).toBe(2); + // any subsequent call to .whenReady() and .whenReadyFromCache() must be a resolved promise + await sdkReadinessManager.sdkStatus.whenReady().then(incTestPassedCount, throwTestFailed); + await sdkReadinessManager.sdkStatus.whenReadyFromCache().then(incTestPassedCount, throwTestFailed); - const sdkReadinessManagerForTimedout = sdkReadinessManagerFactory(EventEmitterMock, fullSettings); + expect(testPassedCount).toBe(4); + }); - const readyForTimeout = sdkReadinessManagerForTimedout.sdkStatus.ready(); + test('.whenReady() and .whenReadyFromCache() promises reject when SDK_READY_TIMED_OUT is emitted before SDK_READY', async () => { + const sdkReadinessManagerForTimedout = sdkReadinessManagerFactory(EventEmitter, fullSettings); - emitTimeoutEvent(sdkReadinessManagerForTimedout.readinessManager); // make the SDK "timed out" + const readyFromCacheForTimeout = sdkReadinessManagerForTimedout.sdkStatus.whenReadyFromCache(); + const readyForTimeout = sdkReadinessManagerForTimedout.sdkStatus.whenReady(); - await readyForTimeout.then( - () => { throw new Error('It should be a promise that was rejected on SDK_READY_TIMED_OUT, not resolved.'); }, - () => { - expect('It should be a promise that will be rejected when the SDK is timed out.'); - testPassedCount++; - } - ); + emitTimeoutEvent(sdkReadinessManagerForTimedout.readinessManager); // make the SDK timeout - // any subsequent call to .ready() must be a rejected promise - await readyForTimeout.then( - () => { throw new Error('It should be a promise that was rejected on SDK_READY_TIMED_OUT, not resolved.'); }, - () => { - expect('A subsequent call should be a rejected promise.'); - testPassedCount++; - } - ); + let testPassedCount = 0; + function incTestPassedCount() { testPassedCount++; } + function throwTestFailed() { throw new Error('It should rejected, not resolved.'); } + + await readyFromCacheForTimeout.then(throwTestFailed,incTestPassedCount); + await readyForTimeout.then(throwTestFailed,incTestPassedCount); - // make the SDK "ready" + // any subsequent call to .whenReady() and .whenReadyFromCache() must be a rejected promise until the SDK is ready + await sdkReadinessManagerForTimedout.sdkStatus.whenReadyFromCache().then(throwTestFailed,incTestPassedCount); + await sdkReadinessManagerForTimedout.sdkStatus.whenReady().then(throwTestFailed,incTestPassedCount); + + // make the SDK ready emitReadyEvent(sdkReadinessManagerForTimedout.readinessManager); - // once SDK_READY, `.ready()` returns a resolved promise - await ready.then( - () => { - expect('It should be a resolved promise when the SDK is ready, even after an SDK timeout.'); - loggerMock.mockClear(); - testPassedCount++; - expect(testPassedCount).toBe(5); - }, - () => { throw new Error('It should be resolved on ready event, not rejected.'); } - ); + // once SDK_READY, `.whenReady()` returns a resolved promise + await sdkReadinessManagerForTimedout.sdkStatus.whenReady().then(incTestPassedCount, throwTestFailed); + await sdkReadinessManagerForTimedout.sdkStatus.whenReadyFromCache().then(incTestPassedCount, throwTestFailed); + + expect(testPassedCount).toBe(6); + }); + + test('whenReady promise counts as an SDK_READY listener', (done) => { + let sdkReadinessManager = sdkReadinessManagerFactory(EventEmitter, fullSettings); + + emitReadyEvent(sdkReadinessManager.readinessManager); + + expect(loggerMock.warn).toBeCalledWith(CLIENT_NO_LISTENER); // We should get a warning if the SDK get's ready before calling the whenReady method or attaching a listener to the ready event + loggerMock.warn.mockClear(); + + sdkReadinessManager = sdkReadinessManagerFactory(EventEmitter, fullSettings); + sdkReadinessManager.sdkStatus.whenReady().then(() => { + expect('whenReady promise is resolved when the gate emits SDK_READY.'); + done(); + }, () => { + throw new Error('This should not be called as the promise is being resolved.'); + }); + + emitReadyEvent(sdkReadinessManager.readinessManager); + + expect(loggerMock.warn).not.toBeCalled(); // But if we have a listener or call the whenReady method, we get no warnings. }); +}); + +// @TODO: remove in next major +describe('SDK Readiness Manager - Ready promise', () => { + + beforeEach(() => { loggerMock.mockClear(); }); - test('Full blown ready promise count as a callback and resolves on SDK_READY', (done) => { + test('ready promise count as a callback and resolves on SDK_READY', (done) => { const sdkReadinessManager = sdkReadinessManagerFactory(EventEmitterMock, fullSettings); const readyPromise = sdkReadinessManager.sdkStatus.ready(); diff --git a/src/readiness/readinessManager.ts b/src/readiness/readinessManager.ts index c69eedce..319e843d 100644 --- a/src/readiness/readinessManager.ts +++ b/src/readiness/readinessManager.ts @@ -3,7 +3,6 @@ import { ISettings } from '../types'; import SplitIO from '../../types/splitio'; import { SDK_SPLITS_ARRIVED, SDK_SPLITS_CACHE_LOADED, SDK_SEGMENTS_ARRIVED, SDK_READY_TIMED_OUT, SDK_READY_FROM_CACHE, SDK_UPDATE, SDK_READY } from './constants'; import { IReadinessEventEmitter, IReadinessManager, ISegmentsEventEmitter, ISplitsEventEmitter } from './types'; -import { STORAGE_LOCALSTORAGE } from '../utils/constants'; function splitsEventEmitterFactory(EventEmitter: new () => SplitIO.IEventEmitter): ISplitsEventEmitter { const splitsEventEmitter = objectAssign(new EventEmitter(), { @@ -91,7 +90,7 @@ export function readinessManagerFactory( if (!isReady && !isDestroyed) { try { syncLastUpdate(); - gate.emit(SDK_READY_FROM_CACHE); + gate.emit(SDK_READY_FROM_CACHE, isReady); } catch (e) { // throws user callback exceptions in next tick setTimeout(() => { throw e; }, 0); @@ -115,9 +114,9 @@ export function readinessManagerFactory( isReady = true; try { syncLastUpdate(); - if (!isReadyFromCache && settings.storage?.type === STORAGE_LOCALSTORAGE) { + if (!isReadyFromCache) { isReadyFromCache = true; - gate.emit(SDK_READY_FROM_CACHE); + gate.emit(SDK_READY_FROM_CACHE, isReady); } gate.emit(SDK_READY); } catch (e) { diff --git a/src/readiness/sdkReadinessManager.ts b/src/readiness/sdkReadinessManager.ts index ee558d47..64e518b3 100644 --- a/src/readiness/sdkReadinessManager.ts +++ b/src/readiness/sdkReadinessManager.ts @@ -9,6 +9,7 @@ import { ERROR_CLIENT_LISTENER, CLIENT_READY_FROM_CACHE, CLIENT_READY, CLIENT_NO const NEW_LISTENER_EVENT = 'newListener'; const REMOVE_LISTENER_EVENT = 'removeListener'; +const TIMEOUT_ERROR = new Error(SDK_READY_TIMED_OUT); /** * SdkReadinessManager factory, which provides the public status API of SDK clients and manager: ready promise, readiness event emitter and constants (SDK_READY, etc). @@ -38,6 +39,8 @@ export function sdkReadinessManagerFactory( } else if (event === SDK_READY) { readyCbCount++; } + } else if (event === SDK_READY_FROM_CACHE && readinessManager.isReadyFromCache()) { + log.error(ERROR_CLIENT_LISTENER, ['SDK_READY_FROM_CACHE']); } }); @@ -93,6 +96,7 @@ export function sdkReadinessManagerFactory( SDK_READY_TIMED_OUT, }, + // @TODO: remove in next major ready() { if (readinessManager.hasTimedout()) { if (!readinessManager.isReady()) { @@ -104,6 +108,32 @@ export function sdkReadinessManagerFactory( return readyPromise; }, + whenReady() { + return new Promise((resolve, reject) => { + if (readinessManager.isReady()) { + resolve(); + } else if (readinessManager.hasTimedout()) { + reject(TIMEOUT_ERROR); + } else { + readinessManager.gate.once(SDK_READY, resolve); + readinessManager.gate.once(SDK_READY_TIMED_OUT, () => reject(TIMEOUT_ERROR)); + } + }); + }, + + whenReadyFromCache() { + return new Promise((resolve, reject) => { + if (readinessManager.isReadyFromCache()) { + resolve(readinessManager.isReady()); + } else if (readinessManager.hasTimedout()) { + reject(TIMEOUT_ERROR); + } else { + readinessManager.gate.once(SDK_READY_FROM_CACHE, () => resolve(readinessManager.isReady())); + readinessManager.gate.once(SDK_READY_TIMED_OUT, () => reject(TIMEOUT_ERROR)); + } + }); + }, + __getStatus() { return { isReady: readinessManager.isReady(), diff --git a/src/sdkClient/__tests__/clientInputValidation.spec.ts b/src/sdkClient/__tests__/clientInputValidation.spec.ts index f70845f7..452949e8 100644 --- a/src/sdkClient/__tests__/clientInputValidation.spec.ts +++ b/src/sdkClient/__tests__/clientInputValidation.spec.ts @@ -14,7 +14,7 @@ const EVALUATION_RESULT = 'on'; const client: any = createClientMock(EVALUATION_RESULT); const readinessManager: any = { - isReady: () => true, + isReadyFromCache: () => true, isDestroyed: () => false }; diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 0e526f72..2e431dc8 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -51,7 +51,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl return treatment; }; - const evaluation = readinessManager.isReady() || readinessManager.isReadyFromCache() ? + const evaluation = readinessManager.isReadyFromCache() ? evaluateFeature(log, key, featureFlagName, attributes, storage) : isAsync ? // If the SDK is not ready, treatment may be incorrect due to having splits but not segments data, or storage is not connected Promise.resolve(treatmentNotReady) : @@ -80,7 +80,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl return treatments; }; - const evaluations = readinessManager.isReady() || readinessManager.isReadyFromCache() ? + const evaluations = readinessManager.isReadyFromCache() ? evaluateFeatures(log, key, featureFlagNames, attributes, storage) : isAsync ? // If the SDK is not ready, treatment may be incorrect due to having splits but not segments data, or storage is not connected Promise.resolve(treatmentsNotReady(featureFlagNames)) : @@ -109,7 +109,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl return treatments; }; - const evaluations = readinessManager.isReady() || readinessManager.isReadyFromCache() ? + const evaluations = readinessManager.isReadyFromCache() ? evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage, methodName) : isAsync ? Promise.resolve({}) : diff --git a/src/sdkClient/clientInputValidation.ts b/src/sdkClient/clientInputValidation.ts index 40765d41..b67025d7 100644 --- a/src/sdkClient/clientInputValidation.ts +++ b/src/sdkClient/clientInputValidation.ts @@ -9,7 +9,7 @@ import { validateSplits, validateTrafficType, validateIfNotDestroyed, - validateIfOperational, + validateIfReadyFromCache, validateEvaluationOptions } from '../utils/inputValidation'; import { startsWith } from '../utils/lang'; @@ -46,7 +46,7 @@ export function clientInputValidationDecorator true), + isReadyFromCache: jest.fn(() => true), isDestroyed: jest.fn(() => false) }, sdkStatus: jest.fn() @@ -77,7 +78,7 @@ describe('Manager with async cache', () => { const cache = new SplitsCachePluggable(loggerMock, keys, wrapperAdapter(loggerMock, {})); const manager = sdkManagerFactory({ mode: 'consumer_partial', log: loggerMock }, cache, sdkReadinessManagerMock); - expect(await manager.split('some_spplit')).toEqual(null); + expect(await manager.split('some_split')).toEqual(null); expect(await manager.splits()).toEqual([]); expect(await manager.names()).toEqual([]); @@ -98,7 +99,7 @@ describe('Manager with async cache', () => { const manager = sdkManagerFactory({ mode: 'consumer_partial', log: loggerMock }, {}, sdkReadinessManagerMock) as SplitIO.IAsyncManager; function validateManager() { - expect(manager.split('some_spplit')).resolves.toBe(null); + expect(manager.split('some_split')).resolves.toBe(null); expect(manager.splits()).resolves.toEqual([]); expect(manager.names()).resolves.toEqual([]); } diff --git a/src/sdkManager/__tests__/index.syncCache.spec.ts b/src/sdkManager/__tests__/index.syncCache.spec.ts index 391a053c..3437f008 100644 --- a/src/sdkManager/__tests__/index.syncCache.spec.ts +++ b/src/sdkManager/__tests__/index.syncCache.spec.ts @@ -9,6 +9,7 @@ import { loggerMock } from '../../logger/__tests__/sdkLogger.mock'; const sdkReadinessManagerMock = { readinessManager: { isReady: jest.fn(() => true), + isReadyFromCache: jest.fn(() => true), isDestroyed: jest.fn(() => false) }, sdkStatus: jest.fn() @@ -62,7 +63,7 @@ describe('Manager with sync cache (In Memory)', () => { sdkReadinessManagerMock.readinessManager.isDestroyed = () => true; function validateManager() { - expect(manager.split('some_spplit')).toBe(null); + expect(manager.split('some_split')).toBe(null); expect(manager.splits()).toEqual([]); expect(manager.names()).toEqual([]); } diff --git a/src/sdkManager/index.ts b/src/sdkManager/index.ts index d241b82e..5260170c 100644 --- a/src/sdkManager/index.ts +++ b/src/sdkManager/index.ts @@ -1,7 +1,7 @@ import { objectAssign } from '../utils/lang/objectAssign'; import { thenable } from '../utils/promise/thenable'; import { find } from '../utils/lang'; -import { validateSplit, validateSplitExistence, validateIfNotDestroyed, validateIfOperational } from '../utils/inputValidation'; +import { validateSplit, validateSplitExistence, validateIfOperational } from '../utils/inputValidation'; import { ISplitsCacheAsync, ISplitsCacheSync } from '../storages/types'; import { ISdkReadinessManager } from '../readiness/types'; import { ISplit } from '../dtos/types'; @@ -66,7 +66,7 @@ export function sdkManagerFactory { - telemetry.recordTimeUntilReadyFromCache(startTime()); + telemetry.recordTimeUntilReadyFromCache(stopTimer()); }); sdkReadinessManager.incInternalReadyCbCount(); readiness.gate.once(SDK_READY, () => { - telemetry.recordTimeUntilReady(startTime()); + telemetry.recordTimeUntilReady(stopTimer()); // Post config data when the SDK is ready and if the telemetry submitter was started if (submitter.isRunning()) { diff --git a/src/trackers/telemetryTracker.ts b/src/trackers/telemetryTracker.ts index 0312cc94..1a0ebc6e 100644 --- a/src/trackers/telemetryTracker.ts +++ b/src/trackers/telemetryTracker.ts @@ -11,11 +11,11 @@ export function telemetryTrackerFactory( ): ITelemetryTracker { if (telemetryCache && now) { - const startTime = timer(now); + const sessionTimer = timer(now); return { trackEval(method) { - const evalTime = timer(now); + const evalTimer = timer(now); return (label) => { switch (label) { @@ -25,20 +25,20 @@ export function telemetryTrackerFactory( case SDK_NOT_READY: // @ts-ignore ITelemetryCacheAsync doesn't implement the method if (telemetryCache.recordNonReadyUsage) telemetryCache.recordNonReadyUsage(); } - telemetryCache.recordLatency(method, evalTime()); + telemetryCache.recordLatency(method, evalTimer()); }; }, trackHttp(operation) { - const httpTime = timer(now); + const httpTimer = timer(now); return (error) => { - (telemetryCache as ITelemetryCacheSync).recordHttpLatency(operation, httpTime()); + (telemetryCache as ITelemetryCacheSync).recordHttpLatency(operation, httpTimer()); if (error && error.statusCode) (telemetryCache as ITelemetryCacheSync).recordHttpError(operation, error.statusCode); else (telemetryCache as ITelemetryCacheSync).recordSuccessfulSync(operation, Date.now()); }; }, sessionLength() { // @ts-ignore ITelemetryCacheAsync doesn't implement the method - if (telemetryCache.recordSessionLength) telemetryCache.recordSessionLength(startTime()); + if (telemetryCache.recordSessionLength) telemetryCache.recordSessionLength(sessionTimer()); }, streamingEvent(e, d) { if (e === AUTH_REJECTION) { diff --git a/src/utils/inputValidation/__tests__/isOperational.spec.ts b/src/utils/inputValidation/__tests__/isOperational.spec.ts index bbc6afa7..19c1373a 100644 --- a/src/utils/inputValidation/__tests__/isOperational.spec.ts +++ b/src/utils/inputValidation/__tests__/isOperational.spec.ts @@ -1,7 +1,7 @@ -import { CLIENT_NOT_READY, ERROR_CLIENT_DESTROYED } from '../../../logger/constants'; +import { CLIENT_NOT_READY_FROM_CACHE, ERROR_CLIENT_DESTROYED } from '../../../logger/constants'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; -import { validateIfNotDestroyed, validateIfOperational } from '../isOperational'; +import { validateIfNotDestroyed, validateIfReadyFromCache } from '../isOperational'; describe('validateIfNotDestroyed', () => { @@ -28,37 +28,25 @@ describe('validateIfNotDestroyed', () => { }); }); -describe('validateIfOperational', () => { - - test('Should return true and log nothing if the SDK was ready.', () => { - const readinessManagerMock = { isReady: jest.fn(() => true) }; - - // @ts-ignore - expect(validateIfOperational(loggerMock, readinessManagerMock, 'test_method')).toBe(true); // It should return true if SDK was ready. - expect(readinessManagerMock.isReady).toBeCalledTimes(1); // It checks for readiness status using the context. - expect(loggerMock.warn).not.toBeCalled(); // But it should not log any warnings. - expect(loggerMock.error).not.toBeCalled(); // But it should not log any errors. - }); +describe('validateIfReadyFromCache', () => { test('Should return true and log nothing if the SDK was ready from cache.', () => { - const readinessManagerMock = { isReady: jest.fn(() => false), isReadyFromCache: jest.fn(() => true) }; + const readinessManagerMock = { isReadyFromCache: jest.fn(() => true) }; // @ts-ignore - expect(validateIfOperational(loggerMock, readinessManagerMock, 'test_method')).toBe(true); // It should return true if SDK was ready. - expect(readinessManagerMock.isReady).toBeCalledTimes(1); // It checks for SDK_READY status. + expect(validateIfReadyFromCache(loggerMock, readinessManagerMock, 'test_method')).toBe(true); // It should return true if SDK was ready. expect(readinessManagerMock.isReadyFromCache).toBeCalledTimes(1); // It checks for SDK_READY_FROM_CACHE status. expect(loggerMock.warn).not.toBeCalled(); // But it should not log any warnings. expect(loggerMock.error).not.toBeCalled(); // But it should not log any errors. }); - test('Should return false and log a warning if the SDK was not ready.', () => { - const readinessManagerMock = { isReady: jest.fn(() => false), isReadyFromCache: jest.fn(() => false) }; + test('Should return false and log a warning if the SDK was not ready from cache.', () => { + const readinessManagerMock = { isReadyFromCache: jest.fn(() => false) }; // @ts-ignore - expect(validateIfOperational(loggerMock, readinessManagerMock, 'test_method')).toBe(false); // It should return true if SDK was ready. - expect(readinessManagerMock.isReady).toBeCalledTimes(1); // It checks for SDK_READY status. + expect(validateIfReadyFromCache(loggerMock, readinessManagerMock, 'test_method')).toBe(false); // It should return true if SDK was ready. expect(readinessManagerMock.isReadyFromCache).toBeCalledTimes(1); // It checks for SDK_READY_FROM_CACHE status. - expect(loggerMock.warn).toBeCalledWith(CLIENT_NOT_READY, ['test_method', '']); // It should log the expected warning. + expect(loggerMock.warn).toBeCalledWith(CLIENT_NOT_READY_FROM_CACHE, ['test_method', '']); // It should log the expected warning. expect(loggerMock.error).not.toBeCalled(); // But it should not log any errors. }); }); diff --git a/src/utils/inputValidation/index.ts b/src/utils/inputValidation/index.ts index eac9777d..f6e06c5e 100644 --- a/src/utils/inputValidation/index.ts +++ b/src/utils/inputValidation/index.ts @@ -7,7 +7,7 @@ export { validateKey } from './key'; export { validateSplit } from './split'; export { validateSplits } from './splits'; export { validateTrafficType } from './trafficType'; -export { validateIfNotDestroyed, validateIfOperational } from './isOperational'; +export { validateIfNotDestroyed, validateIfReadyFromCache, validateIfOperational } from './isOperational'; export { validateSplitExistence } from './splitExistence'; export { validateTrafficTypeExistence } from './trafficTypeExistence'; export { validateEvaluationOptions } from './eventProperties'; diff --git a/src/utils/inputValidation/isOperational.ts b/src/utils/inputValidation/isOperational.ts index 3d990433..5f122926 100644 --- a/src/utils/inputValidation/isOperational.ts +++ b/src/utils/inputValidation/isOperational.ts @@ -1,4 +1,4 @@ -import { ERROR_CLIENT_DESTROYED, CLIENT_NOT_READY } from '../../logger/constants'; +import { ERROR_CLIENT_DESTROYED, CLIENT_NOT_READY_FROM_CACHE } from '../../logger/constants'; import { ILogger } from '../../logger/types'; import { IReadinessManager } from '../../readiness/types'; @@ -9,9 +9,14 @@ export function validateIfNotDestroyed(log: ILogger, readinessManager: IReadines return false; } -export function validateIfOperational(log: ILogger, readinessManager: IReadinessManager, method: string, featureFlagNameOrNames?: string | string[] | false) { - if (readinessManager.isReady() || readinessManager.isReadyFromCache()) return true; +export function validateIfReadyFromCache(log: ILogger, readinessManager: IReadinessManager, method: string, featureFlagNameOrNames?: string | string[] | false) { + if (readinessManager.isReadyFromCache()) return true; - log.warn(CLIENT_NOT_READY, [method, featureFlagNameOrNames ? ` for feature flag ${featureFlagNameOrNames.toString()}` : '']); + log.warn(CLIENT_NOT_READY_FROM_CACHE, [method, featureFlagNameOrNames ? ` for feature flag ${featureFlagNameOrNames.toString()}` : '']); return false; } + +// Operational means that the SDK is ready to evaluate (not destroyed and ready from cache) +export function validateIfOperational(log: ILogger, readinessManager: IReadinessManager, method: string, featureFlagNameOrNames?: string | string[] | false) { + return validateIfNotDestroyed(log, readinessManager, method) && validateIfReadyFromCache(log, readinessManager, method, featureFlagNameOrNames); +} diff --git a/src/utils/inputValidation/splitExistence.ts b/src/utils/inputValidation/splitExistence.ts index 2f3105f9..60ac3743 100644 --- a/src/utils/inputValidation/splitExistence.ts +++ b/src/utils/inputValidation/splitExistence.ts @@ -5,10 +5,10 @@ import { WARN_NOT_EXISTENT_SPLIT } from '../../logger/constants'; /** * This is defined here and in this format mostly because of the logger and the fact that it's considered a validation at product level. - * But it's not going to run on the input validation layer. In any case, the most compeling reason to use it as we do is to avoid going to Redis and get a split twice. + * But it's not going to run on the input validation layer. In any case, the most compelling reason to use it as we do is to avoid going to Redis and get a split twice. */ export function validateSplitExistence(log: ILogger, readinessManager: IReadinessManager, splitName: string, labelOrSplitObj: any, method: string): boolean { - if (readinessManager.isReady()) { // Only if it's ready we validate this, otherwise it may just be that the SDK is not ready yet. + if (readinessManager.isReady()) { // Only if it's ready (synced with BE) we validate this, otherwise it may just be that the SDK is still syncing if (labelOrSplitObj === SPLIT_NOT_FOUND || labelOrSplitObj == null) { log.warn(WARN_NOT_EXISTENT_SPLIT, [method, splitName]); return false; diff --git a/src/utils/settingsValidation/logger/__tests__/index.spec.ts b/src/utils/settingsValidation/logger/__tests__/index.spec.ts index 9b7e765a..4d667375 100644 --- a/src/utils/settingsValidation/logger/__tests__/index.spec.ts +++ b/src/utils/settingsValidation/logger/__tests__/index.spec.ts @@ -49,19 +49,13 @@ describe('logger validators', () => { }); test.each(testTargets)('returns a logger with the provided log level if `debug` property is true or a string log level', (validateLogger) => { - expect(getLoggerLogLevel(validateLogger({ debug: true }))).toBe('DEBUG'); + expect(getLoggerLogLevel(validateLogger({ debug: true, logger: loggerMock }))).toBe('DEBUG'); expect(getLoggerLogLevel(validateLogger({ debug: 'DEBUG' }))).toBe('DEBUG'); - expect(getLoggerLogLevel(validateLogger({ debug: 'INFO' }))).toBe('INFO'); + expect(getLoggerLogLevel(validateLogger({ debug: 'INFO', logger: loggerMock }))).toBe('INFO'); expect(getLoggerLogLevel(validateLogger({ debug: 'WARN' }))).toBe('WARN'); - expect(getLoggerLogLevel(validateLogger({ debug: 'ERROR' }))).toBe('ERROR'); + expect(getLoggerLogLevel(validateLogger({ debug: 'ERROR', logger: loggerMock }))).toBe('ERROR'); expect(getLoggerLogLevel(validateLogger({ debug: 'NONE' }))).toBe('NONE'); - - // When combined with the `logger` option, any log level other than `NONE` (false) will be set to `DEBUG` (true) - expect(getLoggerLogLevel(validateLogger({ debug: 'DEBUG', logger: loggerMock }))).toBe('DEBUG'); - expect(getLoggerLogLevel(validateLogger({ debug: 'INFO', logger: loggerMock }))).toBe('DEBUG'); - expect(getLoggerLogLevel(validateLogger({ debug: 'WARN', logger: loggerMock }))).toBe('DEBUG'); - expect(getLoggerLogLevel(validateLogger({ debug: 'ERROR', logger: loggerMock }))).toBe('DEBUG'); - expect(getLoggerLogLevel(validateLogger({ debug: 'NONE', logger: loggerMock }))).toBe('NONE'); + expect(getLoggerLogLevel(validateLogger({ debug: false }))).toBe('NONE'); expect(consoleLogSpy).not.toBeCalled(); }); diff --git a/types/splitio.d.ts b/types/splitio.d.ts index ebeba4df..49f70c62 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -93,6 +93,7 @@ interface ISharedSettings { urls?: SplitIO.UrlSettings; /** * Custom logger object. If not provided, the SDK will use the default `console.log` method for all log levels. + * Set together with `debug` option to `true` or a log level string to enable logging. */ logger?: SplitIO.Logger; } @@ -145,8 +146,6 @@ interface IPluggableSharedSettings { * config.debug = ErrorLogger() * ``` * - * When combined with the `logger` option, any log level other than `NONE` (false) will be set to `DEBUG` (true), delegating log level control to the custom logger. - * * @defaultValue `false` */ debug?: boolean | SplitIO.LogLevel | SplitIO.ILogger; @@ -170,8 +169,6 @@ interface INonPluggableSharedSettings { * config.debug = 'WARN' * ``` * - * When combined with the `logger` option, any log level other than `NONE` (false) will be set to `DEBUG` (true), delegating log level control to the custom logger. - * * @defaultValue `false` */ debug?: boolean | SplitIO.LogLevel; @@ -528,19 +525,19 @@ declare namespace SplitIO { */ type EventConsts = { /** - * The ready event. + * The ready event emitted once the SDK is ready to evaluate feature flags with cache synchronized with the backend. */ SDK_READY: 'init::ready'; /** - * The ready event when fired with cached data. + * The ready event emitted once the SDK is ready to evaluate feature flags with cache that could be stale. Use SDK_READY if you want to be sure the cache is in sync with the backend. */ SDK_READY_FROM_CACHE: 'init::cache-ready'; /** - * The timeout event. + * The timeout event emitted after `startup.readyTimeout` seconds if the SDK_READY event was not emitted. */ SDK_READY_TIMED_OUT: 'init::timeout'; /** - * The update event. + * The update event emitted when the SDK cache is updated with new data from the backend. */ SDK_UPDATE: 'state::update'; }; @@ -703,7 +700,7 @@ declare namespace SplitIO { */ Event: EventConsts; /** - * Returns a promise that resolves once the SDK has finished loading (`SDK_READY` event emitted) or rejected if the SDK has timedout (`SDK_READY_TIMED_OUT` event emitted). + * Returns a promise that resolves when the SDK has finished initial synchronization with the backend (`SDK_READY` event emitted), or rejected if the SDK has timedout (`SDK_READY_TIMED_OUT` event emitted). * As it's meant to provide similar flexibility to the event approach, given that the SDK might be eventually ready after a timeout event, the `ready` method will return a resolved promise once the SDK is ready. * * Caveats: the method was designed to avoid an unhandled Promise rejection if the rejection case is not handled, so that `onRejected` handler is optional when using promises. @@ -718,8 +715,26 @@ declare namespace SplitIO { * ``` * * @returns A promise that resolves once the SDK is ready or rejects if the SDK has timedout. + * @deprecated Use `whenReady` instead. */ ready(): Promise; + /** + * Returns a promise that resolves when the SDK has finished initial synchronization with the backend (`SDK_READY` event emitted), or rejected if the SDK has timedout (`SDK_READY_TIMED_OUT` event emitted). + * As it's meant to provide similar flexibility than event listeners, given that the SDK might be ready after a timeout event, the `whenReady` method will return a resolved promise once the SDK is ready. + * You must handle the promise rejection to avoid an unhandled promise rejection error, or set the `startup.readyTimeout` configuration option to 0 to avoid the timeout and thus the rejection. + * + * @returns A promise that resolves once the SDK_READY event is emitted or rejects if the SDK has timedout. + */ + whenReady(): Promise; + /** + * Returns a promise that resolves when the SDK is ready for evaluations using cached data, which might not yet be synchronized with the backend (`SDK_READY_FROM_CACHE` event emitted), or rejected if the SDK has timedout (`SDK_READY_TIMED_OUT` event emitted). + * As it's meant to provide similar flexibility than event listeners, given that the SDK might be ready from cache after a timeout event, the `whenReadyFromCache` method will return a resolved promise once the SDK is ready from cache. + * You must handle the promise rejection to avoid an unhandled promise rejection error, or set the `startup.readyTimeout` configuration option to 0 to avoid the timeout and thus the rejection. + * + * @returns A promise that resolves once the SDK_READY_FROM_CACHE event is emitted or rejects if the SDK has timedout. The promise resolves with a boolean value that + * indicates whether the SDK_READY_FROM_CACHE event was emitted together with the SDK_READY event (i.e., the SDK is ready and synchronized with the backend) or not. + */ + whenReadyFromCache(): Promise; } /** * Common definitions between clients for different environments interface. @@ -1653,7 +1668,7 @@ declare namespace SplitIO { * Wait for the SDK client to be ready before calling this method. * * ```js - * await factory.client().ready(); + * await factory.client().whenReady(); * const rolloutPlan = factory.getRolloutPlan(); * ``` *