Skip to content

Commit

Permalink
Use RequestLib in InfoService (#65)
Browse files Browse the repository at this point in the history 
DEFRA/water-abstraction-team#54

We recently added `RequestLib` to bring all our HTTP request logic into one place. It also has support for working behind proxy services. This change updates `InfoService` to use our new module and to get requests working in our AWS environment.
  • Loading branch information
@Cruikshanks
Cruikshanks authored Dec 19, 2022
1 parent 699c219 commit 3f29ba6
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 62 deletions.
55 changes: 22 additions & 33 deletions app/services/health/info.service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const ChildProcess = require('child_process')
const util = require('util')
const exec = util.promisify(ChildProcess.exec)

const RequestLib = require('../../lib/request.lib.js')

const servicesConfig = require('../../../config/services.config.js')

/**
Expand Down Expand Up @@ -67,45 +69,24 @@ async function _getRedisConnectivityData () {

async function _getAddressFacadeData () {
const statusUrl = new URL('/address-service/hola', servicesConfig.addressFacade.url)
const result = await _requestData(statusUrl)
const result = await RequestLib.get(statusUrl.href)

if (result.succeeded) {
return result.response.body
}

return result.succeeded ? result.response.body : result.response
return _parseFailedRequestResult(result)
}

async function _getChargingModuleData () {
const statusUrl = new URL('/status', servicesConfig.chargingModule.url)
const result = await _requestData(statusUrl)

return result.succeeded ? result.response.headers['x-cma-docker-tag'] : result.response
}

async function _requestData (url) {
// As of v12, the got dependency no longer supports CJS modules. This causes us a problem as we are locked into
// using these for the time being. Some workarounds are provided here: https://github.com/sindresorhus/got/issues/1789
// We have gone the route of using await import('got'). We cannot do this at the top level as Node doesn't support
// top level in CJS so we do it here instead.
const { got } = await import('got')
const result = {
succeeded: true,
response: null
}
const result = await RequestLib.get(statusUrl.href)

try {
result.response = await got.get(url, {
retry: {
// We ensure that the only network errors Got retries are timeout errors
errorCodes: ['ETIMEDOUT'],
// We set statusCodes as an empty array to ensure that 4xx, 5xx etc. errors are not retried
statusCodes: []
}
})
} catch (error) {
const statusCode = error.response ? error.response.statusCode : 'N/A'
result.response = `ERROR: ${statusCode} - ${error.name} - ${error.message}`
result.succeeded = false
if (result.succeeded) {
return result.response.headers['x-cma-docker-tag']
}

return result
return _parseFailedRequestResult(result)
}

function _getImportJobsData () {
Expand Down Expand Up @@ -137,22 +118,30 @@ async function _getAppData () {
]

for (const service of services) {
const result = await _requestData(service.url)
const result = await RequestLib.get(service.url.href)

if (result.succeeded) {
const data = JSON.parse(result.response.body)
service.version = data.version
service.commit = data.commit
service.jobs = service.name === 'Import' ? _getImportJobsData() : []
} else {
service.version = result.response
service.version = _parseFailedRequestResult(result)
service.commit = ''
}
}

return services
}

function _parseFailedRequestResult (result) {
if (result.response.statusCode) {
return `ERROR: ${result.response.statusCode} - ${result.response.body}`
}

return `ERROR: ${result.response.name} - ${result.response.message}`
}

module.exports = {
go
}
108 changes: 79 additions & 29 deletions test/services/health/info.service.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
const Lab = require('@hapi/lab')
const Code = require('@hapi/code')
const Sinon = require('sinon')
const Nock = require('nock')
const Proxyquire = require('proxyquire')

const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script()
Expand All @@ -13,50 +12,85 @@ const { expect } = Code
// Test helpers
const servicesConfig = require('../../../config/services.config.js')

// Things we need to stub
const RequestLib = require('../../../app/lib/request.lib.js')

// Thing under test
// Normally we'd set this to `= require('../../app/services/health/info.service')`. But to control how
// `child_process.exec()` behaves in the service, after it's been promisfied we have to use proxyquire.
let InfoService // = require('../../app/services/health/info.service')

describe('Info service', () => {
const goodRequestResults = {
addressFacade: { succeeded: true, response: { statusCode: 200, body: 'hey there' } },
chargingModule: {
succeeded: true,
response: {
statusCode: 200,
headers: {
'x-cma-git-commit': '273604040a47e0977b0579a0fef0f09726d95e39',
'x-cma-docker-tag': 'ghcr.io/defra/sroc-charging-module-api:v9.99.9'
}
}
},
app: { succeeded: true, response: { statusCode: 200, body: '{ "version": "9.0.99", "commit": "99d0e8c" }' } }
}
let requestLibStub

beforeEach(() => {
requestLibStub = Sinon.stub(RequestLib, 'get')
// These requests will remain unchanged throughout the tests. We do alter the ones to the AddressFacade and the
// water-api (foreground-service) though, which is why they are defined separately in each test.
Nock(servicesConfig.chargingModule.url)
.get('/status')
.reply(200, { status: 'alive' },
[
'x-cma-git-commit', '273604040a47e0977b0579a0fef0f09726d95e39',
'x-cma-docker-tag', 'ghcr.io/defra/sroc-charging-module-api:v9.99.9'
]
)

Nock(servicesConfig.serviceBackground.url).get('/health/info').reply(200, { version: '8.0.12', commit: '83d0e8c' })
Nock(servicesConfig.reporting.url).get('/health/info').reply(200, { version: '8.0.11', commit: 'a7030dc' })
Nock(servicesConfig.import.url).get('/health/info').reply(200, { version: '8.0.7', commit: 'a181fb1' })
Nock(servicesConfig.tacticalCrm.url).get('/health/info').reply(200, { version: '8.0.2', commit: '58bd0c1' })
Nock(servicesConfig.externalUi.url).get('/health/info').reply(200, { version: '8.0.0', commit: 'f154e3f' })
Nock(servicesConfig.internalUi.url).get('/health/info').reply(200, { version: '8.0.8', commit: 'f154e3f' })
Nock(servicesConfig.tacticalIdm.url).get('/health/info').reply(200, { version: '8.0.3', commit: '2ddff3c' })
Nock(servicesConfig.permitRepository.url).get('/health/info').reply(200, { version: '8.0.4', commit: '09d5261' })
Nock(servicesConfig.returns.url).get('/health/info').reply(200, { version: '8.0.6', commit: 'b181625' })
requestLibStub
.withArgs(`${servicesConfig.chargingModule.url}/status`)
.resolves(goodRequestResults.chargingModule)
requestLibStub
.withArgs(`${servicesConfig.serviceBackground.url}/health/info`)
.resolves(goodRequestResults.app)
requestLibStub
.withArgs(`${servicesConfig.reporting.url}/health/info`)
.resolves(goodRequestResults.app)
requestLibStub
.withArgs(`${servicesConfig.import.url}/health/info`)
.resolves(goodRequestResults.app)
requestLibStub
.withArgs(`${servicesConfig.tacticalCrm.url}/health/info`)
.resolves(goodRequestResults.app)
requestLibStub
.withArgs(`${servicesConfig.externalUi.url}/health/info`)
.resolves(goodRequestResults.app)
requestLibStub
.withArgs(`${servicesConfig.internalUi.url}/health/info`)
.resolves(goodRequestResults.app)
requestLibStub
.withArgs(`${servicesConfig.tacticalIdm.url}/health/info`)
.resolves(goodRequestResults.app)
requestLibStub
.withArgs(`${servicesConfig.permitRepository.url}/health/info`)
.resolves(goodRequestResults.app)
requestLibStub
.withArgs(`${servicesConfig.returns.url}/health/info`)
.resolves(goodRequestResults.app)
})

afterEach(() => {
Sinon.restore()
Nock.cleanAll()
})

describe('when all the services are running', () => {
beforeEach(async () => {
// In this scenario everything is hunky-dory so we return 2xx responses from these services
Nock(servicesConfig.addressFacade.url).get('/address-service/hola').reply(200, 'hey there')
Nock(servicesConfig.serviceForeground.url).get('/health/info').reply(200, { version: '8.0.1', commit: '83d0e8c' })
requestLibStub
.withArgs(`${servicesConfig.addressFacade.url}/address-service/hola`)
.resolves(goodRequestResults.addressFacade)
requestLibStub
.withArgs(`${servicesConfig.serviceForeground.url}/health/info`)
.resolves(goodRequestResults.app)

// Unfortunately, this convoluted test setup is the only way we've managed to stub how the promisified version of
// `child-process.exec()` behaves in the module under test.
// We create an anonymous stub, which responds differently depending on which service is being checked. We then
// stub the util library's `promisify()` method and tell it to calll our anonymous stub when invoked. The bit that
// stub the util library's `promisify()` method and tell it to call our anonymous stub when invoked. The bit that
// makes all this work is the fact we use Proxyquire to load our stubbed util instead of the real one when we load
// our module under test
const execStub = Sinon
Expand Down Expand Up @@ -90,8 +124,12 @@ describe('Info service', () => {
describe('when a service we check via the shell', () => {
beforeEach(async () => {
// In these scenarios everything is hunky-dory so we return 2xx responses from these services
Nock(servicesConfig.addressFacade.url).get('/address-service/hola').reply(200, 'hey there')
Nock(servicesConfig.serviceForeground.url).get('/health/info').reply(200, { version: '8.0.1', commit: '83d0e8c' })
requestLibStub
.withArgs(`${servicesConfig.addressFacade.url}/address-service/hola`)
.resolves(goodRequestResults.addressFacade)
requestLibStub
.withArgs(`${servicesConfig.serviceForeground.url}/health/info`)
.resolves(goodRequestResults.app)
})

describe('is not running', () => {
Expand Down Expand Up @@ -179,8 +217,14 @@ describe('Info service', () => {

describe('cannot be reached because of a network error', () => {
beforeEach(async () => {
Nock(servicesConfig.addressFacade.url).get('/address-service/hola').replyWithError({ code: 'ECONNRESET' })
Nock(servicesConfig.serviceForeground.url).get('/health/info').replyWithError({ code: 'ECONNRESET' })
const badResult = { succeeded: false, response: new Error('Kaboom') }

requestLibStub
.withArgs(`${servicesConfig.addressFacade.url}/address-service/hola`)
.resolves(badResult)
requestLibStub
.withArgs(`${servicesConfig.serviceForeground.url}/health/info`)
.resolves(badResult)
})

it('handles the error and still returns a result for the other services', async () => {
Expand All @@ -198,8 +242,14 @@ describe('Info service', () => {

describe('returns a 5xx response', () => {
beforeEach(async () => {
Nock(servicesConfig.addressFacade.url).get('/address-service/hola').reply(500, 'Kaboom')
Nock(servicesConfig.serviceForeground.url).get('/health/info').reply(500, 'Kaboom')
const badResult = { succeeded: false, response: { statusCode: 500, body: 'Kaboom' } }

requestLibStub
.withArgs(`${servicesConfig.addressFacade.url}/address-service/hola`)
.resolves(badResult)
requestLibStub
.withArgs(`${servicesConfig.serviceForeground.url}/health/info`)
.resolves(badResult)
})

it('handles the error and still returns a result for the other services', async () => {
Expand Down

0 comments on commit 3f29ba6

Please sign in to comment.