From c9154993f3a725e3350cce47dddfcb1e23f63e4a Mon Sep 17 00:00:00 2001 From: Iqbal Ahmed Date: Fri, 12 Apr 2024 15:49:19 +0100 Subject: [PATCH] PP-11683 Update healthcheck controller to use Axios - Uses the Axios base client from `pay-js-commons` for the healthcheck controller. - Other clients will be replaced in with Axios base client in future PRs. --- app/controllers/healthcheck.controller.js | 34 ++++++-- app/services/clients/base/config.js | 40 +++++++++ app/services/clients/base/config.test.js | 90 ++++++++++++++++++++ app/services/clients/base/request-context.js | 38 +++++++++ app/services/clients/base/request-logger.js | 53 ++++++++++++ package-lock.json | 48 +++++------ package.json | 2 +- server.js | 4 + 8 files changed, 272 insertions(+), 37 deletions(-) create mode 100644 app/services/clients/base/config.js create mode 100644 app/services/clients/base/config.test.js create mode 100644 app/services/clients/base/request-context.js create mode 100644 app/services/clients/base/request-logger.js diff --git a/app/controllers/healthcheck.controller.js b/app/controllers/healthcheck.controller.js index 29afd5f69..0c3b68e51 100644 --- a/app/controllers/healthcheck.controller.js +++ b/app/controllers/healthcheck.controller.js @@ -2,7 +2,10 @@ const _ = require('lodash') const logger = require('../utils/logger')(__filename) const { getLoggingFields } = require('../utils/logging-fields-helper') -const baseClient = require('../services/clients/base.client/base.client') +const { Client } = require('@govuk-pay/pay-js-commons/lib/utils/axios-base-client/axios-base-client') +const { configureClient } = require('../services/clients/base/config') +const SERVICE_NAME = 'frontend' +const client = new Client(SERVICE_NAME) const healthyPingResponse = { ping: { healthy: true } } @@ -11,21 +14,34 @@ const respond = (res, statusCode, data) => { res.end(JSON.stringify(data)) } -module.exports.healthcheck = (req, res) => { +module.exports.healthcheck = async (req, res) => { if (process.env.FORWARD_PROXY_URL) { - baseClient.get(`${process.env.FORWARD_PROXY_URL}/nginx_status`, {}, (err, response) => { - const statusCode = _.get(response, 'statusCode') - if (err || statusCode !== 200) { + const url = `${process.env.FORWARD_PROXY_URL}/nginx_status` + + configureClient(client, url) + + let response + + try { + response = await client.get(url, 'Healthcheck') + + if (response.status !== 200) { logger.error('Healthchecking forward proxy returned error', { ...getLoggingFields(req), - error: err, - status_code: statusCode + status_code: response.status }) respond(res, 502, _.merge(healthyPingResponse, { proxy: { healthy: false } })) } else { - respond(res, 200, _.merge(healthyPingResponse, { proxy: { healthy: true } })) + respond(res, 200, healthyPingResponse) } - }) + } catch (err) { + logger.error('Healthchecking forward proxy returned error', { + ...getLoggingFields(req), + error: err, + status_code: response.status + }) + respond(res, 502, _.merge(healthyPingResponse, { proxy: { healthy: false } })) + } } else { respond(res, 200, healthyPingResponse) } diff --git a/app/services/clients/base/config.js b/app/services/clients/base/config.js new file mode 100644 index 000000000..4c8f5e451 --- /dev/null +++ b/app/services/clients/base/config.js @@ -0,0 +1,40 @@ +'use strict' + +const requestLogger = require('./request-logger') +const { getRequestCorrelationIDField } = require('./request-context') +const { CORRELATION_HEADER } = require('../../../../config/correlation-header') + +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 +} diff --git a/app/services/clients/base/config.test.js b/app/services/clients/base/config.test.js new file mode 100644 index 000000000..dbe9b358a --- /dev/null +++ b/app/services/clients/base/config.test.js @@ -0,0 +1,90 @@ +'use strict' + +const nock = require('nock') +const sinon = require('sinon') +const proxyquire = require('proxyquire') +const { expect } = require('chai') +const { Client } = require('@govuk-pay/pay-js-commons/lib/utils/axios-base-client/axios-base-client') + +const baseUrl = 'http://localhost:8000' +const app = 'an-app' + +const logInfoSpy = sinon.spy() + +function getConfigWithMocks (correlationId) { + const config = proxyquire('./config.js', { + './request-context': { + getRequestCorrelationIDField: () => correlationId + }, + './request-logger': proxyquire('./request-logger', { + '../../../utils/logger': () => ({ + info: logInfoSpy + }) + }) + }) + return config +} + +describe('Client config', () => { + beforeEach(() => { + logInfoSpy.resetHistory() + }) + + describe('Headers', () => { + it('should add correlation ID as header when correlation ID exists on request context', async () => { + const client = new Client(app) + const config = getConfigWithMocks('abc123') + + config.configureClient(client, baseUrl) + + nock(baseUrl) + .get('/') + .reply(200) + + const response = await client.get('/', 'foo') + + expect(response.status).to.equal(200) + expect(response.request.headers).to.have.property('x-request-id', 'abc123') + }) + + it('should not add correlation ID as header when correlation ID does not exist on request context', async () => { + const client = new Client(app) + const config = getConfigWithMocks() + config.configureClient(client, baseUrl) + + nock(baseUrl) + .get('/') + .reply(200) + + const response = await client.get('/', 'foo') + expect(response.status).to.equal(200) + expect(response.request.headers).to.not.have.key('x-request-id') + }) + }) + + describe('Logging', () => { + it('should log request start', async () => { + const client = new Client(app) + const config = getConfigWithMocks('abc123') + config.configureClient(client, baseUrl) + + nock(baseUrl) + .get('/') + .reply(200) + + const response = await client.get('/', 'do something', { + additionalLoggingFields: { foo: 'bar' } + }) + + expect(response.status).to.equal(200) + + sinon.assert.calledWith(logInfoSpy, 'Calling an-app to do something', { + service: app, + method: 'get', + url: '/', + description: 'do something', + foo: 'bar' + }) + }) + }) +}) diff --git a/app/services/clients/base/request-context.js b/app/services/clients/base/request-context.js new file mode 100644 index 000000000..a23298a06 --- /dev/null +++ b/app/services/clients/base/request-context.js @@ -0,0 +1,38 @@ +'use strict' + +const { CORRELATION_ID } = require('@govuk-pay/pay-js-commons').logging.keys + +const { AsyncLocalStorage } = require('async_hooks') +const { CORRELATION_HEADER } = require('../../../../config/correlation-header') + +const asyncLocalStorage = new AsyncLocalStorage() + +function requestContextMiddleware (req, res, next) { + asyncLocalStorage.run({}, () => { + asyncLocalStorage.getStore()[CORRELATION_ID] = req.headers[CORRELATION_HEADER] + next() + }) +} + +function addField (key, value) { + if (asyncLocalStorage.getStore()) { + asyncLocalStorage.getStore()[key] = value + } +} + +function getRequestCorrelationIDField () { + if (asyncLocalStorage.getStore()) { + return asyncLocalStorage.getStore()[CORRELATION_ID] + } +} + +function getLoggingFields () { + return asyncLocalStorage.getStore() +} + +module.exports = { + requestContextMiddleware, + addField, + getRequestCorrelationIDField, + getLoggingFields +} diff --git a/app/services/clients/base/request-logger.js b/app/services/clients/base/request-logger.js new file mode 100644 index 000000000..38c794d62 --- /dev/null +++ b/app/services/clients/base/request-logger.js @@ -0,0 +1,53 @@ +const logger = require('../../../utils/logger')(__filename) + +module.exports = { + logRequestStart: context => { + logger.info(`Calling ${context.service} to ${context.description}`, { + service: context.service, + method: context.method, + url: context.url, + description: context.description, + ...context.additionalLoggingFields + }) + }, + + logRequestEnd: (context) => { + const responseTime = (context.startTime && new Date() - context.startTime) || context.responseTime + logger.info(`${context.method} to ${context.url} ended - elapsed time: ${responseTime} ms`, { + service: context.service, + method: context.method, + url: context.url, + description: context.description, + response_time: responseTime, + status: context.status, + ...context.additionalLoggingFields + }) + }, + + logRequestFailure: (context) => { + let message = `Calling ${context.service} to ${context.description} failed` + if (context.retry) { + message = message + ' - request will be retried' + } + + logger.info(message, { + service: context.service, + method: context.method, + url: context.url, + description: context.description, + status: context.status, + ...context.additionalLoggingFields + }) + }, + + logRequestError: (context, error) => { + logger.error(`Calling ${context.service} to ${context.description} threw exception`, { + service: context.service, + method: context.method, + url: context.url, + description: context.description, + error: error, + ...context.additionalLoggingFields + }) + } +} diff --git a/package-lock.json b/package-lock.json index b7507509f..073570922 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "dependencies": { "@aws-crypto/decrypt-node": "^1.0.3", "@aws-crypto/raw-rsa-keyring-node": "^1.1.0", - "@govuk-pay/pay-js-commons": "^4.0.13", + "@govuk-pay/pay-js-commons": "^5.0.2", "@govuk-pay/pay-js-metrics": "^1.0.6", "@sentry/node": "7.74.0", "cert-info": "^1.5.1", @@ -58,7 +58,7 @@ "chai-as-promised": "^7.1.1", "chai-string": "^1.4.0", "cheerio": "^1.0.0-rc.12", - "chokidar-cli": "*", + "chokidar-cli": "latest", "cypress": "^13.3.1", "dotenv": "^16.3.1", "govuk-country-and-territory-autocomplete": "^1.0.2", @@ -2125,10 +2125,11 @@ } }, "node_modules/@govuk-pay/pay-js-commons": { - "version": "4.0.13", - "resolved": "https://registry.npmjs.org/@govuk-pay/pay-js-commons/-/pay-js-commons-4.0.13.tgz", - "integrity": "sha512-ObZxfs1C8rnH2shY2ovWg98xyjwji5615pD19XEd+Yk0LZNixCm93LUGQMY1Mswk3XULDjYh06ucm1bGBOpJYQ==", + "version": "5.0.2", + "resolved": "https://registry.npmjs.org/@govuk-pay/pay-js-commons/-/pay-js-commons-5.0.2.tgz", + "integrity": "sha512-dCkLfQ5gdhXR4KA+jTGCvZ+oNVDeFiWcrmcWCRlejfItHYZ9P+tT2DGP4l56jVV6S5yKZK/y7zDqCu8/nlKDAg==", "dependencies": { + "axios": "^1.6.5", "lodash": "4.17.21", "moment-timezone": "0.5.43", "rfc822-validate": "1.0.0", @@ -3264,12 +3265,11 @@ "integrity": "sha512-xh1Rl34h6Fi1DC2WWKfxUTVqRsNnr6LsKz2+hfwDxQJWmrx8+c7ylaqBMcHfl1U1r2dsifOvKX3LQuLNZ+XSvA==" }, "node_modules/axios": { - "version": "1.6.4", - "resolved": "https://registry.npmjs.org/axios/-/axios-1.6.4.tgz", - "integrity": "sha512-heJnIs6N4aa1eSthhN9M5ioILu8Wi8vmQW9iHQ9NUvfkJb0lEEDUiIdQNAuBtfUt3FxReaKdpQA5DbmMOqzF/A==", - "dev": true, + "version": "1.6.8", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.6.8.tgz", + "integrity": "sha512-v/ZHtJDU39mDpyBoFVkETcd/uNdxrWRrg3bKpOKzXFA6Bvqopts6ALSMU3y6ijYxbw2B+wPrIv46egTzJXCLGQ==", "dependencies": { - "follow-redirects": "^1.15.4", + "follow-redirects": "^1.15.6", "form-data": "^4.0.0", "proxy-from-env": "^1.1.0" } @@ -3278,7 +3278,6 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.0.tgz", "integrity": "sha512-ETEklSGi5t0QMZuiXoA/Q6vcnxcLQP5vdugSpuAyi6SVGi2clPPp+xgEhuMaHC+zGgn31Kd235W35f7Hykkaww==", - "dev": true, "dependencies": { "asynckit": "^0.4.0", "combined-stream": "^1.0.8", @@ -7307,7 +7306,6 @@ "version": "1.15.6", "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.6.tgz", "integrity": "sha512-wWN62YITEaOpSK584EZXJafH1AGpO8RVgElfkuXbTOrPX4fIfOyEpW/CsiNd8JdYrAoOvafRTOEnvsO++qCqFA==", - "dev": true, "funding": [ { "type": "individual", @@ -13314,8 +13312,7 @@ "node_modules/proxy-from-env": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-1.1.0.tgz", - "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==", - "dev": true + "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==" }, "node_modules/proxyquire": { "version": "2.1.3", @@ -18547,10 +18544,11 @@ } }, "@govuk-pay/pay-js-commons": { - "version": "4.0.13", - "resolved": "https://registry.npmjs.org/@govuk-pay/pay-js-commons/-/pay-js-commons-4.0.13.tgz", - "integrity": "sha512-ObZxfs1C8rnH2shY2ovWg98xyjwji5615pD19XEd+Yk0LZNixCm93LUGQMY1Mswk3XULDjYh06ucm1bGBOpJYQ==", + "version": "5.0.2", + "resolved": "https://registry.npmjs.org/@govuk-pay/pay-js-commons/-/pay-js-commons-5.0.2.tgz", + "integrity": "sha512-dCkLfQ5gdhXR4KA+jTGCvZ+oNVDeFiWcrmcWCRlejfItHYZ9P+tT2DGP4l56jVV6S5yKZK/y7zDqCu8/nlKDAg==", "requires": { + "axios": "^1.6.5", "lodash": "4.17.21", "moment-timezone": "0.5.43", "rfc822-validate": "1.0.0", @@ -19463,12 +19461,11 @@ "integrity": "sha512-xh1Rl34h6Fi1DC2WWKfxUTVqRsNnr6LsKz2+hfwDxQJWmrx8+c7ylaqBMcHfl1U1r2dsifOvKX3LQuLNZ+XSvA==" }, "axios": { - "version": "1.6.4", - "resolved": "https://registry.npmjs.org/axios/-/axios-1.6.4.tgz", - "integrity": "sha512-heJnIs6N4aa1eSthhN9M5ioILu8Wi8vmQW9iHQ9NUvfkJb0lEEDUiIdQNAuBtfUt3FxReaKdpQA5DbmMOqzF/A==", - "dev": true, + "version": "1.6.8", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.6.8.tgz", + "integrity": "sha512-v/ZHtJDU39mDpyBoFVkETcd/uNdxrWRrg3bKpOKzXFA6Bvqopts6ALSMU3y6ijYxbw2B+wPrIv46egTzJXCLGQ==", "requires": { - "follow-redirects": "^1.15.4", + "follow-redirects": "^1.15.6", "form-data": "^4.0.0", "proxy-from-env": "^1.1.0" }, @@ -19477,7 +19474,6 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.0.tgz", "integrity": "sha512-ETEklSGi5t0QMZuiXoA/Q6vcnxcLQP5vdugSpuAyi6SVGi2clPPp+xgEhuMaHC+zGgn31Kd235W35f7Hykkaww==", - "dev": true, "requires": { "asynckit": "^0.4.0", "combined-stream": "^1.0.8", @@ -22680,8 +22676,7 @@ "follow-redirects": { "version": "1.15.6", "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.6.tgz", - "integrity": "sha512-wWN62YITEaOpSK584EZXJafH1AGpO8RVgElfkuXbTOrPX4fIfOyEpW/CsiNd8JdYrAoOvafRTOEnvsO++qCqFA==", - "dev": true + "integrity": "sha512-wWN62YITEaOpSK584EZXJafH1AGpO8RVgElfkuXbTOrPX4fIfOyEpW/CsiNd8JdYrAoOvafRTOEnvsO++qCqFA==" }, "for-each": { "version": "0.3.3", @@ -27264,8 +27259,7 @@ "proxy-from-env": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-1.1.0.tgz", - "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==", - "dev": true + "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==" }, "proxyquire": { "version": "2.1.3", diff --git a/package.json b/package.json index 29fab60b2..651a3eed5 100644 --- a/package.json +++ b/package.json @@ -72,7 +72,7 @@ "dependencies": { "@aws-crypto/decrypt-node": "^1.0.3", "@aws-crypto/raw-rsa-keyring-node": "^1.1.0", - "@govuk-pay/pay-js-commons": "^4.0.13", + "@govuk-pay/pay-js-commons": "^5.0.2", "@govuk-pay/pay-js-metrics": "^1.0.6", "@sentry/node": "7.74.0", "cert-info": "^1.5.1", diff --git a/server.js b/server.js index f1afdea22..ac25395de 100644 --- a/server.js +++ b/server.js @@ -25,6 +25,7 @@ const Sentry = require('./app/utils/sentry.js').initialiseSentry() const { setReportingEndpoints, worldpayIframe } = require('./app/middleware/csp') const correlationHeader = require('./app/middleware/correlation-header') const errorHandlers = require('./app/middleware/error-handlers') +const { requestContextMiddleware } = require('./app/services/clients/base/request-context') // Global constants const { @@ -61,6 +62,7 @@ function initialiseGlobalMiddleware (app) { app.use(staticify.middleware) app.use(function (req, res, next) { + app.use(requestContextMiddleware) res.locals.asset_path = '/public/' res.locals.googlePayMerchantID = GOOGLE_PAY_MERCHANT_ID if (typeof ANALYTICS_TRACKING_ID === 'undefined') { @@ -82,6 +84,8 @@ function initialiseGlobalMiddleware (app) { app.use(compression()) app.disable('x-powered-by') + + app.use(correlationHeader) }