Merge pull request #141 from contentstack/fix/DX-3529-retryLimit-retryDelay

Enhance retry logic with configurable limit and delay and add tests

Author: nadeem-cs

.talismanrc

@@ -3,11 +3,15 @@ fileignoreconfig:
   ignore_detectors:
     - filecontent
 - filename: package-lock.json
-  checksum: 52799bf1f9a1c387a74baeecac6c1c08f22bb8abdd2a1f0e689d8ed374b47635
+  checksum: 61066aedc29ef5bd8904d1ee2384dad828e8f9aab1a6b0360797ec7926e7f8dd
 - filename: .husky/pre-commit
   checksum: 5baabd7d2c391648163f9371f0e5e9484f8fb90fa2284cfc378732ec3192c193
 - filename: test/request.spec.ts
   checksum: 87afd3bb570fd52437404cbe69a39311ad8a8c73bca9d075ecf88652fd3e13f6
 - filename: src/lib/request.ts
   checksum: 86d761c4f50fcf377e52c98e0c4db6f06be955790fc5a0f2ba8fe32a88c60825
+- filename: src/lib/retryPolicy/delivery-sdk-handlers.ts
+  checksum: 08ccd6342b3adbeb7b85309a034b4df4b2ad905a0cc2a3778ab483b61ba41b9e
+- filename: test/retryPolicy/delivery-sdk-handlers.spec.ts
+  checksum: 6d22d7482aa6dccba5554ae497e5b0c3572357a5cead6f4822ee4428edc12207
 version: ""

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@contentstack/core",
-  "version": "1.3.1",
+  "version": "1.3.2",
   "type": "commonjs",
   "main": "./dist/cjs/src/index.js",
   "types": "./dist/cjs/src/index.d.ts",
@@ -20,12 +20,12 @@
     "husky-check": "npx husky install && chmod +x .husky/pre-commit"
   },
   "dependencies": {
-    "axios": "^1.11.0",
+    "axios": "^1.12.2",
     "axios-mock-adapter": "^2.1.0",
+    "husky": "^9.1.7",
     "lodash": "^4.17.21",
     "qs": "^6.14.0",
-    "tslib": "^2.8.1",
-    "husky": "^9.1.7"
+    "tslib": "^2.8.1"
   },
   "files": [
     "dist/*",

package.json

@@ -83,7 +83,17 @@ export const retryResponseErrorHandler = (error: any, config: any, axiosInstance
         }
         error.config.retryCount = retryCount;
 
-        return axiosInstance(error.config);
+        // Apply configured delay for retries
+        return new Promise((resolve, reject) => {
+          setTimeout(async () => {
+            try {
+              const retryResponse = await axiosInstance(error.config);
+              resolve(retryResponse);
+            } catch (retryError) {
+              reject(retryError);
+            }
+          }, config.retryDelay || 300); // Use configured delay with fallback
+        });
       }
     }
 
@@ -99,17 +109,22 @@ export const retryResponseErrorHandler = (error: any, config: any, axiosInstance
   }
 };
 const retry = (error: any, config: any, retryCount: number, retryDelay: number, axiosInstance: AxiosInstance) => {
-  let delayTime: number = retryDelay;
   if (retryCount > config.retryLimit) {
     return Promise.reject(error);
   }
 
-  delayTime = config.retryDelay;
+  // Use the passed retryDelay parameter first, then config.retryDelay, then default
+  const delayTime = retryDelay || config.retryDelay || 300;
   error.config.retryCount = retryCount;
 
-  return new Promise(function (resolve) {
-    return setTimeout(function () {
-      return resolve(axiosInstance(error.request));
+  return new Promise(function (resolve, reject) {
+    return setTimeout(async function () {
+      try {
+        const retryResponse = await axiosInstance(error.config);
+        resolve(retryResponse);
+      } catch (retryError) {
+        reject(retryError);
+      }
     }, delayTime);
   });
 };

src/lib/retryPolicy/delivery-sdk-handlers.ts

@@ -154,7 +154,14 @@ describe('retryResponseErrorHandler', () => {
   });
   it('should call the retry function if retryCondition is passed', async () => {
     const error = {
-      config: { retryOnError: true, retryCount: 4 },
+      config: { 
+        retryOnError: true, 
+        retryCount: 4,
+        method: 'post',
+        url: '/retryURL',
+        data: { key: 'value' },
+        headers: { 'Content-Type': 'application/json' }
+      },
       response: {
         status: 200,
         statusText: 'Success Response but retry needed',
@@ -231,6 +238,103 @@ describe('retryResponseErrorHandler', () => {
     expect(response.status).toBe(200);
   });
 
+  it('should use configured retryDelay for 429 status retries', async () => {
+    const error = {
+      config: { retryOnError: true, retryCount: 1 },
+      response: {
+        status: 429,
+        statusText: 'Rate limit exceeded',
+        headers: {},
+        data: {
+          error_message: 'Rate limit exceeded',
+          error_code: 429,
+          errors: null,
+        },
+      },
+    };
+    const config = { retryLimit: 3, retryDelay: 500 };
+    const client = axios.create();
+
+    mock.onAny().reply(200, { success: true });
+
+    jest.useFakeTimers();
+    const startTime = Date.now();
+
+    const responsePromise = retryResponseErrorHandler(error, config, client);
+
+    // Fast-forward time by the configured delay
+    jest.advanceTimersByTime(500);
+
+    const response = (await responsePromise) as AxiosResponse;
+    expect(response.status).toBe(200);
+
+    jest.useRealTimers();
+  });
+
+  it('should use configured retryDelay for 401 status retries', async () => {
+    const error = {
+      config: { retryOnError: true, retryCount: 1 },
+      response: {
+        status: 401,
+        statusText: 'Unauthorized',
+        headers: {},
+        data: {
+          error_message: 'Unauthorized',
+          error_code: 401,
+          errors: null,
+        },
+      },
+    };
+    const config = { retryLimit: 3, retryDelay: 250 };
+    const client = axios.create();
+
+    mock.onAny().reply(200, { success: true });
+
+    jest.useFakeTimers();
+
+    const responsePromise = retryResponseErrorHandler(error, config, client);
+
+    // Fast-forward time by the configured delay
+    jest.advanceTimersByTime(250);
+
+    const response = (await responsePromise) as AxiosResponse;
+    expect(response.status).toBe(200);
+
+    jest.useRealTimers();
+  });
+
+  it('should use default retryDelay (300ms) when not configured for 429 retries', async () => {
+    const error = {
+      config: { retryOnError: true, retryCount: 1 },
+      response: {
+        status: 429,
+        statusText: 'Rate limit exceeded',
+        headers: {},
+        data: {
+          error_message: 'Rate limit exceeded',
+          error_code: 429,
+          errors: null,
+        },
+      },
+    };
+    const config = { retryLimit: 3 }; // No retryDelay specified
+    const client = axios.create();
+
+    mock.onAny().reply(200, { success: true });
+
+    jest.useFakeTimers();
+
+    const responsePromise = retryResponseErrorHandler(error, config, client);
+
+    // Fast-forward time by the default delay (300ms)
+    jest.advanceTimersByTime(300);
+
+    const response = (await responsePromise) as AxiosResponse;
+    expect(response.status).toBe(200);
+
+    jest.useRealTimers();
+  });
+
   it('should retry when retryCondition is true', async () => {
     const error = {
       config: { retryOnError: true, retryCount: 1 },
@@ -256,6 +360,40 @@ describe('retryResponseErrorHandler', () => {
     expect(retryCondition).toHaveBeenCalledWith(error);
   });
 
+  it('should use configured retryDelay when retryCondition triggers retry', async () => {
+    const error = {
+      config: { retryOnError: true, retryCount: 1 },
+      response: {
+        status: 500,
+        statusText: 'Internal Server Error',
+        headers: {},
+        data: {
+          error_message: 'Internal Server Error',
+          error_code: 500,
+          errors: null,
+        },
+      },
+    };
+    const retryCondition = jest.fn().mockReturnValue(true);
+    const config = { retryLimit: 3, retryCondition, retryDelay: 750 };
+    const client = axios.create();
+
+    mock.onAny().reply(200, { success: true });
+
+    jest.useFakeTimers();
+
+    const responsePromise = retryResponseErrorHandler(error, config, client);
+
+    // Fast-forward time by the configured delay
+    jest.advanceTimersByTime(750);
+
+    const response = (await responsePromise) as AxiosResponse;
+    expect(response.status).toBe(200);
+    expect(retryCondition).toHaveBeenCalledWith(error);
+
+    jest.useRealTimers();
+  });
+
   it('should retry with delay when x-ratelimit-remaining is 0 and retry-after header is present', async () => {
     const error = {
       config: { retryOnError: true, retryCount: 1 },