From 06e0accf22f1e949ee4935bb0cba0e46ca7822c5 Mon Sep 17 00:00:00 2001 From: Jeremy Asuncion Date: Wed, 6 Mar 2024 13:15:46 -0800 Subject: [PATCH] add retry fetch (#1306) * add sleep util * add retry logic to hub API client * unit tests for retry --- frontend/src/utils/HubAPIClient.test.ts | 47 +++++++++++ frontend/src/utils/HubAPIClient.ts | 102 ++++++++++++++++-------- frontend/src/utils/sleep.ts | 5 ++ 3 files changed, 122 insertions(+), 32 deletions(-) create mode 100644 frontend/src/utils/HubAPIClient.test.ts create mode 100644 frontend/src/utils/sleep.ts diff --git a/frontend/src/utils/HubAPIClient.test.ts b/frontend/src/utils/HubAPIClient.test.ts new file mode 100644 index 000000000..de56bc630 --- /dev/null +++ b/frontend/src/utils/HubAPIClient.test.ts @@ -0,0 +1,47 @@ +import axios, { AxiosInstance } from 'axios'; + +import mockPlugin from '@/fixtures/plugin.json'; + +import { HubAPIClient } from './HubAPIClient'; +import { validatePluginData } from './validate'; + +describe('HubAPIClient', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should retry fetching a failed request', async () => { + let retryCount = 0; + jest.spyOn(axios, 'create').mockImplementation(() => { + return { + request: jest.fn(() => { + if (retryCount > 1) { + return Promise.resolve({ + data: mockPlugin, + status: 200, + }); + } + + retryCount += 1; + throw new Error('failure'); + }), + } as unknown as AxiosInstance; + }); + + const client = new HubAPIClient(); + await expect(client.getPlugin('test')).resolves.toEqual( + validatePluginData(mockPlugin), + ); + }); + + it('should fail when the request fails too many times', async () => { + jest.spyOn(axios, 'create').mockImplementation(() => { + return { + request: jest.fn().mockRejectedValue(new Error('failure')), + } as unknown as AxiosInstance; + }); + + const client = new HubAPIClient(); + await expect(client.getPlugin('test')).rejects.toThrow('failure'); + }); +}); diff --git a/frontend/src/utils/HubAPIClient.ts b/frontend/src/utils/HubAPIClient.ts index 725697751..be89b867f 100644 --- a/frontend/src/utils/HubAPIClient.ts +++ b/frontend/src/utils/HubAPIClient.ts @@ -1,3 +1,4 @@ +/* eslint-disable no-await-in-loop */ /* eslint-disable max-classes-per-file */ import axios, { AxiosRequestConfig } from 'axios'; @@ -12,6 +13,7 @@ import { } from '@/types'; import { Logger } from './logger'; +import { sleep } from './sleep'; import { getFullPathFromAxios } from './url'; import { validateMetricsData, @@ -70,12 +72,27 @@ 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 * consistency with static typing and reduce the chance of errors occurring. */ -class HubAPIClient { +export class HubAPIClient { private api = axios.create({ baseURL: API_URL, headers: BROWSER @@ -88,43 +105,64 @@ 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; - try { - const { data, status } = await this.api.request({ - url, - ...config, - }); - - if (SERVER) { - logger.info({ - path, - method, + for (let retry = 0; retry < MAX_RETRIES; retry += 1) { + try { + const { data, status } = await this.api.request({ url, - status, + params: { + ...config?.params, + retryCount: retry, + } as Record, + ...config, }); - } - return data; - } catch (err) { - if (axios.isAxiosError(err)) { - const status = err.response?.status; - const level = - status !== undefined && status >= 400 && status < 500 - ? 'warn' - : 'error'; - - logger[level]({ - message: 'Error sending request', - error: err.message, - method, - path, - url, - ...(err.response?.status ? { status: err.response.status } : {}), - }); + 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; + } } - - throw err; } + + // This edge case should never happen but is required by TypeScript to + // prevent a compile error. + throw new Error('failed to request data'); } async getPluginIndex(): Promise { diff --git a/frontend/src/utils/sleep.ts b/frontend/src/utils/sleep.ts new file mode 100644 index 000000000..134e84c89 --- /dev/null +++ b/frontend/src/utils/sleep.ts @@ -0,0 +1,5 @@ +export function sleep(duration: number): Promise { + return new Promise((resolve) => { + setTimeout(resolve, duration); + }); +}