Protect setContentsJsonString - if localstorage is full, delete all c… (#246)

leoromanovsky · web-flow · commit 6cd411a27437 · 2025-08-20T13:01:44.000-06:00
* Protect setContentsJsonString - if localstorage is full, delete all configuration keys and make a 2nd attempt to write. (FFL-718)

* add spec

* storage re-throws if fails a 2nd time

* correctly mock test to throw on setEntries

* clear in try

* key?

* use applicationLogger

* any
diff --git a/docs/js-client-sdk.iclientconfigsync.configfetchedat.md b/docs/js-client-sdk.iclientconfigsync.configfetchedat.md
@@ -0,0 +1,11 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [@eppo/js-client-sdk](./js-client-sdk.md) &gt; [IClientConfigSync](./js-client-sdk.iclientconfigsync.md) &gt; [configFetchedAt](./js-client-sdk.iclientconfigsync.configfetchedat.md)
+
+## IClientConfigSync.configFetchedAt property
+
+**Signature:**
+
+```typescript
+configFetchedAt?: string;
+```
diff --git a/docs/js-client-sdk.iclientconfigsync.configpublishedat.md b/docs/js-client-sdk.iclientconfigsync.configpublishedat.md
@@ -0,0 +1,11 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [@eppo/js-client-sdk](./js-client-sdk.md) &gt; [IClientConfigSync](./js-client-sdk.iclientconfigsync.md) &gt; [configPublishedAt](./js-client-sdk.iclientconfigsync.configpublishedat.md)
+
+## IClientConfigSync.configPublishedAt property
+
+**Signature:**
+
+```typescript
+configPublishedAt?: string;
+```
diff --git a/docs/js-client-sdk.iclientconfigsync.md b/docs/js-client-sdk.iclientconfigsync.md
@@ -72,6 +72,44 @@ IBanditLogger
 _(Optional)_
 
 
+</td></tr>
+<tr><td>
+
+[configFetchedAt?](./js-client-sdk.iclientconfigsync.configfetchedat.md)
+
+
+</td><td>
+
+
+</td><td>
+
+string
+
+
+</td><td>
+
+_(Optional)_
+
+
+</td></tr>
+<tr><td>
+
+[configPublishedAt?](./js-client-sdk.iclientconfigsync.configpublishedat.md)
+
+
+</td><td>
+
+
+</td><td>
+
+string
+
+
+</td><td>
+
+_(Optional)_
+
+
 </td></tr>
 <tr><td>
 
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eppo/js-client-sdk",
-  "version": "3.16.1",
+  "version": "3.16.2",
   "description": "Eppo SDK for client-side JavaScript applications",
   "main": "dist/index.js",
   "files": [
diff --git a/src/isolatable-hybrid.store.spec.ts b/src/isolatable-hybrid.store.spec.ts
@@ -2,6 +2,7 @@ import {
   IsolatableHybridConfigurationStore,
   ServingStoreUpdateStrategy,
 } from './isolatable-hybrid.store';
+import { StorageFullUnableToWrite } from './string-valued.store';
 
 describe('IsolatableHybridConfigurationStore', () => {
   const syncStoreMock = {
@@ -130,4 +131,49 @@ describe('IsolatableHybridConfigurationStore', () => {
       });
     },
   );
+
+  describe('StorageFullUnableToWrite exception handling', () => {
+    const store = new IsolatableHybridConfigurationStore(syncStoreMock, asyncStoreMock, 'always');
+
+    beforeEach(() => {
+      jest.resetAllMocks();
+    });
+
+    it('should continue operating even when async store fails with StorageFullUnableToWrite', async () => {
+      const entries = { key1: 'value1' };
+      asyncStoreMock.isInitialized.mockReturnValue(true);
+      asyncStoreMock.isExpired.mockReturnValue(true);
+      asyncStoreMock.setEntries.mockRejectedValue(new StorageFullUnableToWrite());
+      syncStoreMock.setEntries.mockResolvedValue(undefined);
+      syncStoreMock.getKeys.mockReturnValue([]);
+
+      // Should not throw - be resilient to async store failures
+      await expect(store.setEntries(entries)).resolves.not.toThrow();
+
+      // Sync store should still be updated for resilience
+      expect(syncStoreMock.setEntries).toHaveBeenCalledWith(entries);
+    });
+
+    it('should handle StorageFullUnableToWrite during initialization', async () => {
+      asyncStoreMock.isInitialized.mockReturnValue(false);
+      asyncStoreMock.setEntries.mockRejectedValue(new StorageFullUnableToWrite());
+
+      await expect(store.init()).resolves.not.toThrow();
+    });
+
+    it('should handle async store failures gracefully', async () => {
+      const entries = { key1: 'value1' };
+      asyncStoreMock.isInitialized.mockReturnValue(true);
+      asyncStoreMock.isExpired.mockReturnValue(true);
+      asyncStoreMock.setEntries.mockRejectedValue(new StorageFullUnableToWrite());
+      syncStoreMock.setEntries.mockResolvedValue(undefined);
+      syncStoreMock.getKeys.mockReturnValue([]);
+
+      // Should not throw even if async store fails
+      await expect(store.setEntries(entries)).resolves.not.toThrow();
+
+      expect(asyncStoreMock.setEntries).toHaveBeenCalledWith(entries);
+      expect(syncStoreMock.setEntries).toHaveBeenCalledWith(entries);
+    });
+  });
 });
diff --git a/src/isolatable-hybrid.store.ts b/src/isolatable-hybrid.store.ts
@@ -1,4 +1,9 @@
-import { HybridConfigurationStore, IAsyncStore, ISyncStore } from '@eppo/js-client-sdk-common';
+import {
+  applicationLogger,
+  HybridConfigurationStore,
+  IAsyncStore,
+  ISyncStore,
+} from '@eppo/js-client-sdk-common';
 
 export type ServingStoreUpdateStrategy = 'always' | 'expired' | 'empty';
 
@@ -21,8 +26,12 @@ export class IsolatableHybridConfigurationStore<T> extends HybridConfigurationSt
   /** @Override */
   public async setEntries(entries: Record<string, T>): Promise<boolean> {
     if (this.persistentStore) {
-      // always update persistent store
-      await this.persistentStore.setEntries(entries);
+      try {
+        // always update persistent store
+        await this.persistentStore.setEntries(entries);
+      } catch (e) {
+        applicationLogger.warn(`Failed to setEntries on persistent store: ${e}`);
+      }
     }
 
     const persistentStoreIsExpired =
diff --git a/src/local-storage-engine.spec.ts b/src/local-storage-engine.spec.ts
@@ -0,0 +1,158 @@
+import { LocalStorageEngine } from './local-storage-engine';
+import { StorageFullUnableToWrite, LocalStorageUnknownFailure } from './string-valued.store';
+
+describe('LocalStorageEngine', () => {
+  let mockLocalStorage: Storage;
+  let engine: LocalStorageEngine;
+
+  beforeEach(() => {
+    mockLocalStorage = {
+      get length() {
+        return this._length || 0;
+      },
+      _length: 0,
+      clear: jest.fn(),
+      getItem: jest.fn(),
+      key: jest.fn(),
+      removeItem: jest.fn(),
+      setItem: jest.fn(),
+    } as Storage & { _length: number };
+    engine = new LocalStorageEngine(mockLocalStorage, 'test');
+  });
+
+  describe('setContentsJsonString', () => {
+    it('should set item successfully when no error occurs', async () => {
+      await engine.setContentsJsonString('test-config');
+
+      expect(mockLocalStorage.setItem).toHaveBeenCalledWith(
+        'eppo-configuration-test',
+        'test-config',
+      );
+    });
+
+    it('should clear eppo-configuration keys and retry on quota exceeded error', async () => {
+      const quotaError = new DOMException('Quota exceeded', 'QuotaExceededError');
+      Object.defineProperty(quotaError, 'code', { value: DOMException.QUOTA_EXCEEDED_ERR });
+
+      // Mock localStorage to have some eppo-configuration keys
+      (mockLocalStorage as Storage & { _length: number })._length = 3;
+      (mockLocalStorage.key as jest.Mock)
+        .mockReturnValueOnce('eppo-configuration-old')
+        .mockReturnValueOnce('other-key')
+        .mockReturnValueOnce('eppo-configuration-meta-old');
+
+      // First call fails with quota error, second call succeeds
+      (mockLocalStorage.setItem as jest.Mock)
+        .mockImplementationOnce(() => {
+          throw quotaError;
+        })
+        .mockImplementationOnce(jest.fn()); // Success on retry
+
+      await engine.setContentsJsonString('test-config');
+
+      // Verify keys were collected and removed
+      expect(mockLocalStorage.removeItem).toHaveBeenCalledWith('eppo-configuration-old');
+      expect(mockLocalStorage.removeItem).toHaveBeenCalledWith('eppo-configuration-meta-old');
+      expect(mockLocalStorage.removeItem).not.toHaveBeenCalledWith('other-key');
+
+      // Verify setItem was called twice (original + retry)
+      expect(mockLocalStorage.setItem).toHaveBeenCalledTimes(2);
+      expect(mockLocalStorage.setItem).toHaveBeenLastCalledWith(
+        'eppo-configuration-test',
+        'test-config',
+      );
+    });
+
+    it('should throw StorageFullUnableToWrite when retry fails', async () => {
+      const quotaError = new DOMException('Quota exceeded', 'QuotaExceededError');
+      Object.defineProperty(quotaError, 'code', { value: DOMException.QUOTA_EXCEEDED_ERR });
+
+      (mockLocalStorage as Storage & { _length: number })._length = 1;
+      (mockLocalStorage.key as jest.Mock).mockReturnValue('eppo-configuration-old');
+
+      // Both calls fail with quota error
+      (mockLocalStorage.setItem as jest.Mock).mockImplementation(() => {
+        throw quotaError;
+      });
+
+      // Should throw StorageFullUnableToWrite
+      await expect(engine.setContentsJsonString('test-config')).rejects.toThrow(
+        StorageFullUnableToWrite,
+      );
+
+      expect(mockLocalStorage.removeItem).toHaveBeenCalledWith('eppo-configuration-old');
+      expect(mockLocalStorage.setItem).toHaveBeenCalledTimes(2);
+    });
+
+    it('should handle quota exceeded error by name', async () => {
+      const quotaError = new DOMException('Quota exceeded');
+      Object.defineProperty(quotaError, 'name', { value: 'QuotaExceededError' });
+
+      (mockLocalStorage as Storage & { _length: number })._length = 1;
+      (mockLocalStorage.key as jest.Mock).mockReturnValue('eppo-configuration-test-key');
+
+      (mockLocalStorage.setItem as jest.Mock)
+        .mockImplementationOnce(() => {
+          throw quotaError;
+        })
+        .mockImplementationOnce(jest.fn()); // Success on retry
+
+      await engine.setContentsJsonString('test-config');
+
+      expect(mockLocalStorage.removeItem).toHaveBeenCalledWith('eppo-configuration-test-key');
+      expect(mockLocalStorage.setItem).toHaveBeenCalledTimes(2);
+    });
+
+    it('should throw LocalStorageUnknownFailure for non-quota errors', async () => {
+      const securityError = new DOMException('Security error', 'SecurityError');
+
+      (mockLocalStorage.setItem as jest.Mock).mockImplementation(() => {
+        throw securityError;
+      });
+
+      // Should throw LocalStorageUnknownFailure for non-quota errors with original error preserved
+      const error = await engine.setContentsJsonString('test-config').catch((e) => e);
+      expect(error).toBeInstanceOf(LocalStorageUnknownFailure);
+      expect(error.message).toContain('Security error');
+      expect(error.originalError).toBeTruthy();
+      expect(error.originalError.message).toBe('Security error');
+
+      expect(mockLocalStorage.removeItem).not.toHaveBeenCalled();
+      expect(mockLocalStorage.setItem).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('clearEppoConfigurationKeys', () => {
+    it('should only remove keys starting with eppo-configuration', async () => {
+      const quotaError = new DOMException('Quota exceeded', 'QuotaExceededError');
+      Object.defineProperty(quotaError, 'code', { value: DOMException.QUOTA_EXCEEDED_ERR });
+
+      (mockLocalStorage as Storage & { _length: number })._length = 5;
+      (mockLocalStorage.key as jest.Mock)
+        .mockReturnValueOnce('eppo-configuration')
+        .mockReturnValueOnce('eppo-configuration-meta')
+        .mockReturnValueOnce('eppo-overrides')
+        .mockReturnValueOnce('other-app-data')
+        .mockReturnValueOnce('eppo-configuration-custom');
+
+      (mockLocalStorage.setItem as jest.Mock)
+        .mockImplementationOnce(() => {
+          throw quotaError;
+        })
+        .mockImplementationOnce(jest.fn()); // Success on retry
+
+      await engine.setContentsJsonString('test-config');
+
+      // Should remove eppo-configuration keys
+      expect(mockLocalStorage.removeItem).toHaveBeenCalledWith('eppo-configuration');
+      expect(mockLocalStorage.removeItem).toHaveBeenCalledWith('eppo-configuration-meta');
+      expect(mockLocalStorage.removeItem).toHaveBeenCalledWith('eppo-configuration-custom');
+
+      // Should not remove other keys
+      expect(mockLocalStorage.removeItem).not.toHaveBeenCalledWith('eppo-overrides');
+      expect(mockLocalStorage.removeItem).not.toHaveBeenCalledWith('other-app-data');
+
+      expect(mockLocalStorage.removeItem).toHaveBeenCalledTimes(3);
+    });
+  });
+});
diff --git a/src/local-storage-engine.ts b/src/local-storage-engine.ts
@@ -1,5 +1,9 @@
 import { CONFIGURATION_KEY, META_KEY } from './storage-key-constants';
-import { IStringStorageEngine } from './string-valued.store';
+import {
+  IStringStorageEngine,
+  StorageFullUnableToWrite,
+  LocalStorageUnknownFailure,
+} from './string-valued.store';
 
 /**
  * Local storage implementation of a string-valued store for storing a configuration and its metadata.
@@ -28,11 +32,66 @@ export class LocalStorageEngine implements IStringStorageEngine {
     return this.localStorage.getItem(this.metaKey);
   };
 
+  /**
+   * @throws StorageFullUnableToWrite
+   * @throws LocalStorageUnknownFailure
+   */
   public setContentsJsonString = async (configurationJsonString: string): Promise<void> => {
-    this.localStorage.setItem(this.contentsKey, configurationJsonString);
+    this.safeWrite(this.contentsKey, configurationJsonString);
   };
 
+  /**
+   * @throws StorageFullUnableToWrite
+   * @throws LocalStorageUnknownFailure
+   */
   public setMetaJsonString = async (metaJsonString: string): Promise<void> => {
-    this.localStorage.setItem(this.metaKey, metaJsonString);
+    this.safeWrite(this.metaKey, metaJsonString);
   };
+
+  /**
+   * @throws StorageFullUnableToWrite
+   * @throws LocalStorageUnknownFailure
+   */
+  private safeWrite(key: string, value: string): void {
+    try {
+      this.localStorage.setItem(key, value);
+    } catch (error) {
+      if (error instanceof DOMException) {
+        // Check for quota exceeded error
+        if (error.code === DOMException.QUOTA_EXCEEDED_ERR || error.name === 'QuotaExceededError') {
+          try {
+            this.clear();
+            // Retry setting the item after clearing
+            this.localStorage.setItem(key, value);
+            return;
+          } catch {
+            throw new StorageFullUnableToWrite();
+          }
+        }
+      }
+      // For any other error, wrap it in our custom exception
+      const errorMessage = error instanceof Error ? error.message : String(error);
+      throw new LocalStorageUnknownFailure(
+        `Failed to write to localStorage: ${errorMessage}`,
+        error instanceof Error ? error : (error as Error),
+      );
+    }
+  }
+
+  public clear(): void {
+    const keysToDelete: string[] = [];
+
+    // Collect all keys that start with 'eppo-configuration'
+    for (let i = 0; i < this.localStorage.length; i++) {
+      const key = this.localStorage.key(i);
+      if (key?.startsWith(CONFIGURATION_KEY)) {
+        keysToDelete.push(key);
+      }
+    }
+
+    // Delete collected keys
+    keysToDelete.forEach((key) => {
+      this.localStorage.removeItem(key);
+    });
+  }
 }
diff --git a/src/local-storage.store.spec.ts b/src/local-storage.store.spec.ts
diff --git a/src/string-valued.store.ts b/src/string-valued.store.ts

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@eppo/js-client-sdk",`
`3`		`- "version": "3.16.1",`
	`3`	`+ "version": "3.16.2",`
`4`	`4`	`"description": "Eppo SDK for client-side JavaScript applications",`
`5`	`5`	`"main": "dist/index.js",`
`6`	`6`	`"files": [`