Merge pull request #42 from contentstack/next

harshithad0703 · web-flow · commit 0a9e84daa42f · 2024-07-19T14:07:52.000+05:30
DX | 05-07-2024 | Hotfix
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 ## Change log
 
+
+### Version: 1.0.3
+#### Date: July-08-2024
+ - Fixed retry response error handling
+
 ### Version: 1.0.2
 #### Date: April-02-2024
  - Update dependency packages
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@contentstack/core",
-  "version": "1.0.2",
+  "version": "1.0.3",
   "type": "commonjs",
   "main": "./dist/cjs/src/index.js",
   "types": "./dist/cjs/src/index.d.ts",
@@ -21,7 +21,7 @@
     "axios": "^1.6.8",
     "axios-mock-adapter": "^1.22.0",
     "lodash": "^4.17.21",
-    "qs": "^6.12.0",
+    "qs": "^6.12.1",
     "tslib": "^2.6.2"
   },
   "files": [
diff --git a/src/lib/request.ts b/src/lib/request.ts
@@ -1,14 +1,14 @@
-import { AxiosInstance } from './types';
+import { Axios } from 'axios';
 
-export async function getData(instance: AxiosInstance, url: string, data?: any) {
+export async function getData(instance: Axios, url: string, data?: any) {
   try {
     const response = await instance.get(url, { params: data });
-    if (response.data) {
+    if (response && response.data) {
       return response.data;
     } else {
       throw Error(JSON.stringify(response));
     }
   } catch (err) {
-    throw err;
+    throw Error(JSON.stringify(err));
   }
 }
diff --git a/src/lib/retryPolicy/delivery-sdk-handlers.ts b/src/lib/retryPolicy/delivery-sdk-handlers.ts
@@ -1,4 +1,5 @@
-import axios, { InternalAxiosRequestConfig, AxiosResponse } from 'axios';
+/* eslint-disable @typescript-eslint/no-throw-literal */
+import axios, { InternalAxiosRequestConfig, AxiosResponse, AxiosInstance } from 'axios';
 
 declare module 'axios' {
   // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -14,60 +15,68 @@ const defaultConfig = {
 };
 
 export const retryRequestHandler = (req: InternalAxiosRequestConfig<any>): InternalAxiosRequestConfig<any> => {
-  req.retryCount = req.retryCount || 0;
+  req.retryCount = req.retryCount || 1;
 
   return req;
 };
 
 export const retryResponseHandler = (response: AxiosResponse) => response;
 
-export const retryResponseErrorHandler = async (error: any, config: any): Promise<any> => {
-  let retryCount = error.config.retryCount;
-  // let retryErrorType = null;
+export const retryResponseErrorHandler = (error: any, config: any, axiosInstance: AxiosInstance) => {
+  try {
+    let retryCount = error.config.retryCount;
+    config = { ...defaultConfig, ...config };
 
-  config = { ...defaultConfig, ...config };
-
-  if (!error.config.retryOnError || retryCount > config.retryLimit) {
-    return Promise.reject(error);
-  }
-
-  const response = error.response;
-  if (!response) {
-    if (error.code === 'ECONNABORTED') {
-      error.response = {
-        ...error.response,
-        status: 408,
-        statusText: `timeout of ${config.timeout}ms exceeded`,
-      };
-
-      return Promise.resolve(error.response);
-    } else {
-      return Promise.reject(error);
+    if (!error.config.retryOnError || retryCount > config.retryLimit) {
+      throw error;
     }
-  } else if (response.status == 429 || response.status == 401) {
-    retryCount++;
-    // retryErrorType = `Error with status: ${response.status}`;
 
-    if (retryCount > config.retryLimit) {
-      return Promise.reject(error);
+    const response = error.response;
+    if (!response) {
+      if (error.code === 'ECONNABORTED') {
+        const customError = {
+          error_message: `Timeout of ${config.timeout}ms exceeded`,
+          error_code: 408,
+          errors: null,
+        };
+        throw customError; // Throw customError object
+      } else {
+        throw error;
+      }
+    } else if (response.status == 429 || response.status == 401) {
+      retryCount++;
+
+      if (retryCount >= config.retryLimit) {
+        if (error.response && error.response.data) {
+          return Promise.reject(error.response.data);
+        }
+        return Promise.reject(error);
+      }
+        error.config.retryCount = retryCount;
+
+      return axiosInstance(error.config);
     }
 
-    await new Promise((resolve) => setTimeout(resolve, 1000));
-
-    error.config.retryCount = retryCount;
+    if (config.retryCondition && config.retryCondition(error)) {
+      retryCount++;
 
-    return axios(error.request);
-  }
-
-  if (config.retryCondition && config.retryCondition(error)) {
-    // retryErrorType = error.response ? `Error with status: ${response.status}` : `Error Code:${error.code}`;
-    retryCount++;
+      return retry(error, config, retryCount, config.retryDelay, axiosInstance);
+    }
 
-    return retry(error, config, retryCount, config.retryDelay);
+    const customError = {
+      status: response.status,
+      statusText: response.statusText,
+      error_message: response.data.error_message,
+      error_code: response.data.error_code,
+      errors: response.data.errors,
+    };
+
+    throw customError;
+  } catch (err) {
+    throw err;
   }
 };
-
-const retry = (error: any, config: any, retryCount: number, retryDelay: number) => {
+const retry = (error: any, config: any, retryCount: number, retryDelay: number, axiosInstance: AxiosInstance) => {
   let delayTime: number = retryDelay;
   if (retryCount > config.retryLimit) {
     return Promise.reject(error);
@@ -78,7 +87,7 @@ const retry = (error: any, config: any, retryCount: number, retryDelay: number)
 
   return new Promise(function (resolve) {
     return setTimeout(function () {
-      return resolve(axios(error.request));
+      return resolve(axiosInstance(error.request));
     }, delayTime);
   });
 };
diff --git a/test/retryPolicy/delivery-sdk-handlers.spec.ts b/test/retryPolicy/delivery-sdk-handlers.spec.ts
@@ -12,7 +12,7 @@ describe('retryRequestHandler', () => {
     const requestConfig: InternalAxiosRequestConfig = { headers: {} as AxiosHeaders };
     const updatedConfig = retryRequestHandler(requestConfig);
 
-    expect(updatedConfig.retryCount).toBe(0);
+    expect(updatedConfig.retryCount).toBe(1);
   });
 });
 
@@ -40,40 +40,65 @@ describe('retryResponseErrorHandler', () => {
   it('should reject the promise if retryOnError is false', async () => {
     const error = { config: { retryOnError: false }, code: 'ECONNABORTED' };
     const config = { retryLimit: 5 };
-
-    await expect(retryResponseErrorHandler(error, config)).rejects.toBe(error);
+    const client = axios.create();
+
+    try {
+      await retryResponseErrorHandler(error, config, client);
+      fail('Expected retryResponseErrorHandler to throw an error');
+    } catch (err) {
+      expect(err).toEqual(expect.objectContaining({
+        code: 'ECONNABORTED',
+        config: expect.objectContaining({ retryOnError: false }),
+      }));
+    }
   });
   it('should reject the promise if retryOnError is true', async () => {
     const error = { config: { retryOnError: true } };
     const config = { retryLimit: 5 };
-
-    await expect(retryResponseErrorHandler(error, config)).rejects.toBe(error);
+    const client = axios.create();
+  
+    try {
+      await retryResponseErrorHandler(error, config, client);
+      fail('Expected retryResponseErrorHandler to throw an error');
+    } catch (err: any) {
+      expect(err.config).toEqual(expect.objectContaining({ retryOnError: true }));
+      expect(err).toEqual(error);
+    }
   });
   it('should resolve the promise to 408 error if retryOnError is true and error code is ECONNABORTED', async () => {
     const error = { config: { retryOnError: true, retryCount: 1 }, code: 'ECONNABORTED' };
     const config = { retryLimit: 5, timeout: 1000 };
-
-    const errorResponse = { status: 408, statusText: 'timeout of 1000ms exceeded' };
-
-    await expect(retryResponseErrorHandler(error, config)).resolves.toEqual(errorResponse);
+    const client = axios.create();
+    try {
+      await retryResponseErrorHandler(error, config, client);
+      fail('Expected retryResponseErrorHandler to throw an error');
+    } catch (err) {
+      expect(err).toEqual(expect.objectContaining({
+        error_code: 408,
+        error_message: `Timeout of ${config.timeout}ms exceeded`,
+        errors: null
+      }));
+    }  
   });
   it('should reject the promise if response status is 429 and retryCount exceeds retryLimit', async () => {
     const error = {
       config: { retryOnError: true, retryCount: 5 },
       response: { status: 429, statusText: 'timeout of 1000ms exceeded' },
     };
     const config = { retryLimit: 5, timeout: 1000 };
+    const client = axios.create();
 
-    await expect(retryResponseErrorHandler(error, config)).rejects.toBe(error);
+    await expect(retryResponseErrorHandler(error, config, client)).rejects.toBe(error);
   });
   it('should reject the promise if response status is 401 and retryCount exceeds retryLimit', async () => {
     const error = {
       config: { retryOnError: true, retryCount: 5 },
       response: { status: 401, statusText: 'timeout of 1000ms exceeded' },
     };
     const config = { retryLimit: 5, timeout: 1000 };
+    const client = axios.create();
 
-    await expect(retryResponseErrorHandler(error, config)).rejects.toBe(error);
+    await expect(retryResponseErrorHandler(error, config, client)).rejects.toBe(error);
   });
   it('should reject the promise if response status is 429 or 401 and retryCount is within limit', async () => {
     const error = {
@@ -87,17 +112,24 @@ describe('retryResponseErrorHandler', () => {
       },
     };
     const config = { retryLimit: 5, timeout: 1000 };
+    const client = axios.create();
 
     const finalResponseObj = {
-      config: { retryOnError: true, retryCount: 5 },
+      config: { retryOnError: true, retryCount: 4 },
       response: { status: 429, statusText: 'timeout of 1000ms exceeded' },
     };
 
     mock.onPost('/retryURL').reply(200, finalResponseObj);
 
-    const finalResponse = await retryResponseErrorHandler(error, config);
+    try {
+      await retryResponseErrorHandler(error, config, client);
+      throw new Error('Expected retryResponseErrorHandler to throw an error');
+    } catch (err: any) {
+      expect(err.response.status).toBe(429);
+      expect(err.response.statusText).toBe(error.response.statusText);
+      expect(err.config.retryCount).toBe(error.config.retryCount);
+    }
 
-    expect(finalResponse.data).toEqual(finalResponseObj);
   });
   it('should call the retry function if retryCondition is passed', async () => {
     const error = {
@@ -113,6 +145,7 @@ describe('retryResponseErrorHandler', () => {
     // eslint-disable-next-line @typescript-eslint/no-shadow
     const retryCondition = (error: any) => true;
     const config = { retryLimit: 5, timeout: 1000, retryCondition: retryCondition };
+    const client = axios.create();
 
     const finalResponseObj = {
       config: { retryOnError: true, retryCount: 5 },
@@ -121,7 +154,7 @@ describe('retryResponseErrorHandler', () => {
 
     mock.onPost('/retryURL').reply(200, finalResponseObj);
 
-    const finalResponse = await retryResponseErrorHandler(error, config);
+    const finalResponse: any = await retryResponseErrorHandler(error, config, client);
 
     expect(finalResponse.data).toEqual(finalResponseObj);
   });
@@ -139,6 +172,7 @@ describe('retryResponseErrorHandler', () => {
     // eslint-disable-next-line @typescript-eslint/no-shadow
     const retryCondition = (error: any) => true;
     const config = { retryLimit: 5, timeout: 1000, retryCondition: retryCondition };
+    const client = axios.create();
 
     const finalResponseObj = {
       config: { retryOnError: true, retryCount: 5 },
@@ -147,6 +181,6 @@ describe('retryResponseErrorHandler', () => {
 
     mock.onPost('/retryURL').reply(200, finalResponseObj);
 
-    await expect(retryResponseErrorHandler(error, config)).rejects.toBe(error);
+    await expect(retryResponseErrorHandler(error, config, client)).rejects.toBe(error);
   });
 });
diff --git a/tools/cleanup.js b/tools/cleanup.js
@@ -3,10 +3,16 @@ const fs = require('fs');
 const Path = require('path');
 /* eslint-enable */
 
-const deleteFolderRecursive = (path) => {
+const sanitizePath = (inputPath) => {
+  return Path.normalize(inputPath).replace(/^(\.\.(\/|\\|$))+/, '');
+};
+
+const deleteFolderRecursive = (inputPath) => {
+  const path = sanitizePath(inputPath);
+  
   if (fs.existsSync(path)) {
     fs.readdirSync(path).forEach((file) => {
-      const curPath = Path.join(path, file);
+      const curPath = Path.join(path, sanitizePath(file));
       if (fs.lstatSync(curPath).isDirectory()) {
         deleteFolderRecursive(curPath);
       } else {

Original file line number	Diff line number	Diff line change
`@@ -1,14 +1,14 @@`
`1`		`-import { AxiosInstance } from './types';`
	`1`	`+import { Axios } from 'axios';`
`2`	`2`
`3`		`-export async function getData(instance: AxiosInstance, url: string, data?: any) {`
	`3`	`+export async function getData(instance: Axios, url: string, data?: any) {`
`4`	`4`	`try {`
`5`	`5`	`const response = await instance.get(url, { params: data });`
`6`		`- if (response.data) {`
	`6`	`+ if (response && response.data) {`
`7`	`7`	`return response.data;`
`8`	`8`	`} else {`
`9`	`9`	`throw Error(JSON.stringify(response));`
`10`	`10`	`}`
`11`	`11`	`} catch (err) {`
`12`		`- throw err;`
	`12`	`+ throw Error(JSON.stringify(err));`
`13`	`13`	`}`
`14`	`14`	`}`