diff --git a/app/services/health/info.service.js b/app/services/health/info.service.js index 42253cf26b..b4efdf309b 100644 --- a/app/services/health/info.service.js +++ b/app/services/health/info.service.js @@ -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') /** @@ -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 () { @@ -137,7 +118,7 @@ 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) @@ -145,7 +126,7 @@ async function _getAppData () { service.commit = data.commit service.jobs = service.name === 'Import' ? _getImportJobsData() : [] } else { - service.version = result.response + service.version = _parseFailedRequestResult(result) service.commit = '' } } @@ -153,6 +134,14 @@ async function _getAppData () { 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 } diff --git a/test/services/health/info.service.test.js b/test/services/health/info.service.test.js index 0ca15877ac..c546bed20e 100644 --- a/test/services/health/info.service.test.js +++ b/test/services/health/info.service.test.js @@ -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() @@ -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 @@ -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', () => { @@ -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 () => { @@ -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 () => {