chore: accounting for localstorage race conditions

Author: joker23

packages/sdk/electron/__tests__/platform/ElectronStorage.test.ts

@@ -94,14 +94,14 @@ it('handles error when clearing values', async () => {
   const storage = new ElectronStorage(logger);
   await storage.clear('key1');
 
-  expect(await storage.get('key1')).toEqual('value1');
+  // Cache is updated synchronously — the in-memory state reflects the clear
+  // even if the disk write failed. The data will be flushed on the next write.
+  expect(await storage.get('key1')).toBeNull();
 
   expect(logger.debug).not.toHaveBeenCalled();
   expect(logger.info).not.toHaveBeenCalled();
   expect(logger.warn).not.toHaveBeenCalled();
-  expect(logger.error).toHaveBeenCalledWith(
-    'Error clearing key from storage: key1, reason: write failed',
-  );
+  expect(logger.error).toHaveBeenCalledWith('Error flushing storage to disk: write failed');
 });
 
 it('handles failed initialization when getting values', async () => {
@@ -225,14 +225,14 @@ it('handles error when setting values', async () => {
   const storage = new ElectronStorage(logger);
   await storage.set('key3', 'value3');
 
-  expect(await storage.get('key3')).toBeNull();
+  // Cache is updated synchronously — the in-memory state reflects the set
+  // even if the disk write failed. The data will be flushed on the next write.
+  expect(await storage.get('key3')).toEqual('value3');
 
   expect(logger.debug).not.toHaveBeenCalled();
   expect(logger.info).not.toHaveBeenCalled();
   expect(logger.warn).not.toHaveBeenCalled();
-  expect(logger.error).toHaveBeenCalledWith(
-    'Error setting key in storage: key3, reason: write failed',
-  );
+  expect(logger.error).toHaveBeenCalledWith('Error flushing storage to disk: write failed');
 });
 
 it('getElectronStorage returns the same instance on subsequent calls', () => {

packages/sdk/electron/src/platform/ElectronStorage.ts

@@ -9,6 +9,8 @@ export default class ElectronStorage implements Storage {
   private readonly _tempFile: string;
   private readonly _initialized: Promise<boolean>;
   private _cache: Map<string, string>;
+  private _flushPending: boolean = false;
+  private _flushQueue: Promise<void> = Promise.resolve();
 
   constructor(private readonly _logger?: LDLogger) {
     this._storageFile = path.join(electron.app.getPath('userData'), 'ldcache');
@@ -58,7 +60,7 @@ export default class ElectronStorage implements Storage {
       } catch (cleanupError) {
         this._logError('Error cleaning up temporary file', cleanupError);
       }
-      throw error; // Re-throw the original error
+      throw error;
     }
   }
 
@@ -78,11 +80,8 @@ export default class ElectronStorage implements Storage {
     try {
       await this._throwIfNotInitialized();
       if (this._cache.has(key)) {
-        const cacheCopy = new Map(this._cache);
-        cacheCopy.delete(key);
-        await this._atomicWriteToFile(cacheCopy);
-        // Only update cache if write was successful
-        this._cache = cacheCopy;
+        this._cache.delete(key);
+        await this._scheduleFlush();
       }
     } catch (error) {
       this._logError(`Error clearing key from storage: ${key}, reason`, error);
@@ -103,15 +102,28 @@ export default class ElectronStorage implements Storage {
   async set(key: string, value: string): Promise<void> {
     try {
       await this._throwIfNotInitialized();
-      const cacheCopy = new Map(this._cache);
-      cacheCopy.set(key, value);
-      await this._atomicWriteToFile(cacheCopy);
-      // Only update cache if write was successful
-      this._cache = cacheCopy;
+      this._cache.set(key, value);
+      await this._scheduleFlush();
     } catch (error) {
       this._logError(`Error setting key in storage: ${key}, reason`, error);
     }
   }
+
+  private _scheduleFlush(): Promise<void> {
+    if (this._flushPending) {
+      return this._flushQueue;
+    }
+    this._flushPending = true;
+    this._flushQueue = this._flushQueue
+      .then(async () => {
+        this._flushPending = false;
+        await this._atomicWriteToFile(new Map(this._cache));
+      })
+      .catch((error) => {
+        this._logError('Error flushing storage to disk', error);
+      });
+    return this._flushQueue;
+  }
 }
 
 // Storage should be a singleton to support multiple instances of the SDK. This should have