diff --git a/frontend/src/pages/plugins/[name].tsx b/frontend/src/pages/plugins/[name].tsx index 8ef022438..a0c9592da 100644 --- a/frontend/src/pages/plugins/[name].tsx +++ b/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({ */ 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(''); - 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({ diff --git a/frontend/src/pages/plugins/index.tsx b/frontend/src/pages/plugins/index.tsx index bc2dc2459..329f93782 100644 --- a/frontend/src/pages/plugins/index.tsx +++ b/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({ 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(''); - 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, + }, + }; }, }); diff --git a/frontend/src/utils/HubAPIClient.ts b/frontend/src/utils/HubAPIClient.ts index be89b867f..b92476350 100644 --- a/frontend/src/utils/HubAPIClient.ts +++ b/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(url: string, config?: AxiosRequestConfig) { - 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({ - url, - params: { - ...config?.params, - retryCount: retry, - } as Record, - ...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({ + 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 { diff --git a/frontend/src/utils/async.test.ts b/frontend/src/utils/async.test.ts new file mode 100644 index 000000000..c724b8d93 --- /dev/null +++ b/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); + }); +}); diff --git a/frontend/src/utils/async.ts b/frontend/src/utils/async.ts new file mode 100644 index 000000000..7c85aa468 --- /dev/null +++ b/frontend/src/utils/async.ts @@ -0,0 +1,112 @@ +/* eslint-disable no-await-in-loop */ +import axios, { + AxiosInstance, + AxiosRequestConfig, + AxiosResponse, + AxiosStatic, +} from 'axios'; + +import { Logger } from './logger'; +import { sleep } from './sleep'; +import { getFullPathFromAxios } from './url'; + +/** + * Max number of times to retry fetching a request. + */ +const DEFAULT_MAX_RETRIES = 3; + +/** + * Backoff factory for increasing the delay when re-fetching requests. + */ +const DEFAULT_RETRY_BACKOFF_FACTOR = 2; + +/** + * Initial delay before retrying a request in milliseconds. + */ +const DEFAULT_INITIAL_RETRY_DELAY_MS = 1000; + +interface AsyncRetryOptions { + backoffFactor?: number; + execute(): Promise; + initialDelay?: number; + onError?(data: { err: Error; isRetrying: boolean; retry: number }): void; + retries?: number; +} + +export async function retryAsync({ + backoffFactor = DEFAULT_RETRY_BACKOFF_FACTOR, + execute, + initialDelay = DEFAULT_INITIAL_RETRY_DELAY_MS, + onError, + retries = DEFAULT_MAX_RETRIES, +}: AsyncRetryOptions) { + let retryDelay = initialDelay; + + for (let retry = 0; retry < retries; retry += 1) { + try { + return await execute(); + } catch (err) { + const isRetrying = retry < retries - 1; + + onError?.({ + isRetrying, + retry, + err: err as Error, + }); + + if (isRetrying) { + await sleep(retryDelay); + retryDelay *= backoffFactor; + } else { + throw err; + } + } + } + + // edge case should not happen, but needed for typescript to not throw error + throw new Error('Failed to execute function'); +} + +interface AsyncAxiosRetryOptions + extends Omit>, 'execute' | 'onError'> { + config?: AxiosRequestConfig; + instance?: AxiosInstance | AxiosStatic; + logger?: Logger; + url?: string; +} + +export async function retryAxios({ + config, + instance = axios, + logger, + url = '', + ...options +}: AsyncAxiosRetryOptions = {}) { + const method = config?.method ?? 'GET'; + const path = getFullPathFromAxios(url, config); + + return retryAsync>({ + ...options, + execute: () => instance.request({ url, ...config }), + onError({ err, isRetrying, retry }) { + 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 } : {}), + }); + } + }, + }); +} diff --git a/frontend/src/utils/index.ts b/frontend/src/utils/index.ts index 8fe3fdd88..1ddb9854b 100644 --- a/frontend/src/utils/index.ts +++ b/frontend/src/utils/index.ts @@ -1,3 +1,4 @@ +export * from './async'; export * from './format'; export * from './logger'; export * from './performance'; diff --git a/frontend/src/utils/repo.ts b/frontend/src/utils/repo.ts index 36a2849cd..be285e41c 100644 --- a/frontend/src/utils/repo.ts +++ b/frontend/src/utils/repo.ts @@ -11,7 +11,7 @@ import { PluginRepoData, PluginRepoFetchError } from '@/types'; const REPO_REGEX = /(?:git@|https:\/\/)(github).com[/:](.*)(?:.git)?/; export interface FetchRepoDataResult { - repo: PluginRepoData; + repo?: PluginRepoData; repoFetchError?: PluginRepoFetchError; } diff --git a/frontend/src/utils/spdx.ts b/frontend/src/utils/spdx.ts index 8be8257c0..26fba52b3 100644 --- a/frontend/src/utils/spdx.ts +++ b/frontend/src/utils/spdx.ts @@ -1,6 +1,34 @@ +/* eslint-disable no-param-reassign */ + import axios from 'axios'; +import { SpdxLicenseData, SpdxLicenseResponse } from '@/store/search/types'; + +import { retryAxios } from './async'; +import { getErrorMessage } from './error'; +import { Logger } from './logger'; + export const spdxLicenseDataAPI = axios.create({ baseURL: 'https://raw.githubusercontent.com/spdx/license-list-data/master/json/licenses.json', }); + +export async function getSpdxProps( + logger?: Logger, +): Promise { + try { + const { + data: { licenses }, + } = await retryAxios({ instance: spdxLicenseDataAPI }); + + return licenses; + } catch (err) { + const error = getErrorMessage(err); + logger?.error({ + message: 'Failed to fetch spdx license data', + error, + }); + + return []; + } +}