From feb5bfb1868c64af2815b95dc1b4923a6c2a312d Mon Sep 17 00:00:00 2001 From: harshithad0703 Date: Mon, 3 Jun 2024 20:55:09 +0530 Subject: [PATCH 1/5] fix: retry response error handling --- src/lib/retryPolicy/delivery-sdk-handlers.ts | 84 +++++++++++--------- 1 file changed, 47 insertions(+), 37 deletions(-) diff --git a/src/lib/retryPolicy/delivery-sdk-handlers.ts b/src/lib/retryPolicy/delivery-sdk-handlers.ts index 20e5870..1945e00 100644 --- a/src/lib/retryPolicy/delivery-sdk-handlers.ts +++ b/src/lib/retryPolicy/delivery-sdk-handlers.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-throw-literal */ import axios, { InternalAxiosRequestConfig, AxiosResponse } from 'axios'; declare module 'axios' { @@ -21,52 +22,61 @@ export const retryRequestHandler = (req: InternalAxiosRequestConfig): Inter export const retryResponseHandler = (response: AxiosResponse) => response; -export const retryResponseErrorHandler = async (error: any, config: any): Promise => { - let retryCount = error.config.retryCount; - // let retryErrorType = null; +export const retryResponseErrorHandler = (error: any, config: any) => { + 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) { + throw error; + } + + setTimeout(() => { + error.config.retryCount = retryCount; + axios(error.request); + }, 1000); + + return; } - 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); + } - 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) => { let delayTime: number = retryDelay; if (retryCount > config.retryLimit) { From f69033acf7989bb77ae16b0fc879add37c14e3b8 Mon Sep 17 00:00:00 2001 From: harshithad0703 Date: Tue, 4 Jun 2024 10:30:58 +0530 Subject: [PATCH 2/5] fix: snyk upgrade and version bump --- CHANGELOG.md | 5 +++++ package-lock.json | 12 ++++++------ package.json | 4 ++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70f7d63..fa3ecff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## Change log + +### Version: 1.0.3 +#### Date: June-05-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 index b678f99..bf51dc7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,17 +1,17 @@ { "name": "@contentstack/core", - "version": "1.0.2", + "version": "1.0.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@contentstack/core", - "version": "1.0.2", + "version": "1.0.3", "dependencies": { "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" }, "devDependencies": { @@ -11559,9 +11559,9 @@ } }, "node_modules/qs": { - "version": "6.12.0", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.12.0.tgz", - "integrity": "sha512-trVZiI6RMOkO476zLGaBIzszOdFPnCCXHPG9kn0yuS1uz6xdVxPfZdB3vUig9pxPFDM9BRAgz/YUIVQ1/vuiUg==", + "version": "6.12.1", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.12.1.tgz", + "integrity": "sha512-zWmv4RSuB9r2mYQw3zxQuHWeU+42aKi1wWig/j4ele4ygELZ7PEO6MM7rim9oAQH2A5MWfsAVf/jPvTPgCbvUQ==", "dependencies": { "side-channel": "^1.0.6" }, diff --git a/package.json b/package.json index 2670b07..eb6f099 100644 --- 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": [ From 0fa51b4bc28b524857f29a57a340f31b3972f137 Mon Sep 17 00:00:00 2001 From: harshithad0703 Date: Tue, 2 Jul 2024 18:16:46 +0530 Subject: [PATCH 3/5] fix: handled retry error response and updated test cases --- src/lib/request.ts | 8 +-- src/lib/retryPolicy/delivery-sdk-handlers.ts | 25 ++++--- .../retryPolicy/delivery-sdk-handlers.spec.ts | 66 ++++++++++++++----- 3 files changed, 66 insertions(+), 33 deletions(-) diff --git a/src/lib/request.ts b/src/lib/request.ts index 8a759c2..2ca1bab 100644 --- 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 index 1945e00..0033d56 100644 --- a/src/lib/retryPolicy/delivery-sdk-handlers.ts +++ b/src/lib/retryPolicy/delivery-sdk-handlers.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-throw-literal */ -import axios, { InternalAxiosRequestConfig, AxiosResponse } from 'axios'; +import axios, { InternalAxiosRequestConfig, AxiosResponse, AxiosInstance } from 'axios'; declare module 'axios' { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -15,14 +15,14 @@ const defaultConfig = { }; export const retryRequestHandler = (req: InternalAxiosRequestConfig): InternalAxiosRequestConfig => { - req.retryCount = req.retryCount || 0; + req.retryCount = req.retryCount || 1; return req; }; export const retryResponseHandler = (response: AxiosResponse) => response; -export const retryResponseErrorHandler = (error: any, config: any) => { +export const retryResponseErrorHandler = (error: any, config: any, axiosInstance: AxiosInstance) => { try { let retryCount = error.config.retryCount; config = { ...defaultConfig, ...config }; @@ -46,22 +46,21 @@ export const retryResponseErrorHandler = (error: any, config: any) => { } else if (response.status == 429 || response.status == 401) { retryCount++; - if (retryCount > config.retryLimit) { - throw error; + if (retryCount >= config.retryLimit) { + if (error.response && error.response.data) { + return Promise.reject(error.response.data); + } + return Promise.reject(error); } - - setTimeout(() => { error.config.retryCount = retryCount; - axios(error.request); - }, 1000); - return; + return axiosInstance(error.config); } if (config.retryCondition && config.retryCondition(error)) { retryCount++; - return retry(error, config, retryCount, config.retryDelay); + return retry(error, config, retryCount, config.retryDelay, axiosInstance); } const customError = { @@ -77,7 +76,7 @@ export const retryResponseErrorHandler = (error: any, config: any) => { 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); @@ -88,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 index 1a54a9f..7aecc5c 100644 --- 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,22 +40,45 @@ 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 = { @@ -63,8 +86,9 @@ describe('retryResponseErrorHandler', () => { 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 = { @@ -72,8 +96,9 @@ describe('retryResponseErrorHandler', () => { 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); }); }); From 9aa63d52162d3631944c3e525b93743c4757b506 Mon Sep 17 00:00:00 2001 From: harshithad0703 Date: Tue, 2 Jul 2024 18:21:21 +0530 Subject: [PATCH 4/5] updated changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa3ecff..6c03eb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### Version: 1.0.3 -#### Date: June-05-2024 +#### Date: July-08-2024 - Fixed retry response error handling ### Version: 1.0.2 From e17f4f4708111859ba1cd18da085e944fd3badbf Mon Sep 17 00:00:00 2001 From: harshithad0703 Date: Mon, 15 Jul 2024 12:10:18 +0530 Subject: [PATCH 5/5] fix: fixed sre vulnerabilities in cleanup.js --- tools/cleanup.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/cleanup.js b/tools/cleanup.js index adf170b..557202f 100644 --- 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 {