From 8e6a03ffe0f85666c4b970b567f2e4c182ef3941 Mon Sep 17 00:00:00 2001 From: Aravind Kumar Date: Fri, 17 May 2024 20:51:15 +0530 Subject: [PATCH 1/8] sca-scan.yml --- .github/workflows/sca-scan.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/sca-scan.yml b/.github/workflows/sca-scan.yml index bf9c1eb..f09161f 100644 --- a/.github/workflows/sca-scan.yml +++ b/.github/workflows/sca-scan.yml @@ -3,7 +3,7 @@ on: pull_request: types: [opened, synchronize, reopened] jobs: - security: + security-sca: runs-on: ubuntu-latest steps: - uses: actions/checkout@master From 0b72b833e71948a95e1976bd4a87f80f4ba5b3c2 Mon Sep 17 00:00:00 2001 From: Aravind Kumar Date: Fri, 17 May 2024 20:51:27 +0530 Subject: [PATCH 2/8] jira.yml --- .github/workflows/jira.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/jira.yml b/.github/workflows/jira.yml index 5ddf87a..caa4bbd 100644 --- a/.github/workflows/jira.yml +++ b/.github/workflows/jira.yml @@ -3,7 +3,7 @@ on: pull_request: types: [opened] jobs: - security: + security-jira: if: ${{ github.actor == 'dependabot[bot]' || github.actor == 'snyk-bot' || contains(github.event.pull_request.head.ref, 'snyk-fix-') || contains(github.event.pull_request.head.ref, 'snyk-upgrade-')}} runs-on: ubuntu-latest steps: @@ -26,3 +26,8 @@ jobs: PR: ${{ github.event.pull_request.html_url }} fields: "${{ secrets.JIRA_FIELDS }}" + - name: Transition issue + uses: atlassian/gajira-transition@v3 + with: + issue: ${{ steps.create.outputs.issue }} + transition: ${{ secrets.JIRA_TRANSITION }} From c6073e4726c3d5ecbb75b7ea4c1c6d5cfd93179f Mon Sep 17 00:00:00 2001 From: Aravind Kumar Date: Fri, 17 May 2024 20:51:28 +0530 Subject: [PATCH 3/8] sast-scan.yml --- .github/workflows/sast-scan.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .github/workflows/sast-scan.yml diff --git a/.github/workflows/sast-scan.yml b/.github/workflows/sast-scan.yml new file mode 100644 index 0000000..3b9521a --- /dev/null +++ b/.github/workflows/sast-scan.yml @@ -0,0 +1,11 @@ +name: SAST Scan +on: + pull_request: + types: [opened, synchronize, reopened] +jobs: + security-sast: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Semgrep Scan + run: docker run -v /var/run/docker.sock:/var/run/docker.sock -v "${PWD}:/src" returntocorp/semgrep semgrep scan --config auto \ No newline at end of file From 56567b206596a3e6f35e03ba57077c0f1914d789 Mon Sep 17 00:00:00 2001 From: Aravind Kumar Date: Fri, 17 May 2024 20:51:30 +0530 Subject: [PATCH 4/8] codeql-analysis.yml From feb5bfb1868c64af2815b95dc1b4923a6c2a312d Mon Sep 17 00:00:00 2001 From: harshithad0703 Date: Mon, 3 Jun 2024 20:55:09 +0530 Subject: [PATCH 5/8] 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 6/8] 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 7/8] 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 8/8] 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