From 197fac82ee51b55e065028c437b8db4398be3ded Mon Sep 17 00:00:00 2001 From: Paulo Tinoco Date: Tue, 12 Nov 2024 14:05:15 -0300 Subject: [PATCH] feat: enhance retryDelay method to improve error handling and support rate limit delays --- src/base.ts | 38 +++++++++--------- tests/axios.test.ts | 77 ------------------------------------- tests/base.test.ts | 93 ++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 104 insertions(+), 104 deletions(-) diff --git a/src/base.ts b/src/base.ts index 8e3c746..3de28e9 100644 --- a/src/base.ts +++ b/src/base.ts @@ -281,24 +281,28 @@ export class BaseClient implements BaseClientInterface { } /** - * The function to use for retry delay - * @description - * If the error is a 429, it will use the value of the rate limit key - * Otherwise, it will use the retry delay from the client options - * @param {number} _retryCount The number of retries - * @param {AxiosError} error The error that was thrown - * @returns {number} The number of milliseconds to wait before retrying - * @memberof BasicClient - * @throws {AxiosError} - * @throws {InvalidAuthOptions} + * Calculates the retry delay based on the retry count and error. + * If the retry count exceeds the maximum tries, it throws the error. + * If the error status is 429 (Too Many Requests), it uses the rate limit key value. + * Otherwise, it uses the retry delay from the client options. + * @param {number} retryCount The number of retries attempted. + * @param {AxiosError} error The error that was thrown. + * @returns {number} The number of milliseconds to wait before retrying. + * @throws {AxiosError} Throws the error if the retry count exceeds the maximum tries. */ retryDelay = (retryCount: number, error: AxiosError | any): number => { - if (retryCount >= this.clientOptions.tries) - throw error?.response ? error.response : error; - return error?.response?.status === - HttpStatusCodesRetryCondition.TooManyRequests - ? parseInt(error.response.headers[this.clientOptions.rateLimitKey]) - : this.clientOptions.retryDelay * 1000; + if (retryCount >= this.clientOptions.tries) throw error; + if ( + error?.response?.status === HttpStatusCodesRetryCondition.TooManyRequests + ) { + const rateLimitDelay = parseInt( + error.response.headers[this.clientOptions.rateLimitKey] + ); + if (!isNaN(rateLimitDelay)) { + return rateLimitDelay * 1000; + } + } + return this.clientOptions.retryDelay * 1000; }; /** @@ -315,7 +319,7 @@ export class BaseClient implements BaseClientInterface { this.retryAuth = true; return Object.values(HttpStatusCodesRetryCondition).includes( - error?.response?.status || HttpStatusCodesRetryCondition.RequestTimeout + error?.response?.status ?? HttpStatusCodesRetryCondition.RequestTimeout ); }; } diff --git a/tests/axios.test.ts b/tests/axios.test.ts index fbcc62c..c581634 100644 --- a/tests/axios.test.ts +++ b/tests/axios.test.ts @@ -5,18 +5,9 @@ import { AuthProviderNotFound, AxiosClient, EndpointNotSet, - HttpStatusCodesRetryCondition, - HtttpStatusCodeError, InvalidAuthOptions } from '../src'; -const statusCodeRetry = Object.values(HttpStatusCodesRetryCondition).filter( - (code) => typeof code === 'number' -); -const statusCodeError = Object.values(HtttpStatusCodeError).filter( - (code) => typeof code === 'number' -); - const endpoints = { create: '/users', delete: '/users/{id}', @@ -104,74 +95,6 @@ describe('AxiosClient', () => { expect(response.status).toEqual(status); }); - it.each(statusCodeRetry)( - 'should return true when calling retryCondition %i', - async (code) => { - const axiosError = { - config: {}, - response: { - status: code - } - } as AxiosError; - expect(clientBasic.retryCondition(axiosError)).toBeTruthy(); - if (code === 401) { - expect(await clientBasic.retryAuth).toBeTruthy(); - } - } - ); - - it.each(statusCodeError)( - 'should return false when calling retryCondition %i', - async (code) => { - const axiosError = { - config: {}, - response: { - status: code - } - } as AxiosError; - expect(clientBasic.retryCondition(axiosError)).toBeFalsy(); - } - ); - - it.each([ - ['too many request', 429, 10], - ['other', 500, 3000] - ])( - 'should return %s delay value when calling retryDelay', - (_type, code, expected) => { - const axiosError = { - message: 'Too Many Requests', - code: 'TOO_MANY_REQUESTS', - config: {}, - response: { - data: { - message: 'Too Many Requests' - }, - status: code, - statusText: 'Too Many Requests', - headers: { 'Retry-After': '10' } as object - } - } as AxiosError; - const response = clientBasic.retryDelay(1, axiosError); - expect(response).toEqual(expected); - } - ); - - it.each([2, 3, 4])( - 'should throw an AxiosError when calling retryDelay with tries equal %i and clientOptions tries equal 3', - (tries) => { - clientBasic.clientOptions.tries = 3; - const axiosError = new AxiosError(); - try { - clientBasic.retryDelay(tries, axiosError); - } catch (error) { - if (tries !== 2) { - expect(error).toBeInstanceOf(AxiosError); - } - } - } - ); - it('should throw an AuthProviderNotFound when calling authentication and authProvider is null', async () => { clientBasic.clientOptions.authProvider = null; try { diff --git a/tests/base.test.ts b/tests/base.test.ts index 867da51..1112eeb 100644 --- a/tests/base.test.ts +++ b/tests/base.test.ts @@ -1,3 +1,5 @@ +import { AxiosError, AxiosHeaders } from 'axios'; + import { AuthProviderNotFound, BaseClient, @@ -237,16 +239,6 @@ describe('BaseClient', () => { expect(result.dataFetched).toEqual(['someData', 'someData2']); }); - it('should throw with error response if exists on retryDelay', async () => { - try { - clientBasic.retryDelay(1, { - response: { status: 500, message: 'requestError' } - }); - } catch (error) { - expect(error).toEqual({ status: 500, message: 'requestError' }); - } - }); - it('should call fetch with default header', async () => { await clientBasicWithDefaultHeader.fetch('1'); expect(clientBasicWithDefaultHeader.makeRequest).toHaveBeenCalledWith({ @@ -361,3 +353,84 @@ describe('BaseClient retryCondition', () => { expect(result).toBe(true); }); }); + +describe('BaseClient retryDelay', () => { + let client: BaseClient; + let axiosError: AxiosError; + const axiosResponse = { + status: HttpStatusCodesRetryCondition.TooManyRequests, + statusText: 'Too Many Requests', + headers: new AxiosHeaders({ + 'retry-after': 3 + }), + data: {}, + config: { + headers: new AxiosHeaders({ + 'retry-after': 3 + }) + } + }; + + beforeEach(() => { + const baseParams = { authProvider: null, endpoints }; + client = new BaseClient(baseParams); + axiosError = new AxiosError( + 'Request failed with status code 500', + '500', + undefined, + undefined, + axiosResponse + ); + }); + + it('should throw error if retryCount exceeds tries', async () => { + client.clientOptions.tries = 3; + try { + await client.retryDelay(3, axiosError); + } catch (error) { + expect(error).toEqual(axiosError); + } + }); + + it('should throw error with not AxiosError', async () => { + client.clientOptions.tries = 3; + const throwError = new Error('error'); + try { + await client.retryDelay(3, throwError); + } catch (error) { + expect(error).toEqual(throwError); + } + }); + + it('should return rate limit delay if error status is TooManyRequests', () => { + client.clientOptions.tries = 3; + client.clientOptions.rateLimitKey = 'retry-after'; + expect(client.retryDelay(1, axiosError)).toBe(3000); + }); + + it('should return retryDelay if error status is TooManyRequests but no rateLimitKey', () => { + client.clientOptions.tries = 3; + client.clientOptions.rateLimitKey = 'other-key'; + client.clientOptions.retryDelay = 5; + expect(client.retryDelay(1, axiosError)).toBe(5000); + }); + + it('should return retryDelay if error status is not TooManyRequests', () => { + client.clientOptions.tries = 3; + if (axiosError.response) { + axiosError.response.status = 500; + } + expect(client.retryDelay(1, axiosError)).toBe(3000); + }); + + it('should throw error if retryCount exceeds tries with non-AxiosError', () => { + client.clientOptions.tries = 3; + const error = new Error('error'); + expect(() => client.retryDelay(3, error)).toThrow(error); + }); + + it('should return retryDelay if error is undefined', () => { + client.clientOptions.tries = 3; + expect(client.retryDelay(1, undefined)).toBe(3000); + }); +});