add retry for spdx data (#1313)

* extract retry utils

* util for getting spdx data

* use util for spdx data

* address feedback + fix issue with fetching spdx data

* fix build error

frontend/src/pages/plugins/[name].tsx

@@ -9,12 +9,12 @@ import { PluginPage } from '@/components/PluginPage';
 import { DEFAULT_REPO_DATA } from '@/constants/plugin';
 import { useLoadingState } from '@/context/loading';
 import { PluginStateProvider } from '@/context/plugin';
-import { SpdxLicenseData, SpdxLicenseResponse } from '@/store/search/types';
+import { SpdxLicenseData } from '@/store/search/types';
 import { PluginData } from '@/types';
 import { createUrl, fetchRepoData, FetchRepoDataResult, Logger } from '@/utils';
 import { getErrorMessage } from '@/utils/error';
 import { hubAPI } from '@/utils/HubAPIClient';
-import { spdxLicenseDataAPI } from '@/utils/spdx';
+import { getSpdxProps } from '@/utils/spdx';
 import { getServerSidePropsHandler } from '@/utils/ssr';
 
 /**
@@ -41,57 +41,49 @@ export const getServerSideProps = getServerSidePropsHandler<Props, Params>({
    */
   async getProps({ params }) {
     const name = String(params?.name);
-    const props: Props = {
-      repo: DEFAULT_REPO_DATA,
-    };
-
     let codeRepo = '';
+    let plugin: PluginData | undefined;
 
     try {
-      const plugin = await hubAPI.getPlugin(name);
+      plugin = await hubAPI.getPlugin(name);
       codeRepo = plugin.code_repository;
-      props.plugin = plugin;
     } catch (err) {
-      props.error = getErrorMessage(err);
+      const error = getErrorMessage(err);
       logger.error({
         message: 'Failed to fetch plugin data',
         plugin: name,
-        error: props.error,
+        error,
       });
 
-      return { props };
+      return {
+        props: { error },
+      };
     }
 
     const repoData = await fetchRepoData(codeRepo);
-    Object.assign(props, repoData);
 
-    if (props.repoFetchError) {
-      const logType = inRange(props.repoFetchError.status, 400, 500)
+    if (repoData.repoFetchError) {
+      const logType = inRange(repoData.repoFetchError.status, 400, 500)
         ? 'info'
         : 'error';
 
       logger[logType]({
         message: 'Failed to fetch repo data',
         plugin: name,
-        error: props.error,
+        error: repoData.repoFetchError,
       });
     }
 
-    try {
-      const {
-        data: { licenses },
-      } = await spdxLicenseDataAPI.get<SpdxLicenseResponse>('');
-      props.licenses = licenses;
-    } catch (err) {
-      props.error = getErrorMessage(err);
+    const licenses = await getSpdxProps(logger);
 
-      logger.error({
-        message: 'Failed to fetch spdx license data',
-        error: props.error,
-      });
-    }
-
-    return { props };
+    return {
+      props: {
+        plugin,
+        licenses,
+        repo: DEFAULT_REPO_DATA,
+        ...repoData,
+      },
+    };
   },
 });
 
@@ -157,7 +149,7 @@ export default function Plugin({
             <PluginStateProvider
               licenses={licenses}
               plugin={plugin}
-              repo={repo}
+              repo={repo ?? DEFAULT_REPO_DATA}
               repoFetchError={repoFetchError}
             >
               <PluginPage />

frontend/src/pages/plugins/index.tsx

@@ -5,12 +5,12 @@ import { ErrorMessage } from '@/components/ErrorMessage';
 import { NotFoundPage } from '@/components/NotFoundPage';
 import { SearchPage } from '@/components/SearchPage';
 import { SearchStoreProvider } from '@/store/search/context';
-import { SpdxLicenseData, SpdxLicenseResponse } from '@/store/search/types';
+import { SpdxLicenseData } from '@/store/search/types';
 import { PluginIndexData } from '@/types';
 import { Logger } from '@/utils';
 import { getErrorMessage } from '@/utils/error';
 import { hubAPI } from '@/utils/HubAPIClient';
-import { spdxLicenseDataAPI } from '@/utils/spdx';
+import { getSpdxProps as getSpdxLicenses } from '@/utils/spdx';
 import { getServerSidePropsHandler } from '@/utils/ssr';
 
 interface Props {
@@ -24,37 +24,30 @@ const logger = new Logger('pages/plugins/index.tsx');
 
 export const getServerSideProps = getServerSidePropsHandler<Props>({
   async getProps() {
-    const props: Props = {
-      status: 200,
-    };
+    let index: PluginIndexData[];
 
     try {
-      const index = await hubAPI.getPluginIndex();
-      props.index = index;
+      index = await hubAPI.getPluginIndex();
     } catch (err) {
-      props.error = getErrorMessage(err);
+      const error = getErrorMessage(err);
 
       logger.error({
         message: 'Failed to plugin index',
-        error: props.error,
+        error,
       });
-    }
-
-    try {
-      const {
-        data: { licenses },
-      } = await spdxLicenseDataAPI.get<SpdxLicenseResponse>('');
-      props.licenses = licenses;
-    } catch (err) {
-      props.error = getErrorMessage(err);
 
-      logger.error({
-        message: 'Failed to fetch spdx license data',
-        error: props.error,
-      });
+      return { props: { error } };
     }
 
-    return { props };
+    const licenses = await getSpdxLicenses(logger);
+
+    return {
+      props: {
+        index,
+        licenses,
+        status: 200,
+      },
+    };
   },
 });
 

frontend/src/utils/HubAPIClient.ts

@@ -12,8 +12,8 @@ import {
   PluginSectionType,
 } from '@/types';
 
+import { retryAxios } from './async';
 import { Logger } from './logger';
-import { sleep } from './sleep';
 import { getFullPathFromAxios } from './url';
 import {
   validateMetricsData,
@@ -72,21 +72,6 @@ function isHubAPIErrorResponse(
   return !!(data as HubAPIErrorResponse).errorType;
 }
 
-/**
- * Max number of times to retry fetching a request.
- */
-const MAX_RETRIES = 3;
-
-/**
- * Backoff factory for increasing the delay when re-fetching requests.
- */
-const RETRY_BACKOFF_FACTOR = 2;
-
-/**
- * Initial delay before retrying a request in milliseconds.
- */
-const INITIAL_RETRY_DELAY_MS = 1000;
-
 /**
  * Class for interacting with the hub API. Each function makes a request to the
  * hub API and runs client-side data validation on the data to ensure
@@ -103,66 +88,24 @@ export class HubAPIClient {
   });
 
   private async sendRequest<T>(url: string, config?: AxiosRequestConfig<T>) {
-    const method = config?.method ?? 'GET';
-    const path = getFullPathFromAxios(url, config);
-    let retryDelay = INITIAL_RETRY_DELAY_MS;
-
-    for (let retry = 0; retry < MAX_RETRIES; retry += 1) {
-      try {
-        const { data, status } = await this.api.request<T>({
-          url,
-          params: {
-            ...config?.params,
-            retryCount: retry,
-          } as Record<string, unknown>,
-          ...config,
-        });
-
-        if (SERVER) {
-          logger.info({
-            path,
-            method,
-            url,
-            status,
-          });
-        }
-
-        return data;
-      } catch (err) {
-        const isRetrying = retry < MAX_RETRIES - 1;
-
-        if (axios.isAxiosError(err)) {
-          const status = err.response?.status;
-          const level =
-            isRetrying ||
-            (status !== undefined && status >= 400 && status < 500)
-              ? 'warn'
-              : 'error';
-
-          logger[level]({
-            message: 'Error sending request',
-            error: err.message,
-            method,
-            path,
-            url,
-            isRetrying,
-            retry,
-            ...(err.response?.status ? { status: err.response.status } : {}),
-          });
-        }
-
-        if (isRetrying) {
-          await sleep(retryDelay);
-          retryDelay *= RETRY_BACKOFF_FACTOR;
-        } else {
-          throw err;
-        }
-      }
+    const { data, status } = await retryAxios<T>({
+      instance: this.api,
+      url,
+      ...config,
+    });
+
+    if (SERVER) {
+      const method = config?.method ?? 'GET';
+      const path = getFullPathFromAxios(url, config);
+      logger.info({
+        path,
+        method,
+        url,
+        status,
+      });
     }
 
-    // This edge case should never happen but is required by TypeScript to
-    // prevent a compile error.
-    throw new Error('failed to request data');
+    return data;
   }
 
   async getPluginIndex(): Promise<PluginIndexData[]> {

frontend/src/utils/async.test.ts

@@ -0,0 +1,82 @@
+import axios from 'axios';
+
+import { retryAsync, retryAxios } from './async';
+
+describe('retryAsync', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should retry promise fails', async () => {
+    const expectedRetryCount = 2;
+    let actualRetryCount = 0;
+
+    async function execute() {
+      if (actualRetryCount === expectedRetryCount) {
+        return Promise.resolve();
+      }
+
+      actualRetryCount += 1;
+      throw new Error('failure');
+    }
+
+    await retryAsync({ execute });
+    expect(actualRetryCount).toBe(expectedRetryCount);
+  });
+
+  it('should fail when promise fails too many times', async () => {
+    const expectedRetryCount = 3;
+    let actualRetryCount = 0;
+
+    // eslint-disable-next-line @typescript-eslint/require-await
+    async function execute() {
+      actualRetryCount += 1;
+      throw new Error('failure');
+    }
+
+    await expect(
+      retryAsync({ execute, retries: expectedRetryCount }),
+    ).rejects.toThrow('failure');
+    expect(actualRetryCount).toBe(expectedRetryCount);
+  });
+});
+
+describe('retryAxios', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should retry fetching a failed request', async () => {
+    const expectedRetryCount = 2;
+    let actualRetryCount = 0;
+    jest.spyOn(axios, 'request').mockImplementation(() => {
+      if (actualRetryCount === expectedRetryCount) {
+        return Promise.resolve({
+          data: 'data',
+          status: 200,
+        });
+      }
+
+      actualRetryCount += 1;
+      throw new Error('failure');
+    });
+
+    const res = await retryAxios({ url: '/test' });
+    expect(res.data).toBe('data');
+    expect(actualRetryCount).toBe(expectedRetryCount);
+  });
+
+  it('should fail when the request fails too many times', async () => {
+    const expectedRetryCount = 3;
+    let actualRetryCount = 0;
+    jest.spyOn(axios, 'request').mockImplementation(() => {
+      actualRetryCount += 1;
+      throw new Error('failure');
+    });
+
+    await expect(
+      retryAxios({ url: '/test', retries: expectedRetryCount }),
+    ).rejects.toThrow('failure');
+    expect(actualRetryCount).toBe(expectedRetryCount);
+  });
+});