Skip to content

Commit

Permalink
PP-11120 Spike replacing request with axios
Browse files Browse the repository at this point in the history
Add axios-base-client.js to replace base.client.js. Replace usage in
connector.client.js as proof of it working.

Added some tests in axios-base-client.test.js, but possibly will need to
add a few more when this is properly implemented.

Have implemented custom retry logic to handle ECONNRESET errors. We
could use the `axios-retry` library for this instead if we don't trust
this.

The intention is that `axios-base-client.js` could be moved to
pay-js-commons. When the client is configure, it accepts an options
object which can provide hook functions to hook into setting the headers
for a request and performing actions on the request starting/ending,
which can be used to do logging per app.
  • Loading branch information
stephencdaly committed Oct 12, 2023
1 parent 32be47c commit d83f8d9
Show file tree
Hide file tree
Showing 6 changed files with 438 additions and 299 deletions.
115 changes: 115 additions & 0 deletions app/services/clients/base-client/axios-base-client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
'use strict'

const http = require('http')
const https = require('https')
const axios = require('axios')
const { RESTClientError } = require('../../../errors')

// TODO: put this this in pay-js-commons. Will need to move RESTClientError and possibly the base type DomainError too
class Client {
constructor (app) {
this._app = app
}

/**
* Configure the client. Should only be called once.
*/
_configure (baseURL, options) {
this._axios = axios.create({
baseURL,
timeout: 60 * 1000,
maxContentLength: 50 * 1000 * 1000,
httpAgent: new http.Agent({
keepAlive: true
}),
httpsAgent: new https.Agent({
keepAlive: true,
rejectUnauthorized: process.env.NODE_ENV === 'production'
}),
headers: {
'Content-Type': 'application/json',
'Accept': 'application/json'
}
})

this._axios.interceptors.request.use(config => {
const requestContext = {
service: this._app,
method: config.method,
url: config.url,
description: config.description,
...config.retryCount && { retryCount: config.retryCount }
}
if (options.onRequestStart) {
options.onRequestStart(requestContext)
}

const headers = options.transformRequestAddHeaders ? options.transformRequestAddHeaders() : {}
Object.entries(headers)
.forEach(([headerKey, headerValue]) => {
config.headers[headerKey] = headerValue
})

return {
...config,
metadata: { start: Date.now() }
}
})

this._axios.interceptors.response.use((response) => {
const responseContext = {
service: this._app,
responseTime: Date.now() - (response.config).metadata.start,
method: response.config.method,
params: response.config.params,
status: response.status,
url: response.config.url,
description: response.config.description
}
if (options.onSuccessResponse) {
options.onSuccessResponse(responseContext)
}
return response
}, async (error) => {
const config = error.config || {}
let errors = error.response.data && (error.response.data.message || error.response.data.errors)
if (errors && Array.isArray(errors)) {
errors = errors.join(', ')
}
const errorContext = {
service: this._app,
responseTime: Date.now() - (config.metadata && config.metadata.start),
method: config.method,
params: config.params,
status: error.response && error.response.status,
url: config.url,
code: (error.response && error.response.status) || error.code,
errorIdentifier: error.response && error.response.data && error.response.data.error_identifier,
reason: error.response && error.response.data && error.response.data.reason,
message: errors || error.response.data || 'Unknown error',
description: config.description
}

// TODO: could use axios-retry to achieve this if desired
// Retry ECONNRESET errors 3 times in total
if (error.code === 'ECONNRESET') {
const retryCount = config.retryCount || 0
if (retryCount < 2) {
config.retryCount = retryCount + 1
if (options.onFailureResponse) {
errorContext.retry = true
options.onFailureResponse(errorContext)
}
await new Promise(resolve => setTimeout(resolve, 500))
return this._axios(config)
}
}
if (options.onFailureResponse) {
options.onFailureResponse(errorContext)
}
throw new RESTClientError(errorContext.message, errorContext.service, errorContext.status, errorContext.errorIdentifier, errorContext.reason)
})
}
}

module.exports = { Client }
128 changes: 128 additions & 0 deletions app/services/clients/base-client/axios-base-client.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
'use strict'

const nock = require('nock')
const sinon = require('sinon')
const chai = require('chai')
const chaiAsPromised = require('chai-as-promised')
const { Client } = require('./axios-base-client')

chai.use(chaiAsPromised)
const { expect } = chai

const baseUrl = 'http://localhost:8000'
const app = 'an-app'

describe('Axios base client', () => {
const requestStartSpy = sinon.spy()
const requestSuccessSpy = sinon.spy()
const requestFailureSpy = sinon.spy()
const client = new Client(app)
client._configure(baseUrl, {
onRequestStart: requestStartSpy,
onSuccessResponse: requestSuccessSpy,
onFailureResponse: requestFailureSpy
})

beforeEach(() => {
requestStartSpy.resetHistory()
requestFailureSpy.resetHistory()
requestSuccessSpy.resetHistory()
})

describe('Response and hooks', () => {
it('should return response and call success hook on 200 response', () => {
const body = { foo: 'bar' }
nock(baseUrl)
.get('/')
.reply(200, body)

return expect(client._axios.get('/', { description: 'foo' })).to.be.fulfilled.then((response) => {
expect(response.data).to.deep.equal(body)
})
})

it('should throw error and call failure hook on 400 response', () => {
const body = {
error_identifier: 'AN-ERROR',
message: 'a-message'
}
nock(baseUrl)
.get('/')
.reply(400, body)

return expect(client._axios.get('/', { description: 'foo' })).to.be.rejected.then((error) => {
expect(error.message).to.equal('a-message')
expect(error.errorCode).to.equal(400)
expect(error.errorIdentifier).to.equal('AN-ERROR')
expect(error.service).to.equal(app)
})
})

it('should throw error and call failure hook on 500 response', () => {
const body = {
error_identifier: 'AN-ERROR',
message: 'a-message'
}
nock(baseUrl)
.get('/')
.reply(500, body)

return expect(client._axios.get('/', { description: 'foo' })).to.be.rejected.then((error) => {
expect(error.message).to.equal('a-message')
expect(error.errorCode).to.equal(500)
expect(error.errorIdentifier).to.equal('AN-ERROR')
expect(error.service).to.equal(app)
})
})
})

describe('Retries', () => {
it('should retry 3 times when ECONNRESET error thrown', () => {
nock(baseUrl)
.get('/')
.times(3)
.replyWithError({
code: 'ECONNRESET',
response: { status: 500 }
})

return expect(client._axios.get('/', { description: 'foo' })).to.be.rejected.then(error => {
expect(error.errorCode).to.equal(500)
sinon.assert.calledThrice(requestStartSpy)
requestStartSpy.getCall(0).calledWithMatch({
method: 'get',
url: '/'
})
requestStartSpy.getCall(1).calledWithMatch({
method: 'get',
url: '/',
retryCount: 2
})
requestStartSpy.getCall(2).calledWithMatch({
method: 'get',
url: '/',
retryCount: 3
})
sinon.assert.calledThrice(requestFailureSpy)
sinon.assert.calledWithMatch(requestFailureSpy, { retry: true })
expect(nock.isDone()).to.eq(true)
})
})

it('should not retry for an error other than ECONNRESET', () => {
nock(baseUrl)
.get('/')
.replyWithError({
response: { status: 500 }
})

return expect(client._axios.get('/')).to.be.rejected.then(error => {
expect(error.errorCode).to.equal(500)
sinon.assert.calledOnce(requestStartSpy)
sinon.assert.calledOnce(requestFailureSpy)
expect(requestFailureSpy.getCall(0).args.retry === undefined)
expect(nock.isDone()).to.eq(true)
})
})
})
})
5 changes: 3 additions & 2 deletions app/services/clients/base-client/base.client.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ function makeRequest (method, opts) {
requestLogger.logRequestError(loggingContext, err)
})
call.on('response', response => {
requestLogger.logRequestEnd(loggingContext, response)
loggingContext.status = response.statusCode
requestLogger.logRequestEnd(loggingContext)
if (!(response && SUCCESS_CODES.includes(response.statusCode))) {
requestLogger.logRequestFailure(loggingContext, response)
requestLogger.logRequestFailure(loggingContext)
}
})
return call
Expand Down
40 changes: 40 additions & 0 deletions app/services/clients/base-client/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict'

const requestLogger = require('../../../utils/request-logger')
const { getRequestCorrelationIDField } = require('../../../utils/request-context')
const { CORRELATION_HEADER } = require('../../../../config')

function transformRequestAddHeaders () {
const correlationId = getRequestCorrelationIDField()
const headers = {}
if (correlationId) {
headers[CORRELATION_HEADER] = correlationId
}
return headers
}

function onRequestStart (context) {
requestLogger.logRequestStart(context)
}

function onSuccessResponse (context) {
requestLogger.logRequestEnd(context)
}

function onFailureResponse (context) {
requestLogger.logRequestEnd(context)
requestLogger.logRequestFailure(context)
}

function configureClient (client, baseUrl) {
client._configure(baseUrl, {
transformRequestAddHeaders,
onRequestStart,
onSuccessResponse,
onFailureResponse
})
}

module.exports = {
configureClient
}
Loading

0 comments on commit d83f8d9

Please sign in to comment.