fix: circuit breaker persistent cache with safety fixes (#281)

* fix: persist circuit breaker cache to IndexedDB across page reloads

On page reload, all 28+ circuit breaker in-memory caches are lost,
triggering 20-30 simultaneous POST requests to Vercel edge functions.

Wire the existing persistent-cache.ts (IndexedDB + localStorage +
Tauri fallback) into CircuitBreaker so every breaker automatically:

- Hydrates from IndexedDB on first execute() call (~1-5ms read)
- Writes to IndexedDB fire-and-forget on every recordSuccess()
- Falls back to stale persistent data on network failure
- Auto-disables for breakers with cacheTtlMs=0 (live pricing)

Zero consumer code changes -- all 28+ breaker call sites untouched.
Reloads within the cache TTL (default 10min) serve instantly from
IndexedDB with zero network calls.

Also adds deletePersistentCache() to persistent-cache.ts for clean
cache invalidation via clearCache().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test: add Playwright e2e tests for circuit breaker persistent cache

7 tests covering: IndexedDB persistence on success, hydration on new
instance, TTL expiry forcing fresh fetch, 24h stale ceiling rejection,
clearCache cleanup, cacheTtlMs=0 auto-disable, and network failure
fallback to stale persistent data.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: desktop cache deletion + clearCache race condition

P1: deletePersistentCache sent empty string to write_cache_entry,
which fails Rust's serde_json::from_str (not valid JSON). Add
dedicated delete_cache_entry Tauri command that removes the key
from the in-memory HashMap and flushes to disk.

P2: clearCache() set persistentLoaded=false, allowing a concurrent
execute() to re-hydrate stale data from IndexedDB before the async
delete completed. Remove the reset — after explicit clear there is
no reason to re-hydrate from persistent storage.

* fix: default persistCache to false, fix falsy data guard

P1b: 6 breakers store Date objects (weather, aviation, ACLED,
military-flights, military-vessels, GDACS) which become strings
after JSON round-trip. Callers like MapPopup.getTimeUntil() call
date.getTime() on hydrated strings → TypeError. Change default
to false (opt-in) so persistence requires explicit confirmation
that the payload is JSON-safe.

P2: `if (!entry?.data) return` drops valid falsy payloads (0,
false, empty string). Use explicit null/undefined check instead.

* fix: address blocking review issues on circuit breaker persistence

- clearCache() nulls persistentLoadPromise to orphan in-flight hydration
- delete_cache_entry defers disk flush to exit handler (avoids 14MB sync write)
- hydratePersistentCache checks TTL before setting lastDataState to 'cached'
- deletePersistentCache resets cacheDbPromise on IDB error + logs warning
- hydration catch logs warning instead of silently swallowing
- deletePersistentCache respects isStorageQuotaExceeded() for localStorage

---------

Co-authored-by: Elias El Khoury <efk@anghami.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

e2e/circuit-breaker-persistence.spec.ts

@@ -0,0 +1,359 @@
+import { expect, test } from '@playwright/test';
+
+/**
+ * Circuit Breaker persistent cache tests.
+ *
+ * Each test creates a CircuitBreaker directly (avoiding the global registry),
+ * exercises the persistence path via IndexedDB, and cleans up after itself.
+ */
+test.describe('circuit breaker persistent cache', () => {
+
+  test('recordSuccess persists data to IndexedDB', async ({ page }) => {
+    await page.goto('/tests/runtime-harness.html');
+
+    const result = await page.evaluate(async () => {
+      const { CircuitBreaker } = await import('/src/utils/circuit-breaker.ts');
+      const { getPersistentCache, deletePersistentCache } = await import('/src/services/persistent-cache.ts');
+
+      const name = `test-persist-${Date.now()}`;
+      const breaker = new CircuitBreaker<{ value: number }>({
+        name,
+        cacheTtlMs: 60_000,
+        persistCache: true,
+      });
+
+      const payload = { value: 42 };
+      try {
+        const result = await breaker.execute(async () => payload, { value: 0 });
+
+        // Give fire-and-forget write time to complete
+        await new Promise((r) => setTimeout(r, 200));
+
+        const entry = await getPersistentCache<{ value: number }>(`breaker:${name}`);
+
+        return {
+          executeResult: result.value,
+          persistedData: entry?.data?.value ?? null,
+          persistedAge: entry ? Date.now() - entry.updatedAt : null,
+        };
+      } finally {
+        await deletePersistentCache(`breaker:${name}`);
+      }
+    });
+
+    expect(result.executeResult).toBe(42);
+    expect(result.persistedData).toBe(42);
+    expect(result.persistedAge).not.toBeNull();
+    expect(result.persistedAge as number).toBeLessThan(5000);
+  });
+
+  test('new breaker instance hydrates from IndexedDB on first execute', async ({ page }) => {
+    await page.goto('/tests/runtime-harness.html');
+
+    const result = await page.evaluate(async () => {
+      const { CircuitBreaker } = await import('/src/utils/circuit-breaker.ts');
+      const { setPersistentCache, deletePersistentCache } = await import('/src/services/persistent-cache.ts');
+
+      const name = `test-hydrate-${Date.now()}`;
+      const cacheKey = `breaker:${name}`;
+
+      // Pre-seed IndexedDB with a recent entry (simulating a previous session)
+      await setPersistentCache(cacheKey, { value: 99 });
+
+      let fetchCalled = false;
+      const breaker = new CircuitBreaker<{ value: number }>({
+        name,
+        cacheTtlMs: 60_000,
+        persistCache: true,
+      });
+
+      try {
+        const result = await breaker.execute(async () => {
+          fetchCalled = true;
+          return { value: -1 };
+        }, { value: 0 });
+
+        return {
+          result: result.value,
+          fetchCalled,
+          dataState: breaker.getDataState().mode,
+        };
+      } finally {
+        await deletePersistentCache(cacheKey);
+      }
+    });
+
+    // Should serve hydrated data, NOT call fetch
+    expect(result.result).toBe(99);
+    expect(result.fetchCalled).toBe(false);
+    expect(result.dataState).toBe('cached');
+  });
+
+  test('expired persistent entry triggers fresh fetch (TTL respected)', async ({ page }) => {
+    await page.goto('/tests/runtime-harness.html');
+
+    const result = await page.evaluate(async () => {
+      const { CircuitBreaker } = await import('/src/utils/circuit-breaker.ts');
+      const { deletePersistentCache } = await import('/src/services/persistent-cache.ts');
+
+      const name = `test-ttl-${Date.now()}`;
+      const cacheKey = `breaker:${name}`;
+
+      // Pre-seed IndexedDB with an entry that's older than the TTL.
+      // We do this by writing directly to IndexedDB with an old timestamp.
+      const DB_NAME = 'worldmonitor_persistent_cache';
+      const STORE = 'entries';
+
+      await new Promise<void>((resolve, reject) => {
+        const request = indexedDB.open(DB_NAME, 1);
+        request.onupgradeneeded = () => {
+          const db = request.result;
+          if (!db.objectStoreNames.contains(STORE)) {
+            db.createObjectStore(STORE, { keyPath: 'key' });
+          }
+        };
+        request.onsuccess = () => {
+          const db = request.result;
+          const tx = db.transaction(STORE, 'readwrite');
+          tx.objectStore(STORE).put({
+            key: cacheKey,
+            data: { value: 111 },
+            updatedAt: Date.now() - 120_000, // 2 minutes ago
+          });
+          tx.oncomplete = () => resolve();
+          tx.onerror = () => reject(tx.error);
+        };
+        request.onerror = () => reject(request.error);
+      });
+
+      let fetchCalled = false;
+      const breaker = new CircuitBreaker<{ value: number }>({
+        name,
+        cacheTtlMs: 5_000, // 5 second TTL — the persistent entry (2min old) is expired
+        persistCache: true,
+      });
+
+      try {
+        const result = await breaker.execute(async () => {
+          fetchCalled = true;
+          return { value: 222 };
+        }, { value: 0 });
+
+        // Wait for fire-and-forget write
+        await new Promise((r) => setTimeout(r, 200));
+
+        return {
+          result: result.value,
+          fetchCalled,
+          dataState: breaker.getDataState().mode,
+        };
+      } finally {
+        await deletePersistentCache(cacheKey);
+      }
+    });
+
+    // Persistent entry was expired, so fetch MUST have been called
+    expect(result.fetchCalled).toBe(true);
+    expect(result.result).toBe(222);
+    expect(result.dataState).toBe('live');
+  });
+
+  test('persistent entry older than 24h stale ceiling is not hydrated', async ({ page }) => {
+    await page.goto('/tests/runtime-harness.html');
+
+    const result = await page.evaluate(async () => {
+      const { CircuitBreaker } = await import('/src/utils/circuit-breaker.ts');
+      const { deletePersistentCache } = await import('/src/services/persistent-cache.ts');
+
+      const name = `test-stale-${Date.now()}`;
+      const cacheKey = `breaker:${name}`;
+
+      const DB_NAME = 'worldmonitor_persistent_cache';
+      const STORE = 'entries';
+
+      // Seed with a 25-hour-old entry
+      await new Promise<void>((resolve, reject) => {
+        const request = indexedDB.open(DB_NAME, 1);
+        request.onupgradeneeded = () => {
+          const db = request.result;
+          if (!db.objectStoreNames.contains(STORE)) {
+            db.createObjectStore(STORE, { keyPath: 'key' });
+          }
+        };
+        request.onsuccess = () => {
+          const db = request.result;
+          const tx = db.transaction(STORE, 'readwrite');
+          tx.objectStore(STORE).put({
+            key: cacheKey,
+            data: { value: 333 },
+            updatedAt: Date.now() - 25 * 60 * 60 * 1000, // 25 hours ago
+          });
+          tx.oncomplete = () => resolve();
+          tx.onerror = () => reject(tx.error);
+        };
+        request.onerror = () => reject(request.error);
+      });
+
+      let fetchCalled = false;
+      const breaker = new CircuitBreaker<{ value: number }>({
+        name,
+        cacheTtlMs: 999_999_999, // Very long TTL — would serve if hydrated
+        persistCache: true,
+      });
+
+      try {
+        const result = await breaker.execute(async () => {
+          fetchCalled = true;
+          return { value: 444 };
+        }, { value: 0 });
+
+        return {
+          result: result.value,
+          fetchCalled,
+          dataState: breaker.getDataState().mode,
+        };
+      } finally {
+        await deletePersistentCache(cacheKey);
+      }
+    });
+
+    // 25h entry exceeds 24h ceiling, should NOT be hydrated — fetch must fire
+    expect(result.fetchCalled).toBe(true);
+    expect(result.result).toBe(444);
+    expect(result.dataState).toBe('live');
+  });
+
+  test('clearCache removes persistent entry from IndexedDB', async ({ page }) => {
+    await page.goto('/tests/runtime-harness.html');
+
+    const result = await page.evaluate(async () => {
+      const { CircuitBreaker } = await import('/src/utils/circuit-breaker.ts');
+      const { getPersistentCache, deletePersistentCache } = await import('/src/services/persistent-cache.ts');
+
+      const name = `test-clear-${Date.now()}`;
+      const cacheKey = `breaker:${name}`;
+
+      const breaker = new CircuitBreaker<{ value: number }>({
+        name,
+        cacheTtlMs: 60_000,
+        persistCache: true,
+      });
+
+      try {
+        // Populate cache
+        await breaker.execute(async () => ({ value: 555 }), { value: 0 });
+        await new Promise((r) => setTimeout(r, 200));
+
+        const beforeClear = await getPersistentCache<{ value: number }>(cacheKey);
+
+        // Clear cache
+        breaker.clearCache();
+        await new Promise((r) => setTimeout(r, 200));
+
+        const afterClear = await getPersistentCache<{ value: number }>(cacheKey);
+
+        return {
+          beforeClearValue: beforeClear?.data?.value ?? null,
+          afterClearValue: afterClear?.data?.value ?? null,
+        };
+      } finally {
+        await deletePersistentCache(cacheKey);
+      }
+    });
+
+    expect(result.beforeClearValue).toBe(555);
+    expect(result.afterClearValue).toBeNull();
+  });
+
+  test('persistCache disabled when cacheTtlMs is 0', async ({ page }) => {
+    await page.goto('/tests/runtime-harness.html');
+
+    const result = await page.evaluate(async () => {
+      const { CircuitBreaker } = await import('/src/utils/circuit-breaker.ts');
+      const { getPersistentCache, deletePersistentCache } = await import('/src/services/persistent-cache.ts');
+
+      const name = `test-disabled-${Date.now()}`;
+      const cacheKey = `breaker:${name}`;
+
+      const breaker = new CircuitBreaker<{ value: number }>({
+        name,
+        cacheTtlMs: 0, // Should auto-disable persistence
+      });
+
+      try {
+        await breaker.execute(async () => ({ value: 666 }), { value: 0 });
+        await new Promise((r) => setTimeout(r, 200));
+
+        const entry = await getPersistentCache<{ value: number }>(cacheKey);
+
+        return {
+          persisted: entry?.data?.value ?? null,
+        };
+      } finally {
+        await deletePersistentCache(cacheKey);
+      }
+    });
+
+    // cacheTtlMs=0 auto-disables persistence — nothing should be in IndexedDB
+    expect(result.persisted).toBeNull();
+  });
+
+  test('network failure after reload serves persistent fallback', async ({ page }) => {
+    await page.goto('/tests/runtime-harness.html');
+
+    const result = await page.evaluate(async () => {
+      const { CircuitBreaker } = await import('/src/utils/circuit-breaker.ts');
+      const { setPersistentCache, deletePersistentCache } = await import('/src/services/persistent-cache.ts');
+
+      const name = `test-fallback-${Date.now()}`;
+      const cacheKey = `breaker:${name}`;
+
+      // Seed IndexedDB with data that is OUTSIDE cacheTtlMs but WITHIN 24h ceiling.
+      // This simulates a reload 30 minutes after last successful fetch.
+      await setPersistentCache(cacheKey, { value: 777 });
+
+      // Backdate the updatedAt to 30 minutes ago
+      const DB_NAME = 'worldmonitor_persistent_cache';
+      const STORE = 'entries';
+      await new Promise<void>((resolve, reject) => {
+        const request = indexedDB.open(DB_NAME, 1);
+        request.onsuccess = () => {
+          const db = request.result;
+          const tx = db.transaction(STORE, 'readwrite');
+          tx.objectStore(STORE).put({
+            key: cacheKey,
+            data: { value: 777 },
+            updatedAt: Date.now() - 30 * 60 * 1000, // 30 minutes ago
+          });
+          tx.oncomplete = () => resolve();
+          tx.onerror = () => reject(tx.error);
+        };
+        request.onerror = () => reject(request.error);
+      });
+
+      const breaker = new CircuitBreaker<{ value: number }>({
+        name,
+        cacheTtlMs: 600_000, // 10 min TTL — 30min entry is expired
+        persistCache: true,
+      });
+
+      try {
+        // Fetch fails — should fall back to stale persistent data via getCachedOrDefault
+        const result = await breaker.execute(async () => {
+          throw new Error('Network failure');
+        }, { value: 0 });
+
+        return {
+          result: result.value,
+          dataState: breaker.getDataState().mode,
+        };
+      } finally {
+        await deletePersistentCache(cacheKey);
+      }
+    });
+
+    // Stale persistent data (777) is better than default (0)
+    expect(result.result).toBe(777);
+    expect(result.dataState).toBe('unavailable');
+  });
+});

src-tauri/src/main.rs

@@ -314,6 +314,20 @@ fn read_cache_entry(cache: tauri::State<'_, PersistentCache>, key: String) -> Re
     Ok(cache.get(&key))
 }
 
+#[tauri::command]
+fn delete_cache_entry(cache: tauri::State<'_, PersistentCache>, key: String) -> Result<(), String> {
+    {
+        let mut data = cache.data.lock().unwrap_or_else(|e| e.into_inner());
+        data.remove(&key);
+    }
+    {
+        let mut dirty = cache.dirty.lock().unwrap_or_else(|e| e.into_inner());
+        *dirty = true;
+    }
+    // Disk flush deferred to exit handler (cache.flush) — avoids blocking main thread
+    Ok(())
+}
+
 #[tauri::command]
 fn write_cache_entry(app: AppHandle, cache: tauri::State<'_, PersistentCache>, key: String, value: String) -> Result<(), String> {
     let parsed_value: Value = serde_json::from_str(&value)
@@ -967,6 +981,7 @@ fn main() {
             get_desktop_runtime_info,
             read_cache_entry,
             write_cache_entry,
+            delete_cache_entry,
             open_logs_folder,
             open_sidecar_log_file,
             open_settings_window_command,

src/services/persistent-cache.ts

@@ -127,6 +127,40 @@ export async function setPersistentCache<T>(key: string, data: T): Promise<void>
   }
 }
 
+export async function deletePersistentCache(key: string): Promise<void> {
+  if (isDesktopRuntime()) {
+    try {
+      await invokeTauri<void>('delete_cache_entry', { key });
+      return;
+    } catch {
+      // Fall through to browser storage
+    }
+  }
+
+  if (isIndexedDbAvailable()) {
+    try {
+      const db = await getCacheDb();
+      await new Promise<void>((resolve, reject) => {
+        const tx = db.transaction(CACHE_STORE, 'readwrite');
+        tx.oncomplete = () => resolve();
+        tx.onerror = () => reject(tx.error);
+        tx.objectStore(CACHE_STORE).delete(key);
+      });
+      return;
+    } catch (error) {
+      console.warn('[persistent-cache] IndexedDB delete failed; falling back to localStorage', error);
+      cacheDbPromise = null;
+    }
+  }
+
+  if (isStorageQuotaExceeded()) return;
+  try {
+    localStorage.removeItem(`${CACHE_PREFIX}${key}`);
+  } catch {
+    // Ignore
+  }
+}
+
 export function cacheAgeMs(updatedAt: number): number {
   return Math.max(0, Date.now() - updatedAt);
 }

src/utils/circuit-breaker.ts

@@ -22,11 +22,16 @@ export interface CircuitBreakerOptions {
   maxFailures?: number;
   cooldownMs?: number;
   cacheTtlMs?: number;
+  /** Persist cache to IndexedDB across page reloads. Default: false.
+   *  Opt-in only — cached payloads must be JSON-safe (no Date objects).
+   *  Auto-disabled when cacheTtlMs === 0. */
+  persistCache?: boolean;
 }
 
 const DEFAULT_MAX_FAILURES = 2;
 const DEFAULT_COOLDOWN_MS = 5 * 60 * 1000; // 5 minutes
 const DEFAULT_CACHE_TTL_MS = 10 * 60 * 1000; // 10 minutes
+const PERSISTENT_STALE_CEILING_MS = 24 * 60 * 60 * 1000; // 24h — discard persistent entries older than this
 
 
 function isDesktopOfflineMode(): boolean {
@@ -42,6 +47,9 @@ export class CircuitBreaker<T> {
   private maxFailures: number;
   private cooldownMs: number;
   private cacheTtlMs: number;
+  private persistEnabled: boolean;
+  private persistentLoaded = false;
+  private persistentLoadPromise: Promise<void> | null = null;
   private lastDataState: BreakerDataState = { mode: 'unavailable', timestamp: null, offline: false };
 
   constructor(options: CircuitBreakerOptions) {
@@ -49,6 +57,62 @@ export class CircuitBreaker<T> {
     this.maxFailures = options.maxFailures ?? DEFAULT_MAX_FAILURES;
     this.cooldownMs = options.cooldownMs ?? DEFAULT_COOLDOWN_MS;
     this.cacheTtlMs = options.cacheTtlMs ?? DEFAULT_CACHE_TTL_MS;
+    this.persistEnabled = this.cacheTtlMs === 0
+      ? false
+      : (options.persistCache ?? false);
+  }
+
+  private get persistKey(): string {
+    return `breaker:${this.name}`;
+  }
+
+  /** Hydrate in-memory cache from persistent storage on first call. */
+  private hydratePersistentCache(): Promise<void> {
+    if (this.persistentLoaded) return Promise.resolve();
+    if (this.persistentLoadPromise) return this.persistentLoadPromise;
+
+    this.persistentLoadPromise = (async () => {
+      try {
+        const { getPersistentCache } = await import('../services/persistent-cache');
+        const entry = await getPersistentCache<T>(this.persistKey);
+        if (entry == null || entry.data === undefined || entry.data === null) return;
+
+        const age = Date.now() - entry.updatedAt;
+        if (age > PERSISTENT_STALE_CEILING_MS) return;
+
+        // Only hydrate if in-memory cache is empty (don't overwrite live data)
+        if (this.cache === null) {
+          this.cache = { data: entry.data, timestamp: entry.updatedAt };
+          const withinTtl = (Date.now() - entry.updatedAt) < this.cacheTtlMs;
+          this.lastDataState = {
+            mode: withinTtl ? 'cached' : 'unavailable',
+            timestamp: entry.updatedAt,
+            offline: false,
+          };
+        }
+      } catch (err) {
+        console.warn(`[${this.name}] Persistent cache hydration failed:`, err);
+      } finally {
+        this.persistentLoaded = true;
+        this.persistentLoadPromise = null;
+      }
+    })();
+
+    return this.persistentLoadPromise;
+  }
+
+  /** Fire-and-forget write to persistent storage. */
+  private writePersistentCache(data: T): void {
+    import('../services/persistent-cache').then(({ setPersistentCache }) => {
+      setPersistentCache(this.persistKey, data).catch(() => {});
+    }).catch(() => {});
+  }
+
+  /** Fire-and-forget delete from persistent storage. */
+  private deletePersistentCache(): void {
+    import('../services/persistent-cache').then(({ deletePersistentCache }) => {
+      deletePersistentCache(this.persistKey).catch(() => {});
+    }).catch(() => {});
   }
 
   isOnCooldown(): boolean {
@@ -96,10 +160,18 @@ export class CircuitBreaker<T> {
     this.state = { failures: 0, cooldownUntil: 0 };
     this.cache = { data, timestamp: Date.now() };
     this.lastDataState = { mode: 'live', timestamp: Date.now(), offline: false };
+
+    if (this.persistEnabled) {
+      this.writePersistentCache(data);
+    }
   }
 
   clearCache(): void {
     this.cache = null;
+    this.persistentLoadPromise = null; // orphan any in-flight hydration
+    if (this.persistEnabled) {
+      this.deletePersistentCache();
+    }
   }
 
   recordFailure(error?: string): void {
@@ -117,6 +189,11 @@ export class CircuitBreaker<T> {
   ): Promise<R> {
     const offline = isDesktopOfflineMode();
 
+    // Hydrate from persistent storage on first call (~1-5ms IndexedDB read)
+    if (this.persistEnabled && !this.persistentLoaded) {
+      await this.hydratePersistentCache();
+    }
+
     if (this.isOnCooldown()) {
       console.log(`[${this.name}] Currently unavailable, ${this.getCooldownRemaining()}s remaining`);
       const cachedFallback = this.getCached();