refactor: simplify SDK readiness checks

EmilianoSanchez · EmilianoSanchez · commit 8b6a8d552e83 · 2025-10-22T12:21:23.000-03:00
diff --git 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/messages/warn.ts b/src/logger/messages/warn.ts
@@ -14,7 +14,7 @@ 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_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 Readiness detected. Incorrect control treatments could have been logged if you called getTreatment/s while the SDK was not yet ready.'],
   // input validation
   [c.WARN_SETTING_NULL, '%s: Property "%s" is of invalid type. Setting value to null.'],
diff --git 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
@@ -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
@@ -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<TClient extends SplitIO.IClient |
     const isNotDestroyed = validateIfNotDestroyed(log, readinessManager, methodName);
     const options = validateEvaluationOptions(log, maybeOptions, methodName);
 
-    validateIfOperational(log, readinessManager, methodName, nameOrNames);
+    validateIfReadyFromCache(log, readinessManager, methodName, nameOrNames);
 
     const valid = isNotDestroyed && key && nameOrNames && attributes !== false;
 
diff --git a/src/sdkManager/__tests__/index.asyncCache.spec.ts b/src/sdkManager/__tests__/index.asyncCache.spec.ts
@@ -15,6 +15,7 @@ import SplitIO from '../../../types/splitio';
 const sdkReadinessManagerMock = {
   readinessManager: {
     isReady: jest.fn(() => 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
@@ -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
@@ -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<TSplitCache extends ISplitsCacheSync | ISplits
        */
       split(featureFlagName: string) {
         const splitName = validateSplit(log, featureFlagName, SPLIT_FN_LABEL);
-        if (!validateIfNotDestroyed(log, readinessManager, SPLIT_FN_LABEL) || !validateIfOperational(log, readinessManager, SPLIT_FN_LABEL) || !splitName) {
+        if (!validateIfOperational(log, readinessManager, SPLIT_FN_LABEL) || !splitName) {
           return isAsync ? Promise.resolve(null) : null;
         }
 
@@ -87,7 +87,7 @@ export function sdkManagerFactory<TSplitCache extends ISplitsCacheSync | ISplits
        * Get the feature flag objects present on the factory storage
        */
       splits() {
-        if (!validateIfNotDestroyed(log, readinessManager, SPLITS_FN_LABEL) || !validateIfOperational(log, readinessManager, SPLITS_FN_LABEL)) {
+        if (!validateIfOperational(log, readinessManager, SPLITS_FN_LABEL)) {
           return isAsync ? Promise.resolve([]) : [];
         }
         const currentSplits = splits.getAll();
@@ -100,7 +100,7 @@ export function sdkManagerFactory<TSplitCache extends ISplitsCacheSync | ISplits
        * Get the feature flag names present on the factory storage
        */
       names() {
-        if (!validateIfNotDestroyed(log, readinessManager, NAMES_FN_LABEL) || !validateIfOperational(log, readinessManager, NAMES_FN_LABEL)) {
+        if (!validateIfOperational(log, readinessManager, NAMES_FN_LABEL)) {
           return isAsync ? Promise.resolve([]) : [];
         }
         const splitNames = splits.getSplitNames();
diff --git 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
@@ -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
@@ -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
@@ -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;
