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 }} 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 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 70f7d63..6c03eb8 100644 --- 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 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": [ 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 20e5870..0033d56 100644 --- 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): InternalAxiosRequestConfig => { - 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 => { - 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 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); }); });