maybe simplify error handling in RemoteFeatureFlagController

packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.test.ts

@@ -1,3 +1,4 @@
+import { BASE_URL } from '../constants';
 import type { FeatureFlags } from '../remote-feature-flag-controller-types';
 import {
   ClientType,
@@ -6,10 +7,7 @@
 } from '../remote-feature-flag-controller-types';
 import { ClientConfigApiService } from './client-config-api-service';
 
-const BASE_URL = 'https://client-config.api.cx.metamask.io/v1';
-
 describe('ClientConfigApiService', () => {
-  let mockFetch: jest.Mock;
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   let consoleErrorSpy: jest.SpyInstance;
   const mockFeatureFlags: FeatureFlags = [
@@ -26,11 +24,13 @@
   });
 
   beforeEach(() => {
-    mockFetch = jest.fn();
     consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
   });
 
   it('should successfully fetch and return feature flags', async () => {
+    const mockFetch = createMockFetch({
+      data: mockFeatureFlags,
+    });
     const clientConfigApiService = new ClientConfigApiService({
       fetch: mockFetch,
       retries: 0,
@@ -41,31 +41,22 @@
       },
     });
 
-    mockFetch.mockResolvedValueOnce({
-      ok: true,
-      status: 200,
-      statusText: 'OK',
-      json: async () => mockFeatureFlags,
-    });
-
-    const result = await clientConfigApiService.fetchRemoteFeatureFlags();
+    const result = await clientConfigApiService.fetchRemoteFeatureFlag();
 
     expect(mockFetch).toHaveBeenCalledWith(
       `${BASE_URL}/flags?client=extension&distribution=main&environment=prod`,
       { cache: 'no-cache' },
     );
 
     expect(result).toStrictEqual({
-      error: false,
-      message: 'Success',
-      statusCode: '200',
-      statusText: 'OK',
-      cachedData: mockFeatureFlags,
+      remoteFeatureFlag: mockFeatureFlags,
       cacheTimestamp: expect.any(Number),
     });
   });
 
   it('should return cached data when API request fails and cached data is available', async () => {
+    const mockFetch = createMockFetch({ error: networkError });
+
     const clientConfigApiService = new ClientConfigApiService({
       fetch: mockFetch,
       retries: 0,
@@ -76,27 +67,14 @@
       },
     });
 
-    const cachedData = [{ feature3: true }];
+    const remoteFeatureFlag = [{ feature3: true }];
     const cacheTimestamp = Date.now();
 
-    mockFetch.mockRejectedValueOnce(networkError);
-
-    const result = await clientConfigApiService.fetchRemoteFeatureFlags({
-      cachedData,
-      cacheTimestamp,
-    });
-
-    expect(result).toStrictEqual({
-      error: true,
-      message: 'Network error',
-      statusCode: '503',
-      statusText: 'Service Unavailable',
-      cachedData,
-      cacheTimestamp,
-    });
+    expect(clientConfigApiService.fetchRemoteFeatureFlag()).rejects.toThrow(networkError);
   });
 
   it('should return empty object when API request fails and cached data is not available', async () => {
+    const mockFetch = createMockFetch({ error: networkError });
     const clientConfigApiService = new ClientConfigApiService({
       fetch: mockFetch,
       retries: 0,
@@ -107,20 +85,17 @@
       },
     });
 
-    mockFetch.mockRejectedValueOnce(networkError);
-    const result = await clientConfigApiService.fetchRemoteFeatureFlags();
-
-    expect(result).toStrictEqual({
-      error: true,
-      message: 'Network error',
-      statusCode: '503',
-      statusText: 'Service Unavailable',
-      cachedData: [],
-      cacheTimestamp: expect.any(Number),
-    });
+    expect(clientConfigApiService.fetchRemoteFeatureFlag()).rejects.toThrow(networkError);
   });
 
   it('should handle non-200 responses without cache data', async () => {
+    const mockFetch = createMockFetch({
+      options: {
+        ok: false,
+        status: 404,
+        statusText: 'Not Found',
+      },
+    });
     const clientConfigApiService = new ClientConfigApiService({
       fetch: mockFetch,
       retries: 0,
@@ -130,26 +105,17 @@
         environment: EnvironmentType.Production,
       },
     });
-
-    mockFetch.mockResolvedValueOnce({
-      ok: false,
-      status: 404,
-      statusText: 'Not Found',
-    });
-
-    const result = await clientConfigApiService.fetchRemoteFeatureFlags();
-    const currentTime = Date.now();
-    expect(result).toStrictEqual({
-      error: true,
-      message: 'Failed to fetch flags',
-      statusCode: '404',
-      statusText: 'Not Found',
-      cachedData: [],
-      cacheTimestamp: currentTime,
-    });
+    expect(clientConfigApiService.fetchRemoteFeatureFlag()).rejects.toThrow('Failed to fetch feature flags');
   });
 
   it('should handle non-200 responses with cache data', async () => {
+    const mockFetch = createMockFetch({
+      options: {
+        ok: false,
+        status: 404,
+        statusText: 'Not Found',
+      },
+    });
     const clientConfigApiService = new ClientConfigApiService({
       fetch: mockFetch,
       retries: 0,
@@ -160,30 +126,11 @@
       },
     });
 
-    const cachedData = [{ feature3: true }];
-    const cacheTimestamp = Date.now();
-    mockFetch.mockResolvedValueOnce({
-      ok: false,
-      status: 404,
-      statusText: 'Not Found',
-    });
-
-    const result = await clientConfigApiService.fetchRemoteFeatureFlags({
-      cachedData,
-      cacheTimestamp,
-    });
-
-    expect(result).toStrictEqual({
-      error: true,
-      message: 'Failed to fetch flags',
-      statusCode: '404',
-      statusText: 'Not Found',
-      cachedData,
-      cacheTimestamp,
-    });
+    expect(clientConfigApiService.fetchRemoteFeatureFlag()).rejects.toThrow('Failed to fetch feature flags');
   });
 
   it('should retry the fetch the specified number of times on failure', async () => {
+    const mockFetch = createMockFetch({ error: networkError });
     const maxRetries = 3;
     const clientConfigApiService = new ClientConfigApiService({
       fetch: mockFetch,
@@ -195,24 +142,13 @@
       },
     });
 
-    // Mock fetch to fail every time
-    mockFetch.mockRejectedValue(networkError);
-
-    const result = await clientConfigApiService.fetchRemoteFeatureFlags();
-    const currentTime = Date.now();
-    expect(result).toStrictEqual({
-      error: true,
-      message: 'Network error',
-      statusCode: '503',
-      statusText: 'Service Unavailable',
-      cachedData: [],
-      cacheTimestamp: currentTime,
-    });
+    expect(clientConfigApiService.fetchRemoteFeatureFlag()).rejects.toThrow('Failed to fetch feature flags');
     // Check that fetch was retried the correct number of times
     expect(mockFetch).toHaveBeenCalledTimes(maxRetries + 1); // Initial + retries
   });
 
   it('should open the circuit breaker after consecutive failures', async () => {
+    const mockFetch = createMockFetch({ error: networkError });
     const maxFailures = 3; // Set max consecutive failures for circuit breaker
     const clientConfigApiService = new ClientConfigApiService({
       fetch: mockFetch,
@@ -224,22 +160,19 @@
       },
     });
 
-    // Mock fetch to fail every time
-    mockFetch.mockRejectedValue(networkError);
-
     // Trigger fetch attempts
     for (let i = 0; i < maxFailures; i++) {
-      await clientConfigApiService.fetchRemoteFeatureFlags();
+      await clientConfigApiService.fetchRemoteFeatureFlag();
     }
 
-    const result = await clientConfigApiService.fetchRemoteFeatureFlags();
+    const result = await clientConfigApiService.fetchRemoteFeatureFlag();
 
     expect(result).toStrictEqual({
       error: true,
       message: 'Execution prevented because the circuit breaker is open',
       statusCode: null,
       statusText: null,
-      cachedData: [],
+      remoteFeatureFlag: [],
       cacheTimestamp: expect.any(Number),
     });
 
@@ -251,9 +184,14 @@
     jest.setTimeout(7000);
     const onDegraded = jest.fn();
     const slowFetchTime = 5500; // Exceed the DEFAULT_DEGRADED_THRESHOLD (5000ms)
+    // Mock fetch to take a long time
+    const mockSlowFetch = createMockFetch({
+      data: mockFeatureFlags,
+      delay: slowFetchTime,
+    });
 
     const clientConfigApiService = new ClientConfigApiService({
-      fetch: mockFetch,
+      fetch: mockSlowFetch,
       onDegraded,
       config: {
         client: ClientType.Extension,
@@ -262,43 +200,17 @@
       },
     });
 
-    // Mock fetch to take a long time
-    mockFetch.mockImplementation(
-      () =>
-        new Promise((resolve) =>
-          setTimeout(
-            () =>
-              resolve({
-                ok: true,
-                status: 200,
-                statusText: 'OK',
-                json: async () => mockFeatureFlags,
-              }),
-            slowFetchTime,
-          ),
-        ),
-    );
-
-    await clientConfigApiService.fetchRemoteFeatureFlags();
+    await clientConfigApiService.fetchRemoteFeatureFlag();
 
     // Verify the degraded callback was called
     expect(onDegraded).toHaveBeenCalled();
   }, 7000);
 
   it('should succeed on a subsequent fetch attempt after retries', async () => {
     const maxRetries = 2;
-    const clientConfigApiService = new ClientConfigApiService({
-      fetch: mockFetch,
-      retries: maxRetries,
-      config: {
-        client: ClientType.Extension,
-        distribution: DistributionType.Main,
-        environment: EnvironmentType.Production,
-      },
-    });
-
     // Mock fetch to fail initially, then succeed
-    mockFetch
+    const mockFetch = jest
+      .fn()
       .mockRejectedValueOnce(networkError) // First attempt fails
       .mockRejectedValueOnce(networkError) // Second attempt fails
       .mockResolvedValueOnce({
@@ -307,20 +219,77 @@
         statusText: 'OK',
         json: async () => mockFeatureFlags, // Third attempt succeeds
       });
+    const clientConfigApiService = new ClientConfigApiService({
+      fetch: mockFetch,
+      retries: maxRetries,
+      config: {
+        client: ClientType.Extension,
+        distribution: DistributionType.Main,
+        environment: EnvironmentType.Production,
+      },
+    });
 
-    const result = await clientConfigApiService.fetchRemoteFeatureFlags();
+    const result = await clientConfigApiService.fetchRemoteFeatureFlag();
 
     // Verify success on the third attempt
     expect(result).toStrictEqual({
       error: false,
       message: 'Success',
       statusCode: '200',
       statusText: 'OK',
-      cachedData: mockFeatureFlags,
+      remoteFeatureFlag: mockFeatureFlags,
       cacheTimestamp: expect.any(Number),
     });
 
     // Verify fetch was retried the correct number of times
     expect(mockFetch).toHaveBeenCalledTimes(maxRetries + 1); // Initial + retries
   });
 });
+
+/**
+ * Creates a mock fetch function with configurable response data and options
+ * @template T - The type of data to be returned by the fetch response
+ * @param params - Configuration parameters
+ * @param params.data - The data to be returned in the response body
+ * @param params.options - Optional Response properties to override defaults
+ * @param params.error - Error to reject with (if provided, mock will reject instead of resolve)
+ * @param params.delay - Delay in milliseconds before resolving/rejecting
+ * @returns A Jest mock function that resolves with a fetch-like Response object (or rejects with error if provided)
+ */
+function createMockFetch<ResponseData>({
+  data,
+  options = {},
+  error,
+  delay = 0,
+}: {
+  data?: ResponseData;
+  options?: Partial<Response>;
+  error?: Error;
+  delay?: number;
+}) {
+  if (error) {
+    return jest
+      .fn()
+      .mockImplementation(
+        () =>
+          new Promise((_, reject) => setTimeout(() => reject(error), delay)),
+      );
+  }
+
+  return jest.fn().mockImplementation(
+    () =>
+      new Promise((resolve) =>
+        setTimeout(
+          () =>
+            resolve({
+              ok: true,
+              status: 200,
+              statusText: 'OK',
+              json: async () => data,
+              ...options,
+            }),
+          delay,
+        ),
+      ),
+  );
+}