Merge pull request #419 from splitio/inlocalstorage_async_validateCache

[Custom InLocalStorage] Refactor storage `validateCache` method to support async operations

Author: EmilianoSanchez

CHANGES.txt

@@ -47,6 +47,15 @@
    - Removed internal ponyfills for `Map` and `Set` global objects, dropping support for IE and other outdated browsers. The SDK now requires the runtime environment to support these features natively or to provide a polyfill.
    - Removed the `sync.localhostMode` configuration option to plug the LocalhostMode module.
 
+1.17.1 (July 25, 2025)
+ - Updated the Redis storage to avoid lazy require of the `ioredis` dependency when the SDK is initialized.
+ - Updated some transitive dependencies for vulnerability fixes.
+ - Bugfix - Enhanced HTTP client module to implement timeouts for failing requests that might otherwise remain pending indefinitely on some Fetch API implementations, pausing the SDK synchronization process.
+ - Bugfix - Properly handle rejected promises when using targeting rules with segment matchers in consumer modes (e.g., Redis and Pluggable storages).
+ - Bugfix - Sanitize the `SplitSDKMachineName` header value to avoid exceptions on HTTP/S requests when it contains non ISO-8859-1 characters (Related to issue https://github.com/splitio/javascript-client/issues/847).
+ - Bugfix - Fixed an issue with the SDK_UPDATE event on server-side, where it was not being emitted if there was an empty segment and the SDK received a feature flag update notification.
+ - Bugfix - Fixed an issue with the server-side polling manager that caused dangling timers when the SDK was destroyed before it was ready.
+
 1.17.0 (September 6, 2024)
  - Added `sync.requestOptions.getHeaderOverrides` configuration option to enhance SDK HTTP request Headers for Authorization Frameworks.
  - Added `isTimedout` and `lastUpdate` properties to IStatusInterface to keep track of the timestamp of the last SDK event, used on React and Redux SDKs.

src/storages/inLocalStorage/__tests__/validateCache.spec.ts

@@ -28,8 +28,8 @@ describe('validateCache', () => {
     localStorage.clear();
   });
 
-  test('if there is no cache, it should return false', () => {
-    expect(validateCache({}, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
+  test('if there is no cache, it should return false', async () => {
+    expect(await validateCache({}, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
 
     expect(logSpy).not.toHaveBeenCalled();
 
@@ -43,11 +43,11 @@ describe('validateCache', () => {
     expect(localStorage.getItem(keys.buildLastClear())).toBeNull();
   });
 
-  test('if there is cache and it must not be cleared, it should return true', () => {
+  test('if there is cache and it must not be cleared, it should return true', async () => {
     localStorage.setItem(keys.buildSplitsTillKey(), '1');
     localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
 
-    expect(validateCache({}, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true);
+    expect(await validateCache({}, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true);
 
     expect(logSpy).not.toHaveBeenCalled();
 
@@ -61,12 +61,12 @@ describe('validateCache', () => {
     expect(localStorage.getItem(keys.buildLastClear())).toBeNull();
   });
 
-  test('if there is cache and it has expired, it should clear cache and return false', () => {
+  test('if there is cache and it has expired, it should clear cache and return false', async () => {
     localStorage.setItem(keys.buildSplitsTillKey(), '1');
     localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
     localStorage.setItem(keys.buildLastUpdatedKey(), Date.now() - 1000 * 60 * 60 * 24 * 2 + ''); // 2 days ago
 
-    expect(validateCache({ expirationDays: 1 }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
+    expect(await validateCache({ expirationDays: 1 }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
 
     expect(logSpy).toHaveBeenCalledWith('storage:localstorage: Cache expired more than 1 days ago. Cleaning up cache');
 
@@ -79,11 +79,11 @@ describe('validateCache', () => {
     expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true);
   });
 
-  test('if there is cache and its hash has changed, it should clear cache and return false', () => {
+  test('if there is cache and its hash has changed, it should clear cache and return false', async () => {
     localStorage.setItem(keys.buildSplitsTillKey(), '1');
     localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
 
-    expect(validateCache({}, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another-sdk-key' } }, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
+    expect(await validateCache({}, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another-sdk-key' } }, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
 
     expect(logSpy).toHaveBeenCalledWith('storage:localstorage: SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache');
 
@@ -96,12 +96,12 @@ describe('validateCache', () => {
     expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true);
   });
 
-  test('if there is cache and clearOnInit is true, it should clear cache and return false', () => {
+  test('if there is cache and clearOnInit is true, it should clear cache and return false', async () => {
     // Older cache version (without last clear)
     localStorage.setItem(keys.buildSplitsTillKey(), '1');
     localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
 
-    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
+    expect(await validateCache({ clearOnInit: true }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
 
     expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache');
 
@@ -117,13 +117,13 @@ describe('validateCache', () => {
     // If cache is cleared, it should not clear again until a day has passed
     logSpy.mockClear();
     localStorage.setItem(keys.buildSplitsTillKey(), '1');
-    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true);
+    expect(await validateCache({ clearOnInit: true }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true);
     expect(logSpy).not.toHaveBeenCalled();
     expect(localStorage.getItem(keys.buildLastClear())).toBe(lastClear); // Last clear should not have changed
 
     // If a day has passed, it should clear again
     localStorage.setItem(keys.buildLastClear(), (Date.now() - 1000 * 60 * 60 * 24 - 1) + '');
-    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
+    expect(await validateCache({ clearOnInit: true }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
     expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache');
     expect(splits.clear).toHaveBeenCalledTimes(2);
     expect(rbSegments.clear).toHaveBeenCalledTimes(2);

src/storages/inLocalStorage/index.ts

@@ -40,6 +40,7 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt
     const rbSegments = new RBSegmentsCacheInLocal(settings, keys);
     const segments = new MySegmentsCacheInLocal(log, keys);
     const largeSegments = new MySegmentsCacheInLocal(log, myLargeSegmentsKeyBuilder(prefix, matchingKey));
+    let validateCachePromise: Promise<boolean> | undefined;
 
     return {
       splits,
@@ -53,10 +54,12 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt
       uniqueKeys: new UniqueKeysCacheInMemoryCS(),
 
       validateCache() {
-        return validateCache(options, settings, keys, splits, rbSegments, segments, largeSegments);
+        return validateCachePromise || (validateCachePromise = validateCache(options, settings, keys, splits, rbSegments, segments, largeSegments));
       },
 
-      destroy() { },
+      destroy() {
+        return Promise.resolve();
+      },
 
       // When using shared instantiation with MEMORY we reuse everything but segments (they are customer per key).
       shared(matchingKey: string) {

src/storages/inLocalStorage/validateCache.ts

@@ -67,27 +67,29 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS
  *
  * @returns `true` if cache is ready to be used, `false` otherwise (cache was cleared or there is no cache)
  */
-export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, rbSegments: RBSegmentsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean {
+export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, rbSegments: RBSegmentsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): Promise<boolean> {
 
-  const currentTimestamp = Date.now();
-  const isThereCache = splits.getChangeNumber() > -1;
+  return Promise.resolve().then(() => {
+    const currentTimestamp = Date.now();
+    const isThereCache = splits.getChangeNumber() > -1;
 
-  if (validateExpiration(options, settings, keys, currentTimestamp, isThereCache)) {
-    splits.clear();
-    rbSegments.clear();
-    segments.clear();
-    largeSegments.clear();
+    if (validateExpiration(options, settings, keys, currentTimestamp, isThereCache)) {
+      splits.clear();
+      rbSegments.clear();
+      segments.clear();
+      largeSegments.clear();
 
-    // Update last clear timestamp
-    try {
-      localStorage.setItem(keys.buildLastClear(), currentTimestamp + '');
-    } catch (e) {
-      settings.log.error(LOG_PREFIX + e);
-    }
+      // Update last clear timestamp
+      try {
+        localStorage.setItem(keys.buildLastClear(), currentTimestamp + '');
+      } catch (e) {
+        settings.log.error(LOG_PREFIX + e);
+      }
 
-    return false;
-  }
+      return false;
+    }
 
-  // Check if ready from cache
-  return isThereCache;
+    // Check if ready from cache
+    return isThereCache;
+  });
 }

src/storages/types.ts

@@ -479,7 +479,7 @@ export interface IStorageSync extends IStorageBase<
   IUniqueKeysCacheSync
 > {
   // Defined in client-side
-  validateCache?: () => boolean, // @TODO support async
+  validateCache?: () => Promise<boolean>,
   largeSegments?: ISegmentsCacheSync,
 }
 

src/sync/__tests__/syncManagerOnline.spec.ts

@@ -43,7 +43,7 @@ const pushManagerMock = {
 // Mocked pushManager
 const pushManagerFactoryMock = jest.fn(() => pushManagerMock);
 
-test('syncManagerOnline should start or not the submitter depending on user consent status', () => {
+test('syncManagerOnline should start or not the submitter depending on user consent status', async () => {
   const settings = { ...fullSettings };
 
   const syncManager = syncManagerOnlineFactory()({
@@ -52,14 +52,14 @@ test('syncManagerOnline should start or not the submitter depending on user cons
   });
   const submitterManager = syncManager.submitterManager!;
 
-  syncManager.start();
+  await syncManager.start();
   expect(submitterManager.start).toBeCalledTimes(1);
   expect(submitterManager.start).lastCalledWith(false); // SubmitterManager should start all submitters, if userConsent is undefined
   syncManager.stop();
   expect(submitterManager.stop).toBeCalledTimes(1);
 
   settings.userConsent = 'UNKNOWN';
-  syncManager.start();
+  await syncManager.start();
   expect(submitterManager.start).toBeCalledTimes(2);
   expect(submitterManager.start).lastCalledWith(true); // SubmitterManager should start only telemetry submitter, if userConsent is unknown
   syncManager.stop();
@@ -69,7 +69,7 @@ test('syncManagerOnline should start or not the submitter depending on user cons
   expect(submitterManager.execute).lastCalledWith(true); // SubmitterManager should flush only telemetry, if userConsent is unknown
 
   settings.userConsent = 'GRANTED';
-  syncManager.start();
+  await syncManager.start();
   expect(submitterManager.start).toBeCalledTimes(3);
   expect(submitterManager.start).lastCalledWith(false); // SubmitterManager should start all submitters, if userConsent is granted
   syncManager.stop();
@@ -79,7 +79,7 @@ test('syncManagerOnline should start or not the submitter depending on user cons
   expect(submitterManager.execute).lastCalledWith(false); // SubmitterManager should flush all submitters, if userConsent is granted
 
   settings.userConsent = 'DECLINED';
-  syncManager.start();
+  await syncManager.start();
   expect(submitterManager.start).toBeCalledTimes(4);
   expect(submitterManager.start).lastCalledWith(true); // SubmitterManager should start only telemetry submitter, if userConsent is declined
   syncManager.stop();
@@ -90,7 +90,7 @@ test('syncManagerOnline should start or not the submitter depending on user cons
 
 });
 
-test('syncManagerOnline should syncAll a single time when sync is disabled', () => {
+test('syncManagerOnline should syncAll a single time when sync is disabled', async () => {
   const settings = { ...fullSettings };
 
   // disable sync
@@ -106,19 +106,19 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', ()
   expect(pushManagerFactoryMock).not.toBeCalled();
 
   // Test pollingManager for Main client
-  syncManager.start();
+  await syncManager.start();
 
   expect(pollingManagerMock.start).not.toBeCalled();
   expect(pollingManagerMock.syncAll).toBeCalledTimes(1);
 
   syncManager.stop();
-  syncManager.start();
+  await syncManager.start();
 
   expect(pollingManagerMock.start).not.toBeCalled();
   expect(pollingManagerMock.syncAll).toBeCalledTimes(1);
 
   syncManager.stop();
-  syncManager.start();
+  await syncManager.start();
 
   expect(pollingManagerMock.start).not.toBeCalled();
   expect(pollingManagerMock.syncAll).toBeCalledTimes(1);
@@ -139,12 +139,12 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', ()
 
   pollingSyncManagerShared.stop();
 
-  syncManager.start();
+  await syncManager.start();
 
   expect(pollingManagerMock.start).not.toBeCalled();
 
   syncManager.stop();
-  syncManager.start();
+  await syncManager.start();
 
   expect(pollingManagerMock.start).not.toBeCalled();
 
@@ -175,7 +175,7 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', ()
   expect(pushManagerFactoryMock).toBeCalled();
 
   // Test pollingManager for Main client
-  testSyncManager.start();
+  await testSyncManager.start();
 
   expect(pushManagerMock.start).toBeCalled();
 

src/sync/offline/syncTasks/fromObjectSyncTask.ts

@@ -59,8 +59,7 @@ export function fromObjectUpdaterFactory(
 
         if (startingUp) {
           startingUp = false;
-          const isCacheLoaded = storage.validateCache ? storage.validateCache() : false;
-          Promise.resolve().then(() => {
+          Promise.resolve(storage.validateCache ? storage.validateCache() : false).then((isCacheLoaded) => {
             // Emits SDK_READY_FROM_CACHE
             if (isCacheLoaded) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED);
             // Emits SDK_READY

src/sync/syncManagerOnline.ts

@@ -89,36 +89,41 @@ export function syncManagerOnlineFactory(
       start() {
         running = true;
 
-        if (startFirstTime) {
-          const isCacheLoaded = storage.validateCache ? storage.validateCache() : false;
-          if (isCacheLoaded) Promise.resolve().then(() => { readiness.splits.emit(SDK_SPLITS_CACHE_LOADED); });
-        }
+        // @TODO once event, impression and telemetry storages support persistence, call when `validateCache` promise is resolved
+        submitterManager.start(!isConsentGranted(settings));
 
-        // start syncing splits and segments
-        if (pollingManager) {
+        return Promise.resolve(storage.validateCache ? storage.validateCache() : false).then((isCacheLoaded) => {
+          if (!running) return;
 
-          // If synchronization is disabled pushManager and pollingManager should not start
-          if (syncEnabled) {
-            if (pushManager) {
-              // Doesn't call `syncAll` when the syncManager is resuming
+          if (startFirstTime) {
+            // Emits SDK_READY_FROM_CACHE
+            if (isCacheLoaded) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED);
+
+          }
+
+          // start syncing splits and segments
+          if (pollingManager) {
+
+            // If synchronization is disabled pushManager and pollingManager should not start
+            if (syncEnabled) {
+              if (pushManager) {
+                // Doesn't call `syncAll` when the syncManager is resuming
+                if (startFirstTime) {
+                  pollingManager.syncAll();
+                }
+                pushManager.start();
+              } else {
+                pollingManager.start();
+              }
+            } else {
               if (startFirstTime) {
                 pollingManager.syncAll();
               }
-              pushManager.start();
-            } else {
-              pollingManager.start();
-            }
-          } else {
-            if (startFirstTime) {
-              pollingManager.syncAll();
             }
           }
-        }
-
-        // start periodic data recording (events, impressions, telemetry).
-        submitterManager.start(!isConsentGranted(settings));
 
-        startFirstTime = false;
+          startFirstTime = false;
+        });
       },
 
       /**

src/utils/settingsValidation/storage/storageCS.ts

@@ -8,7 +8,7 @@ import { IStorageFactoryParams, IStorageSync } from '../../../storages/types';
 
 export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSync {
   const result = InMemoryStorageCSFactory(params);
-  result.validateCache = () => true; // to emit SDK_READY_FROM_CACHE
+  result.validateCache = () => Promise.resolve(true); // to emit SDK_READY_FROM_CACHE
   return result;
 }
 __InLocalStorageMockFactory.type = STORAGE_MEMORY;