Merge pull request #1023 from abore9769/main

fix(frontend): reliability — timeout, 5xx retry, promise rejection handling, cache invalidation

Author: hman38705

frontend/src/lib/api/__tests__/cache.test.ts

@@ -81,4 +81,56 @@ describe('apiCache', () => {
       expect(CACHE_TTL.LONG).toBe(30 * 60 * 1000);
     });
   });
+
+  describe('tag-based invalidation (#947)', () => {
+    it('invalidates entries matching any given tag while leaving others intact', () => {
+      apiCache.set('url/markets/featured', [{ id: 1 }], CACHE_TTL.SHORT, ['markets']);
+      apiCache.set('url/blockchain/markets/1', { id: 1 }, CACHE_TTL.MEDIUM, ['markets', 'blockchain']);
+      apiCache.set('url/statistics', { total: 100 }, CACHE_TTL.MEDIUM, ['statistics']);
+
+      apiCache.invalidateByTags(['markets']);
+
+      expect(apiCache.get('url/markets/featured')).toBeNull();
+      expect(apiCache.get('url/blockchain/markets/1')).toBeNull();
+      // statistics has a different tag and must survive
+      expect(apiCache.get('url/statistics')).toEqual({ total: 100 });
+    });
+
+    it('invalidates entries carrying multiple tags when any matches', () => {
+      apiCache.set('url/blockchain/stats', { txs: 50 }, CACHE_TTL.MEDIUM, ['blockchain', 'statistics']);
+
+      apiCache.invalidateByTags(['blockchain']);
+
+      expect(apiCache.get('url/blockchain/stats')).toBeNull();
+    });
+
+    it('does not affect entries without tags', () => {
+      apiCache.set('url/content', { items: [] }, CACHE_TTL.SHORT); // no tags
+
+      apiCache.invalidateByTags(['markets']);
+
+      expect(apiCache.get('url/content')).toEqual({ items: [] });
+    });
+
+    it('GET returns fresh data after mutation invalidates its cache tag', () => {
+      // Simulate a cached GET response tagged 'markets'.
+      apiCache.set('url/markets/featured', [{ id: 1, title: 'Old' }], CACHE_TTL.SHORT, ['markets']);
+      expect(apiCache.get('url/markets/featured')).toEqual([{ id: 1, title: 'Old' }]);
+
+      // Simulate a mutation that invalidates the 'markets' tag.
+      apiCache.invalidateByTags(['markets']);
+
+      // Cache miss — the next GET will fetch fresh data from the server.
+      expect(apiCache.get('url/markets/featured')).toBeNull();
+    });
+
+    it('is a no-op when no entries carry the given tag', () => {
+      apiCache.set('url/email/analytics', { sent: 10 }, CACHE_TTL.MEDIUM, ['email']);
+
+      // Invalidating an unrelated tag must not remove anything.
+      apiCache.invalidateByTags(['newsletter']);
+
+      expect(apiCache.get('url/email/analytics')).toEqual({ sent: 10 });
+    });
+  });
 });

frontend/src/lib/api/__tests__/client.test.ts

@@ -1,4 +1,5 @@
 import { api, ApiError } from '../client';
+import { apiCache } from '../cache';
 
 describe('API Client', () => {
   const originalFetch = global.fetch;
@@ -7,6 +8,8 @@ describe('API Client', () => {
   beforeEach(() => {
     process.env.NEXT_PUBLIC_API_URL = 'http://localhost:3001';
     global.fetch = jest.fn();
+    // Clear the module-level cache singleton so each test starts clean.
+    apiCache.clear();
   });
 
   afterEach(() => {
@@ -144,40 +147,43 @@ describe('API Client', () => {
       await expect(api.getBlockchainMarket(999)).rejects.toThrow('Market not found');
     });
 
-    it('should handle 500 Server Error', async () => {
-      (global.fetch as jest.Mock).mockResolvedValueOnce({
+    it('should handle 500 Server Error after exhausting retries', async () => {
+      const serverError = {
         ok: false,
         status: 500,
         statusText: 'Internal Server Error',
         json: async () => ({ message: 'Database connection failed' }),
-      });
+      };
+      // GET requests retry 5xx up to maxRetries (3) times — mock all attempts.
+      (global.fetch as jest.Mock).mockResolvedValue(serverError);
 
       await expect(api.getStatistics()).rejects.toThrow('Database connection failed');
-    });
+      expect(global.fetch).toHaveBeenCalledTimes(4); // 1 initial + 3 retries
+    }, 30_000);
 
     it('should fallback to statusText when error response has no message', async () => {
-      (global.fetch as jest.Mock).mockResolvedValueOnce({
+      const serverError = {
         ok: false,
         status: 503,
         statusText: 'Service Unavailable',
         json: async () => ({}),
-      });
+      };
+      (global.fetch as jest.Mock).mockResolvedValue(serverError);
 
       await expect(api.health()).rejects.toThrow('Service Unavailable');
-    });
+    }, 30_000);
 
     it('should fallback to HTTP status when response is not JSON', async () => {
-      (global.fetch as jest.Mock).mockResolvedValueOnce({
+      const serverError = {
         ok: false,
         status: 502,
         statusText: 'Bad Gateway',
-        json: async () => {
-          throw new Error('Invalid JSON');
-        },
-      });
+        json: async () => { throw new Error('Invalid JSON'); },
+      };
+      (global.fetch as jest.Mock).mockResolvedValue(serverError);
 
       await expect(api.health()).rejects.toThrow('Bad Gateway');
-    });
+    }, 30_000);
   });
 
   describe('Retry behavior', () => {
@@ -347,6 +353,170 @@ describe('API Client', () => {
     });
   });
 
+  describe('Request timeout (#945)', () => {
+    afterEach(() => {
+      jest.useRealTimers();
+    });
+
+    it('fires after 10 seconds and rejects with a distinct TIMEOUT_ERROR', async () => {
+      jest.useFakeTimers();
+
+      (global.fetch as jest.Mock).mockImplementation((_url: string, init: RequestInit) =>
+        new Promise((_resolve, reject) => {
+          (init.signal as AbortSignal).addEventListener('abort', () =>
+            reject(new DOMException('The operation was aborted.', 'AbortError'))
+          );
+        })
+      );
+
+      // Pre-attach .catch so the rejection is never "unhandled" while timers advance.
+      let caughtError: unknown;
+      const settledPromise = api.health().catch(err => { caughtError = err; });
+
+      await jest.advanceTimersByTimeAsync(10_000);
+      await settledPromise;
+
+      expect(caughtError).toMatchObject({
+        name: 'ApiError',
+        code: 'TIMEOUT_ERROR',
+        message: expect.stringContaining('timed out'),
+      });
+    });
+
+    it('surfaces a timeout error distinct from a generic network error', async () => {
+      jest.useFakeTimers();
+
+      (global.fetch as jest.Mock).mockImplementation((_url: string, init: RequestInit) =>
+        new Promise((_resolve, reject) => {
+          (init.signal as AbortSignal).addEventListener('abort', () =>
+            reject(new DOMException('Aborted', 'AbortError'))
+          );
+        })
+      );
+
+      let caughtError: unknown;
+      const settledPromise = api.getStatistics().catch(err => { caughtError = err; });
+      await jest.advanceTimersByTimeAsync(10_000);
+      await settledPromise;
+
+      expect(caughtError).toBeInstanceOf(ApiError);
+      expect((caughtError as ApiError).code).toBe('TIMEOUT_ERROR');
+      // Must NOT be the generic network message so the UI can branch on it.
+      expect((caughtError as ApiError).message).not.toContain('Unable to reach the server');
+    });
+  });
+
+  describe('5xx retry logic (#946)', () => {
+    it('retries GET requests on 5xx: two 502s then 200 succeeds', async () => {
+      const mockData = { status: 'ok' };
+      const gatewayError = {
+        ok: false,
+        status: 502,
+        statusText: 'Bad Gateway',
+        headers: new Map(),
+        json: async () => ({ message: 'Bad Gateway' }),
+      };
+      (global.fetch as jest.Mock)
+        .mockResolvedValueOnce(gatewayError)
+        .mockResolvedValueOnce(gatewayError)
+        .mockResolvedValueOnce({
+          ok: true,
+          text: async () => JSON.stringify(mockData),
+        });
+
+      const result = await api.health();
+      expect(result).toEqual(mockData);
+      expect(global.fetch).toHaveBeenCalledTimes(3); // 1 initial + 2 retries
+    }, 10_000);
+
+    it('does not retry 5xx for POST requests', async () => {
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: false,
+        status: 503,
+        statusText: 'Service Unavailable',
+        json: async () => ({ message: 'Service Unavailable' }),
+      });
+
+      await expect(
+        api.newsletterSubscribe({ email: 'test@example.com' })
+      ).rejects.toThrow('Service Unavailable');
+      expect(global.fetch).toHaveBeenCalledTimes(1);
+    });
+
+    it('does not retry 5xx for DELETE requests', async () => {
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: false,
+        status: 502,
+        statusText: 'Bad Gateway',
+        json: async () => ({ message: 'Bad Gateway' }),
+      });
+
+      await expect(
+        api.newsletterUnsubscribe('test@example.com')
+      ).rejects.toThrow('Bad Gateway');
+      expect(global.fetch).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('Cache invalidation strategy (#947)', () => {
+    it('invalidates only tagged resources on mutation — not the entire cache', async () => {
+      // Prime a statistics cache entry tagged 'statistics'.
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: true,
+        text: async () => JSON.stringify({ total_markets: 10 }),
+      });
+      await api.getStatistics();
+
+      // Prime a markets cache entry tagged 'markets'.
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: true,
+        text: async () => JSON.stringify([{ id: 1 }]),
+      });
+      await api.getFeaturedMarkets();
+
+      // Mutate: resolveMarket invalidates 'markets', 'blockchain', 'statistics'.
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: true,
+        text: async () => JSON.stringify({ invalidated_keys: 2 }),
+      });
+      await api.resolveMarket(1);
+
+      // Both statistics and markets caches must be cleared.
+      // A fresh fetch call should now happen for getStatistics.
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: true,
+        text: async () => JSON.stringify({ total_markets: 11 }),
+      });
+      const freshStats = await api.getStatistics();
+      expect(freshStats).toEqual({ total_markets: 11 });
+    });
+
+    it('GET returns fresh data after a mutation invalidates its cache tag', async () => {
+      // Cache getFeaturedMarkets.
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: true,
+        text: async () => JSON.stringify([{ id: 1, title: 'Old Market' }]),
+      });
+      const first = await api.getFeaturedMarkets();
+      expect(first).toEqual([{ id: 1, title: 'Old Market' }]);
+
+      // resolveMarket invalidates the 'markets' tag.
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: true,
+        text: async () => JSON.stringify({ invalidated_keys: 1 }),
+      });
+      await api.resolveMarket(1);
+
+      // Next getFeaturedMarkets must hit the network, not the cache.
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: true,
+        text: async () => JSON.stringify([{ id: 1, title: 'Resolved Market' }]),
+      });
+      const second = await api.getFeaturedMarkets();
+      expect(second).toEqual([{ id: 1, title: 'Resolved Market' }]);
+    });
+  });
+
   describe('DELETE requests', () => {
     it('should handle DELETE requests with body', async () => {
       const mockResponse = { success: true };
@@ -404,12 +574,14 @@ describe('API Client', () => {
     });
 
     it('should classify 5xx as server error', async () => {
-      (global.fetch as jest.Mock).mockResolvedValueOnce({
+      const serverError = {
         ok: false,
         status: 503,
         statusText: 'Service Unavailable',
         json: async () => ({ message: 'Service Unavailable' }),
-      });
+      };
+      // GET retries on 5xx — provide enough responses to exhaust retries.
+      (global.fetch as jest.Mock).mockResolvedValue(serverError);
 
       try {
         await api.getStatistics();
@@ -419,7 +591,7 @@ describe('API Client', () => {
         expect((e as ApiError).isServerError).toBe(true);
         expect((e as ApiError).isClientError).toBe(false);
       }
-    });
+    }, 30_000);
 
     it('should have name "ApiError"', async () => {
       (global.fetch as jest.Mock).mockRejectedValueOnce(new Error('offline'));

frontend/src/lib/api/cache.ts

@@ -9,6 +9,7 @@ interface CacheEntry<T> {
   timestamp: number;
   ttl: number;
   stale: boolean;
+  tags?: readonly string[];
 }
 
 export interface CacheResult<T> {
@@ -52,17 +53,33 @@ class ApiCache {
   }
 
   /**
-   * Set cache entry with TTL in milliseconds
+   * Set cache entry with TTL in milliseconds and optional resource tags.
+   * Tags enable targeted invalidation: use the same tag on related GET entries
+   * so a single mutation can invalidate exactly the affected resources.
    */
-  set<T>(key: string, data: T, ttlMs: number): void {
+  set<T>(key: string, data: T, ttlMs: number, tags?: string[]): void {
     this.cache.set(key, {
       data,
       timestamp: Date.now(),
       ttl: ttlMs,
       stale: false,
+      tags,
     });
   }
 
+  /**
+   * Invalidate all cache entries that carry at least one of the given tags.
+   * Call this after a mutation to drop only the affected resource namespaces
+   * rather than clearing the entire cache.
+   */
+  invalidateByTags(tags: string[]): void {
+    for (const [key, entry] of this.cache.entries()) {
+      if (entry.tags?.some(tag => tags.includes(tag))) {
+        this.cache.delete(key);
+      }
+    }
+  }
+
   /**
    * Mark a single cache entry as stale. Called when the API returns an error
    * for a fresh request so the UI can display the old value with a warning.