From c45097785031cdcb85a1a6a81bc44cac5e92502c Mon Sep 17 00:00:00 2001 From: Boris Tomic <16724532+boristomic@users.noreply.github.com> Date: Mon, 4 Dec 2023 14:11:07 +0100 Subject: [PATCH 1/3] feat!: parse json error response Signed-off-by: Boris Tomic <16724532+boristomic@users.noreply.github.com> BREAKING CHANGE: Error response is parsed as JSON if response headers have content-type application/json and if it's possible to parse the data as JSON. Otherwise response will be retuned as string or default response will be returned as empty string. --- src/wrapper.spec.ts | 55 +++++++++++++++++++++++++++++++-------------- src/wrapper.ts | 16 +++++++++++-- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/wrapper.spec.ts b/src/wrapper.spec.ts index 8b5bb9c..4d84b3c 100644 --- a/src/wrapper.spec.ts +++ b/src/wrapper.spec.ts @@ -119,24 +119,45 @@ describe('RequestWrapper', function() { expect(requestArgument.httpsAgent).to.eql(agents.httpsAgent); }); - 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' }); + context('error responses', () => { + it('should return JSON in EscherRequestError if response data is json parsable', async () => { + requestGetStub.restore(); + nock('http://very.host.io:443') + .get('/purchases/1/content') + .reply(400, { replyText: 'Unknown route' }, { 'Content-Type': 'application/json; charset=utf-8' }); - 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(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' }); + } + }); + + it('should return text and not fail parsing response data if wrong content-type headers are set', async () => { + requestGetStub.restore(); + nock('http://very.host.io:443') + .get('/purchases/1/content') + .reply(500, 'Unexpected Error', { 'Content-Type': 'application/json; charset=utf-8' }); + + 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: 500)'); + expect(error.code).to.eql(500); + expect(error.data).to.eql('Unexpected Error'); + } + }); }); + describe('when empty response is allowed', function() { beforeEach(function() { extendedRequestOptions.allowEmptyResponse = true; @@ -216,7 +237,7 @@ describe('RequestWrapper', function() { 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' })); + expect(error.data).to.eql({ replyText: '404 Not Found' }); } }); }); @@ -340,7 +361,7 @@ describe('RequestWrapper', function() { 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' })); + expect(error.data).to.eql({ replyText: '404 Not Found' }); } }); diff --git a/src/wrapper.ts b/src/wrapper.ts index b303e0c..159209c 100644 --- a/src/wrapper.ts +++ b/src/wrapper.ts @@ -109,10 +109,22 @@ export class RequestWrapper { code: error.response.status, reply_text: (error.response?.data || '') })); + let data = ''; + if (error.response != null && + this.isJsonResponse(error.response) && + error.response.data != null && + typeof error.response.data === 'string') { + try { + data = JSON.parse(error.response.data); + } catch (_) { + data = error.response.data; + } + } + throw new EscherRequestError( 'Error in http response (status: ' + error.response.status + ')', error.response.status, - (error.response?.data || '') as string + data ); } else { if (!axios.isCancel(error)) { @@ -142,7 +154,7 @@ export class RequestWrapper { }; } - private isJsonResponse(response: TransformedResponse) { + private isJsonResponse(response: T) { return response.headers['content-type'] && response.headers['content-type'].indexOf('application/json') !== -1; } From b7685540631d2eda72648a6bbae15a2e3bf5585e Mon Sep 17 00:00:00 2001 From: Boris Tomic <16724532+boristomic@users.noreply.github.com> Date: Mon, 4 Dec 2023 15:02:49 +0100 Subject: [PATCH 2/3] chore: ensure headers are case insensitive Signed-off-by: Boris Tomic <16724532+boristomic@users.noreply.github.com> --- src/wrapper.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/wrapper.ts b/src/wrapper.ts index 159209c..8eb2375 100644 --- a/src/wrapper.ts +++ b/src/wrapper.ts @@ -155,8 +155,16 @@ export class RequestWrapper { } private isJsonResponse(response: T) { - return response.headers['content-type'] && - response.headers['content-type'].indexOf('application/json') !== -1; + let headers: RawAxiosResponseHeaders | AxiosResponseHeaders = {}; + Object.assign(headers, response.headers); + headers = Object.entries(response.headers) + .reduce((acc: RawAxiosResponseHeaders | AxiosResponseHeaders, [key, val]) => { + acc[key.toLowerCase()] = val.toLowerCase(); + return acc; + }, ({})); + + return headers['content-type'] && + headers['content-type'].indexOf('application/json') !== -1; } private getLogParameters(extraParametersToLog = {}) { From dd4bf82c35df287b1e582c2d6d16cc39d505bd06 Mon Sep 17 00:00:00 2001 From: Boris Tomic <16724532+boristomic@users.noreply.github.com> Date: Mon, 11 Dec 2023 15:52:08 +0100 Subject: [PATCH 3/3] Revert "chore: ensure headers are case insensitive" This reverts commit b7685540631d2eda72648a6bbae15a2e3bf5585e. --- src/wrapper.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/wrapper.ts b/src/wrapper.ts index 8eb2375..159209c 100644 --- a/src/wrapper.ts +++ b/src/wrapper.ts @@ -155,16 +155,8 @@ export class RequestWrapper { } private isJsonResponse(response: T) { - let headers: RawAxiosResponseHeaders | AxiosResponseHeaders = {}; - Object.assign(headers, response.headers); - headers = Object.entries(response.headers) - .reduce((acc: RawAxiosResponseHeaders | AxiosResponseHeaders, [key, val]) => { - acc[key.toLowerCase()] = val.toLowerCase(); - return acc; - }, ({})); - - return headers['content-type'] && - headers['content-type'].indexOf('application/json') !== -1; + return response.headers['content-type'] && + response.headers['content-type'].indexOf('application/json') !== -1; } private getLogParameters(extraParametersToLog = {}) {