From 5abd8cadccff6138977e723f443c5164e2dba512 Mon Sep 17 00:00:00 2001 From: Zsombor Margetan Date: Thu, 6 Apr 2023 14:09:13 +0200 Subject: [PATCH] feat(wrapper): restructure request error handling EME-6117 Co-authored-by: Gabor Soos BREAKING CHANGE: error response data is always string --- package.json | 1 + src/wrapper.spec.ts | 456 ++++++++++++++++++++++---------------------- src/wrapper.ts | 45 +++-- 3 files changed, 256 insertions(+), 246 deletions(-) diff --git a/package.json b/package.json index dc3853e..96fbcc2 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,7 @@ "eslint-plugin-no-only-tests": "3.0.0", "eslint-plugin-security": "1.5.0", "mocha": "10.0.0", + "nock": "13.3.0", "semantic-release": "19.0.5", "sinon": "14.0.0", "sinon-chai": "3.7.0", diff --git a/src/wrapper.spec.ts b/src/wrapper.spec.ts index e52fc0e..bf9bdbe 100644 --- a/src/wrapper.spec.ts +++ b/src/wrapper.spec.ts @@ -6,302 +6,308 @@ import { expect } from 'chai'; import { ExtendedRequestOption, RequestWrapper } from './wrapper'; import { EscherRequestError } from './requestError'; import { AxiosRequestConfig } from 'axios'; +import nock from 'nock'; describe('RequestWrapper', function() { - let apiResponse: any; - let expectedApiResponse: any; - let extendedRequestOptions: ExtendedRequestOption; - let expectedRequestOptions: AxiosRequestConfig; - let cancelToken: CancelToken; - - beforeEach(function() { - apiResponse = { - headers: { 'content-type': 'application/json' }, - data: JSON.stringify({ data: 1 }), - status: 200, - statusText: 'OK' - }; - - expectedApiResponse = { - headers: { 'content-type': 'application/json' }, - body: { data: 1 }, - statusCode: 200, - statusMessage: 'OK' - }; - - extendedRequestOptions = { - secure: true, - port: 443, - host: 'very.host.io', - rejectUnauthorized: true, - headers: [ - ['content-type', 'very-format'], - ['x-custom', 'alma'] - ], - prefix: '', - timeout: 15000, - allowEmptyResponse: false, - maxContentLength: 10485760, - keepAlive: false, - credentialScope: '', - method: 'GET', - url: 'http://very.host.io:443/purchases/1/content', - path: '/purchases/1/content' - }; - - const CancelToken = axios.CancelToken; - const source = CancelToken.source(); - cancelToken = source.token; - - expectedRequestOptions = { - method: 'get', - url: 'http://very.host.io:443/purchases/1/content', - data: undefined, - headers: { - 'content-type': 'very-format', - 'x-custom': 'alma' - }, - timeout: 15000, - maxContentLength: 10485760, - cancelToken: cancelToken - }; - }); - - describe('request handling', function() { - let wrapper: RequestWrapper; - let requestGetStub: SinonStub; - let source: any; + describe('functionality tests', () => { + let apiResponse: any; + let expectedApiResponse: any; + let extendedRequestOptions: ExtendedRequestOption; + let expectedRequestOptions: AxiosRequestConfig; + let cancelToken: CancelToken; beforeEach(function() { - const instanceStub = axios.create(); - sinon.stub(axios, 'create').returns(instanceStub); - requestGetStub = sinon.stub(instanceStub, 'request'); - requestGetStub.resolves(apiResponse); - source = { - token: cancelToken, - cancel: sinon.stub() + apiResponse = { + headers: { 'content-type': 'application/json' }, + data: JSON.stringify({ data: 1 }), + status: 200, + statusText: 'OK' }; - sinon.stub(axios.CancelToken, 'source').returns(source); - - wrapper = new RequestWrapper(extendedRequestOptions, 'http:'); - }); - - it('should send GET request and return its response', async () => { - const response = await wrapper.send(); - - expect(response).to.be.eql(expectedApiResponse); - const requestArgument = requestGetStub.args[0][0]; - expect(requestArgument).to.containSubset(expectedRequestOptions); - expect(requestArgument).not.to.have.own.property('httpAgent'); - expect(requestArgument).not.to.have.own.property('httpsAgent'); - }); - it('should pass http agents to axios', async () => { - const agents = { - httpAgent: new http.Agent({ keepAlive: true }), - httpsAgent: new https.Agent({ keepAlive: true }) + expectedApiResponse = { + headers: { 'content-type': 'application/json' }, + body: { data: 1 }, + statusCode: 200, + statusMessage: 'OK' }; - wrapper = new RequestWrapper( - Object.assign(agents, extendedRequestOptions), - 'http:' - ); - await wrapper.send(); + extendedRequestOptions = { + secure: true, + port: 443, + host: 'very.host.io', + rejectUnauthorized: true, + headers: [ + ['content-type', 'very-format'], + ['x-custom', 'alma'] + ], + prefix: '', + timeout: 15000, + allowEmptyResponse: false, + maxContentLength: 10485760, + keepAlive: false, + credentialScope: '', + method: 'GET', + url: 'http://very.host.io:443/purchases/1/content', + path: '/purchases/1/content' + }; - const requestArgument = requestGetStub.args[0][0]; - expect(requestArgument.httpAgent).to.eql(agents.httpAgent); - expect(requestArgument.httpsAgent).to.eql(agents.httpsAgent); + const CancelToken = axios.CancelToken; + const source = CancelToken.source(); + cancelToken = source.token; + + expectedRequestOptions = { + method: 'get', + url: 'http://very.host.io:443/purchases/1/content', + data: undefined, + headers: { + 'content-type': 'very-format', + 'x-custom': 'alma' + }, + timeout: 15000, + maxContentLength: 10485760, + cancelToken: cancelToken + }; }); - it('should throw error when response code is 400 or above', async () => { - apiResponse.status = 400; - apiResponse.data = JSON.stringify({ replyText: 'Unknown route' }); - - try { - await wrapper.send(); - throw new Error('Error should have been thrown'); - } catch (err) { - const error = err as EscherRequestError; - expect(error).to.be.an.instanceof(EscherRequestError); - expect(error.message).to.eql('Error in http response (status: 400)'); - expect(error.code).to.eql(400); - expect(error.data).to.eql({ replyText: 'Unknown route' }); - } - }); + describe('request handling', function() { + let wrapper: RequestWrapper; + let requestGetStub: SinonStub; + let source: any; - describe('when empty response is allowed', function() { beforeEach(function() { - extendedRequestOptions.allowEmptyResponse = true; + const instanceStub = axios.create(); + sinon.stub(axios, 'create').returns(instanceStub); + requestGetStub = sinon.stub(instanceStub, 'request'); + requestGetStub.resolves(apiResponse); + source = { + token: cancelToken, + cancel: sinon.stub() + }; + sinon.stub(axios.CancelToken, 'source').returns(source); + wrapper = new RequestWrapper(extendedRequestOptions, 'http:'); }); - - it('should allow body to be empty', async () => { - apiResponse.headers['content-type'] = 'text/html'; - apiResponse.data = ''; - apiResponse.status = 204; - + it('should send GET request and return its response', async () => { const response = await wrapper.send(); - expect(response.statusCode).to.eql(204); + expect(response).to.be.eql(expectedApiResponse); + const requestArgument = requestGetStub.args[0][0]; + expect(requestArgument).to.containSubset(expectedRequestOptions); + expect(requestArgument).not.to.have.own.property('httpAgent'); + expect(requestArgument).not.to.have.own.property('httpsAgent'); }); + it('should pass http agents to axios', async () => { + const agents = { + httpAgent: new http.Agent({ keepAlive: true }), + httpsAgent: new https.Agent({ keepAlive: true }) + }; + wrapper = new RequestWrapper( + Object.assign(agents, extendedRequestOptions), + 'http:' + ); - it('should throw error if json is not parsable (empty)', async () => { - apiResponse.data = ''; + await wrapper.send(); - try { - await wrapper.send(); - } catch (err) { - const error = err as EscherRequestError; - expect(error).to.be.an.instanceof(EscherRequestError); - expect(error.message).to.match(/Unexpected end/); - expect(error.code).to.eql(500); - return; - } - throw new Error('Error should have been thrown'); + const requestArgument = requestGetStub.args[0][0]; + expect(requestArgument.httpAgent).to.eql(agents.httpAgent); + expect(requestArgument.httpsAgent).to.eql(agents.httpsAgent); }); - }); - describe('when empty response is not allowed', function() { - it('should throw error if response body is empty', async () => { - apiResponse.data = ''; + it('should throw error when response code is 400 or above', async () => { + requestGetStub.restore(); + nock('http://very.host.io:443') + .get('/purchases/1/content') + .reply(400, { replyText: 'Unknown route' }); try { await wrapper.send(); + throw new Error('Error should have been thrown'); } catch (err) { const error = err as EscherRequestError; expect(error).to.be.an.instanceof(EscherRequestError); - expect(error.message).to.eql('Empty http response'); - expect(error.code).to.eql(500); - expect(error.data).to.eql(expectedApiResponse.statusMessage); - return; + expect(error.message).to.eql('Error in http response (status: 400)'); + expect(error.code).to.eql(400); + expect(error.data).to.eql(JSON.stringify({ replyText: 'Unknown route' })); } - throw new Error('Error should have been thrown'); }); - it('should throw error with status message if response body is empty and status message exists', async () => { - apiResponse.data = ''; - apiResponse.statusText = 'dummy status message'; + describe('when empty response is allowed', function() { + beforeEach(function() { + extendedRequestOptions.allowEmptyResponse = true; + wrapper = new RequestWrapper(extendedRequestOptions, 'http:'); + }); - try { - await wrapper.send(); - } catch (err) { - const error = err as EscherRequestError; - expect(error.data).to.eql(apiResponse.statusText); - return; - } - throw new Error('Error should have been thrown'); - }); - it('should throw a http response error even if the response body is empty', async () => { - apiResponse.data = '404 Not Found'; - apiResponse.status = 404; - apiResponse.headers = { 'content-type': 'text/plain' }; + it('should allow body to be empty', async () => { + apiResponse.headers['content-type'] = 'text/html'; + apiResponse.data = ''; + apiResponse.status = 204; - try { - await wrapper.send(); - throw new Error('should throw'); - } catch (err) { - const error = err as EscherRequestError; - expect(error).to.be.an.instanceOf(EscherRequestError); - expect(error.code).to.eql(apiResponse.status); - expect(error.message).to.eql('Error in http response (status: 404)'); - expect(error.data).to.eql(apiResponse.data); - } - }); - }); + const response = await wrapper.send(); - describe('when there was an axios error', function() { - let isCancel: any; - let axiosError: any; + expect(response.statusCode).to.eql(204); + }); - beforeEach(function() { - axiosError = { - message: 'axios error message', - stack: [] - }; - isCancel = sinon.stub(axios, 'isCancel'); - requestGetStub.rejects(axiosError); + + it('should throw error if json is not parsable (empty)', async () => { + apiResponse.data = ''; + + try { + await wrapper.send(); + } catch (err) { + const error = err as EscherRequestError; + expect(error).to.be.an.instanceof(EscherRequestError); + expect(error.message).to.match(/Unexpected end/); + expect(error.code).to.eql(500); + return; + } + throw new Error('Error should have been thrown'); + }); }); - context('when the request has not been canceled', function() { - beforeEach(function() { - isCancel.returns(false); + describe('when empty response is not allowed', function() { + it('should throw error if response body is empty', async () => { + apiResponse.data = ''; + + try { + await wrapper.send(); + } catch (err) { + const error = err as EscherRequestError; + expect(error).to.be.an.instanceof(EscherRequestError); + expect(error.message).to.eql('Empty http response'); + expect(error.code).to.eql(500); + expect(error.data).to.eql(expectedApiResponse.statusMessage); + return; + } + throw new Error('Error should have been thrown'); }); - it('cancels the request', async () => { + it('should throw error with status message if response body is empty and status message exists', async () => { + apiResponse.data = ''; + apiResponse.statusText = 'dummy status message'; + try { await wrapper.send(); - throw new Error('should throw SuiteRequestError'); } catch (err) { - expect(source.cancel).to.have.been.calledWith(); + const error = err as EscherRequestError; + expect(error.data).to.eql(apiResponse.statusText); + return; + } + throw new Error('Error should have been thrown'); + }); + + it('should throw a http response error even if the response body is empty', async () => { + requestGetStub.restore(); + nock('http://very.host.io:443') + .get('/purchases/1/content') + .reply(404, { replyText: '404 Not Found' }); + + try { + await wrapper.send(); + throw new Error('should throw'); + } catch (err) { + const error = err as EscherRequestError; + expect(error).to.be.an.instanceOf(EscherRequestError); + expect(error.code).to.eql(404); + expect(error.message).to.eql('Error in http response (status: 404)'); + expect(error.data).to.eql(JSON.stringify({ replyText: '404 Not Found' })); } }); }); - context('when the request has already been canceled', function() { + describe('when there was an axios error', function() { + let isCancel: any; + let axiosError: any; + beforeEach(function() { - isCancel.returns(true); + axiosError = { + message: 'axios error message', + stack: [] + }; + isCancel = sinon.stub(axios, 'isCancel'); + requestGetStub.rejects(axiosError); + }); + + context('when the request has not been canceled', function() { + beforeEach(function() { + isCancel.returns(false); + }); + + it('cancels the request', async () => { + try { + await wrapper.send(); + throw new Error('should throw SuiteRequestError'); + } catch (err) { + expect(source.cancel).to.have.been.calledWith(); + } + }); + }); + + context('when the request has already been canceled', function() { + beforeEach(function() { + isCancel.returns(true); + }); + + it('does not cancel the request', async () => { + try { + await wrapper.send(); + throw new Error('should throw SuiteRequestError'); + } catch (err) { + expect(source.cancel).not.to.have.been.calledWith(); + } + }); }); - it('does not cancel the request', async () => { + it('should pass original error code to SuiteRequestError', async () => { try { + axiosError.code = 'ECONNABORTED'; + await wrapper.send(); throw new Error('should throw SuiteRequestError'); } catch (err) { - expect(source.cancel).not.to.have.been.calledWith(); + const error = err as EscherRequestError; + expect(error.originalCode).to.eql('ECONNABORTED'); + expect(error.code).to.eql(503); + expect(error.data).to.eql({ replyText: 'axios error message' }); } }); }); - it('should pass original error code to SuiteRequestError', async () => { - try { - axiosError.code = 'ECONNABORTED'; + it('should throw error if json is not parsable (malformed)', async () => { + apiResponse.data = 'this is an invalid json'; + + try { await wrapper.send(); - throw new Error('should throw SuiteRequestError'); } catch (err) { const error = err as EscherRequestError; - expect(error.originalCode).to.eql('ECONNABORTED'); - expect(error.code).to.eql(503); - expect(error.data).to.eql({ replyText: 'axios error message' }); + expect(error).to.be.an.instanceof(EscherRequestError); + expect(error.message).to.match(/Unexpected token/); + expect(error.code).to.eql(500); + return; } + throw new Error('Error should have been thrown'); }); - }); - - - it('should throw error if json is not parsable (malformed)', async () => { - apiResponse.data = 'this is an invalid json'; - - try { - await wrapper.send(); - } catch (err) { - const error = err as EscherRequestError; - expect(error).to.be.an.instanceof(EscherRequestError); - expect(error.message).to.match(/Unexpected token/); - expect(error.code).to.eql(500); - return; - } - throw new Error('Error should have been thrown'); - }); - it('should parse JSON if content-type header contains charset too', async () => { - const testJson = { text: 'Test JSON text' }; - apiResponse.headers['content-type'] = 'application/json; charset=utf-8'; - apiResponse.data = JSON.stringify(testJson); + it('should parse JSON if content-type header contains charset too', async () => { + const testJson = { text: 'Test JSON text' }; + apiResponse.headers['content-type'] = 'application/json; charset=utf-8'; + apiResponse.data = JSON.stringify(testJson); - const response = await wrapper.send(); + const response = await wrapper.send(); - expect(response.body).to.eql(testJson); - }); + expect(response.body).to.eql(testJson); + }); - it('should send GET request with given timeout in options', async () => { - extendedRequestOptions.timeout = 60000; - await (new RequestWrapper(extendedRequestOptions, 'http:')).send(); + it('should send GET request with given timeout in options', async () => { + extendedRequestOptions.timeout = 60000; + await (new RequestWrapper(extendedRequestOptions, 'http:')).send(); - const requestArgument = requestGetStub.args[0][0]; - expect(requestArgument.timeout).to.eql(extendedRequestOptions.timeout); + const requestArgument = requestGetStub.args[0][0]; + expect(requestArgument.timeout).to.eql(extendedRequestOptions.timeout); + }); }); }); }); diff --git a/src/wrapper.ts b/src/wrapper.ts index d707564..02e07bb 100644 --- a/src/wrapper.ts +++ b/src/wrapper.ts @@ -35,7 +35,11 @@ export class RequestWrapper { private readonly payload: any; private readonly requestOptions: ExtendedRequestOption; - constructor(requestOptions: ExtendedRequestOption, protocol: string, payload: any = undefined) { + constructor( + requestOptions: ExtendedRequestOption, + protocol: string, + payload: any = undefined, + ) { this.requestOptions = requestOptions; this.protocol = protocol; this.payload = payload; @@ -61,7 +65,6 @@ export class RequestWrapper { timeout: reqOptions.timeout, transformResponse: [body => body], maxContentLength: this.requestOptions.maxContentLength, - validateStatus: () => true, cancelToken: source.token }; @@ -95,31 +98,31 @@ export class RequestWrapper { } private handleResponseError(error: AxiosError, source: CancelTokenSource) { - if (!axios.isCancel(error)) { - source.cancel(); - logger.info('Canceled request'); - } - logger.fromError('fatal_error', error, this.getLogParameters()); - - const recoverableErrorCodes = ['ECONNRESET', 'ETIMEDOUT', 'ECONNREFUSED', 'ECONNABORTED']; - const code = recoverableErrorCodes.includes(error.code || '') ? 503 : 500; - - throw new EscherRequestError(error.message, code, undefined, error.code); - } - - private handleResponse(response: TransformedResponse): TransformedResponse { - if (response.statusCode >= 400) { + if (error.response?.status) { logger.error('server_error', this.getLogParameters({ - code: response.statusCode, - reply_text: response.body.replyText + code: error.response.status, + reply_text: (error.response?.data || '') })); throw new EscherRequestError( - 'Error in http response (status: ' + response.statusCode + ')', - response.statusCode, - this.parseBody(response) + 'Error in http response (status: ' + error.response.status + ')', + error.response.status, + (error.response?.data || '') as string ); + } else { + if (!axios.isCancel(error)) { + source.cancel(); + logger.info('Canceled request'); + } + logger.fromError('fatal_error', error, this.getLogParameters()); + + const recoverableErrorCodes = ['ECONNRESET', 'ETIMEDOUT', 'ECONNREFUSED', 'ECONNABORTED']; + const code = recoverableErrorCodes.includes(error.code || '') ? 503 : 500; + + throw new EscherRequestError(error.message, code, undefined, error.code); } + } + private handleResponse(response: TransformedResponse): TransformedResponse { if (!this.requestOptions.allowEmptyResponse && !response.body) { logger.error('server_error empty response data', this.getLogParameters()); throw new EscherRequestError('Empty http response', 500, response.statusMessage);