Merge pull request #90 from paulo-tinoco/fix/retryConditional

feat: enhance retryCondition to handle undefined errors and improve s…

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;
   };
 
   /**
@@ -311,11 +315,11 @@ export class BaseClient implements BaseClientInterface {
    * @memberof BasicClient
    */
   retryCondition = (error: AxiosError | any): boolean => {
-    if (error.response.status === HttpStatusCodesRetryCondition.Unauthorized)
+    if (error?.response?.status === HttpStatusCodesRetryCondition.Unauthorized)
       this.retryAuth = true;
 
     return Object.values(HttpStatusCodesRetryCondition).includes(
-      error.response.status
+      error?.response?.status ?? HttpStatusCodesRetryCondition.RequestTimeout
     );
   };
 }

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 {

tests/base.test.ts

@@ -1,19 +1,22 @@
+import { AxiosError, AxiosHeaders } from 'axios';
+
 import {
   AuthProviderNotFound,
   BaseClient,
   EndpointNotSet,
+  HttpStatusCodesRetryCondition,
   InvalidAuthOptions
 } from '../src';
 
+const endpoints = {
+  create: '/users',
+  delete: '/users/{id}',
+  fetch: '/users/{id}',
+  searchAllPages: '/users',
+  search: '/users',
+  update: '/users/{id}'
+};
 describe('BaseClient', () => {
-  const endpoints = {
-    create: '/users',
-    delete: '/users/{id}',
-    fetch: '/users/{id}',
-    searchAllPages: '/users',
-    search: '/users',
-    update: '/users/{id}'
-  };
   let clientBasic: BaseClient;
   let clientBasicWithDefaultHeader: BaseClient;
 
@@ -236,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({
@@ -318,3 +311,126 @@ describe('BaseClient', () => {
     });
   });
 });
+describe('BaseClient retryCondition', () => {
+  let client: BaseClient;
+
+  beforeEach(() => {
+    const baseParams = { authProvider: null, endpoints };
+    client = new BaseClient(baseParams);
+  });
+
+  it('should not response if error is undefined', () => {
+    const result = client.retryCondition(undefined);
+    expect(result).toBe(true);
+  });
+
+  it('should set retryAuth to true if error status is Unauthorized', () => {
+    const error = {
+      response: { status: HttpStatusCodesRetryCondition.Unauthorized }
+    };
+    const result = client.retryCondition(error);
+    expect(client.retryAuth).toBe(true);
+    expect(result).toBe(true);
+  });
+
+  it('should return true if error status is in HttpStatusCodesRetryCondition', () => {
+    const error = {
+      response: { status: HttpStatusCodesRetryCondition.TooManyRequests }
+    };
+    const result = client.retryCondition(error);
+    expect(result).toBe(true);
+  });
+
+  it('should return false if error status is not in HttpStatusCodesRetryCondition', () => {
+    const error = { response: { status: 418 } }; // I'm a teapot
+    const result = client.retryCondition(error);
+    expect(result).toBe(false);
+  });
+
+  it('should return true if error status is undefined and default to RequestTimeout', () => {
+    const error = { response: {} };
+    const result = client.retryCondition(error);
+    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);
+  });
+});