Add GlobalNotifier to the app (#100)

https://eaflood.atlassian.net/browse/WATER-3833

Whilst working on [Flag bill runs as errored on CHA failure](#99) we realised that by no longer throwing an error and sending them back via [Boom](https://hapi.dev/module/boom/) we'd lose any logs or notifications that the request to the Charge Module had failed.

We want to log this and send a notification to our [Errbit](https://github.com/errbit/errbit) instance. Looking at what we have though, all our [notification](app/lib) logic is tied up with the [Hapi request](https://hapi.dev/api/?v=21.1.0#request).

We don't need to know the specific request which failed in this case, as it avoids passing it around. But we also want to avoid instantiating both the [Airbrake notifier](https://github.com/airbrake/airbrake-js/tree/master/packages/node) and [Pino](https://github.com/pinojs/hapi-pino) every time we want to log something.

So, as an example, our [RequestNotifierPlugin](app/plugins/request-notifier.plugin.js) takes advantage of the work the [AirbrakePlugin](app/plugins/airbrake.plugin.js) does to set up the Airbrake notifier.

We'll add a plugin that does something similar but adds the 'notifier' (our name for something that can both log and send something to Errbit) to the [Node global namespace](https://nodejs.org/api/globals.html#globals_global) instead of the request. We'll then have something we can reuse anywhere whilst instantiating only once per process.

We're aware of the 'never use globals' rule. But as with all rules, there is always an exception!

.labrc.js

@@ -15,6 +15,9 @@ module.exports = {
     '__asyncGenerator', '__asyncDelegator', '__asyncValues', '__makeTemplateObject', '__importStar', '__importDefault',
     '__classPrivateFieldGet', '__classPrivateFieldSet',
     // We also ignore globals exposed by global-agent:
-    'GLOBAL_AGENT','ROARR'
+    'GLOBAL_AGENT','ROARR',
+    // GlobalNotifier is added by us a global in a server plugin. It's how we make logging available anywhere in the app
+    // whilst avoiding having to pass it around
+    'GlobalNotifier'
   ].join(',')
 }

app/lib/global-notifier.lib.js

@@ -0,0 +1,42 @@
+'use strict'
+
+/**
+ * @module GlobalNotifierLib
+ */
+
+const BaseNotifierLib = require('./base-notifier.lib.js')
+
+/**
+ * A combined logging and Airbrake (Errbit) notification manager for actions that take place outside of a
+ * {@link https://hapi.dev/api/?v=20.1.2#request|Hapi request}
+ *
+ * Created for use with the `app/plugins/global-notifier.plugin.js`.
+ */
+class GlobalNotifierLib extends BaseNotifierLib {
+  constructor (logger, notifier) {
+    // This is here more to make it clear that we expect these args to be provided. BaseNotifierLib has the built-in
+    // ability to instantiate them if not provided. But for our use case in global-notifier.plugin.js we want to ensure
+    // we are using the existing instances.
+    if (!logger || !notifier) {
+      throw new Error('new instance of GlobalNotifierLib is missing a required argument')
+    }
+
+    super(logger, notifier)
+  }
+
+  _formatLogPacket (message, data) {
+    return {
+      message,
+      ...data
+    }
+  }
+
+  _formatNotifyPacket (message, data) {
+    return {
+      message,
+      session: data
+    }
+  }
+}
+
+module.exports = GlobalNotifierLib

app/lib/request.lib.js

@@ -49,6 +49,10 @@ async function get (url, additionalOptions = {}) {
     result.response = error
   }
 
+  if (!result.succeeded) {
+    _logFailure('GET', result, url, additionalOptions)
+  }
+
   return result
 }
 
@@ -72,6 +76,10 @@ async function post (url, additionalOptions = {}) {
     result.response = error
   }
 
+  if (!result.succeeded) {
+    _logFailure('POST', result, url, additionalOptions)
+  }
+
   return result
 }
 
@@ -85,6 +93,19 @@ async function _importGot () {
   return got
 }
 
+function _logFailure (method, result, url, additionalOptions) {
+  const data = {
+    method,
+    result,
+    url,
+    additionalOptions
+  }
+
+  const severity = result.response instanceof Error ? 'errored' : 'failed'
+
+  global.GlobalNotifier.omfg(`${method} request ${severity}`, data)
+}
+
 /**
  * Combines the custom Got options provided by the caller with our defaults
  *

app/plugins/global-notifier.plugin.js

@@ -0,0 +1,17 @@
+'use strict'
+
+/**
+ * Plugin to add a globally available notifier for logging and sending exceptions to Errbit
+ * @module GlobalNotifierPlugin
+ */
+
+const GlobalNotifierLib = require('../lib/global-notifier.lib.js')
+
+const GlobalNotifierPlugin = {
+  name: 'global-notifier',
+  register: (server, _options) => {
+    global.GlobalNotifier = new GlobalNotifierLib(server.logger, server.app.airbrake)
+  }
+}
+
+module.exports = GlobalNotifierPlugin

app/server.js

@@ -6,6 +6,7 @@ const AirbrakePlugin = require('./plugins/airbrake.plugin.js')
 const BlippPlugin = require('./plugins/blipp.plugin.js')
 const ChargingModuleTokenCachePlugin = require('./plugins/charging-module-token-cache.plugin.js')
 const ErrorPagesPlugin = require('./plugins/error-pages.plugin.js')
+const GlobalNotifierPlugin = require('./plugins/global-notifier.plugin.js')
 const HapiPinoPlugin = require('./plugins/hapi-pino.plugin.js')
 const RequestNotifierPlugin = require('./plugins/request-notifier.plugin.js')
 const RouterPlugin = require('./plugins/router.plugin.js')
@@ -21,6 +22,7 @@ const registerPlugins = async (server) => {
   await server.register(RouterPlugin)
   await server.register(HapiPinoPlugin())
   await server.register(AirbrakePlugin)
+  await server.register(GlobalNotifierPlugin)
   await server.register(ChargingModuleTokenCachePlugin)
   await server.register(ErrorPagesPlugin)
   await server.register(RequestNotifierPlugin)

test/lib/global-notifier.lib.test.js

@@ -0,0 +1,80 @@
+'use strict'
+
+// Test framework dependencies
+const Lab = require('@hapi/lab')
+const Code = require('@hapi/code')
+const Sinon = require('sinon')
+
+const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script()
+const { expect } = Code
+
+// Things we need to stub
+const BaseNotifierLib = require('../../app/lib/base-notifier.lib.js')
+
+// Thing under test
+const GlobalNotifierLib = require('../../app/lib/global-notifier.lib.js')
+
+describe('GlobalNotifierLib class', () => {
+  let airbrakeFake
+  let pinoFake
+
+  beforeEach(async () => {
+    airbrakeFake = { notify: Sinon.fake.resolves({ id: 1 }), flush: Sinon.fake() }
+    Sinon.stub(BaseNotifierLib.prototype, '_setNotifier').returns(airbrakeFake)
+
+    pinoFake = { info: Sinon.fake(), error: Sinon.fake() }
+    Sinon.stub(BaseNotifierLib.prototype, '_setLogger').returns(pinoFake)
+  })
+
+  afterEach(() => {
+    Sinon.restore()
+  })
+
+  describe('#constructor', () => {
+    describe("when the 'logger' argument is not provided", () => {
+      it('throws an error', () => {
+        expect(() => new GlobalNotifierLib(null, airbrakeFake)).to.throw()
+      })
+    })
+
+    describe("when the 'notifier' argument is not provided", () => {
+      it('throws an error', () => {
+        expect(() => new GlobalNotifierLib(pinoFake)).to.throw()
+      })
+    })
+  })
+
+  describe('when a log entry is made', () => {
+    const id = '1234567890'
+    const message = 'say what test'
+
+    it('formats it as expected', () => {
+      const expectedArgs = {
+        message,
+        id
+      }
+      const testNotifier = new GlobalNotifierLib(pinoFake, airbrakeFake)
+      testNotifier.omg(message, { id })
+
+      expect(pinoFake.info.calledOnceWith(expectedArgs)).to.be.true()
+    })
+  })
+
+  describe('when an airbrake notification is sent', () => {
+    const message = 'hell no test'
+    const data = { offTheChart: true }
+
+    it('formats it as expected', () => {
+      const expectedArgs = {
+        message,
+        session: {
+          ...data
+        }
+      }
+      const testNotifier = new GlobalNotifierLib(pinoFake, airbrakeFake)
+      testNotifier.omfg(message, data)
+
+      expect(airbrakeFake.notify.calledOnceWith(expectedArgs)).to.be.true()
+    })
+  })
+})

test/lib/request.lib.test.js

@@ -17,10 +17,20 @@ const RequestLib = require('../../app/lib/request.lib.js')
 
 describe('RequestLib', () => {
   const testDomain = 'http://example.com'
+  let notifierStub
+
+  beforeEach(() => {
+    // RequestLib depends on the GlobalNotifier to have been set. This happens in app/plugins/global-notifier.plugin.js
+    // when the app starts up and the plugin is registered. As we're not creating an instance of Hapi server in this
+    // test we recreate the condition by setting it directly with our own stub
+    notifierStub = { omfg: Sinon.stub() }
+    global.GlobalNotifier = notifierStub
+  })
 
   afterEach(() => {
     Sinon.restore()
     Nock.cleanAll()
+    delete global.GlobalNotifier
   })
 
   describe('#get()', () => {
@@ -52,6 +62,12 @@ describe('RequestLib', () => {
           Nock(testDomain).get('/').reply(500, { data: 'hello world' })
         })
 
+        it('records the failure', async () => {
+          await RequestLib.get(testDomain)
+
+          expect(notifierStub.omfg.calledWith('GET request failed')).to.be.true()
+        })
+
         describe('the result it returns', () => {
           it("has a 'succeeded' property marked as false", async () => {
             const result = await RequestLib.get(testDomain)
@@ -74,6 +90,12 @@ describe('RequestLib', () => {
           Nock(testDomain).get('/').replyWithError({ code: 'ECONNRESET' })
         })
 
+        it('records the error', async () => {
+          await RequestLib.get(testDomain)
+
+          expect(notifierStub.omfg.calledWith('GET request errored')).to.be.true()
+        })
+
         describe('the result it returns', () => {
           it("has a 'succeeded' property marked as false", async () => {
             const result = await RequestLib.get(testDomain)
@@ -218,6 +240,12 @@ describe('RequestLib', () => {
           Nock(testDomain).post('/').reply(500, { data: 'hello world' })
         })
 
+        it('records the failure', async () => {
+          await RequestLib.post(testDomain)
+
+          expect(notifierStub.omfg.calledWith('POST request failed')).to.be.true()
+        })
+
         describe('the result it returns', () => {
           it("has a 'succeeded' property marked as false", async () => {
             const result = await RequestLib.post(testDomain)
@@ -240,6 +268,12 @@ describe('RequestLib', () => {
           Nock(testDomain).post('/').replyWithError({ code: 'ECONNRESET' })
         })
 
+        it('records the error', async () => {
+          await RequestLib.post(testDomain)
+
+          expect(notifierStub.omfg.calledWith('POST request errored')).to.be.true()
+        })
+
         describe('the result it returns', () => {
           it("has a 'succeeded' property marked as false", async () => {
             const result = await RequestLib.post(testDomain)

test/plugins/global-notifier.plugin.test.js

@@ -0,0 +1,31 @@
+'use strict'
+
+// Test framework dependencies
+const Lab = require('@hapi/lab')
+const Code = require('@hapi/code')
+
+const { describe, it, beforeEach } = exports.lab = Lab.script()
+const { expect } = Code
+
+// Test helpers
+const GlobalNotifierLib = require('../../app/lib/global-notifier.lib.js')
+
+// For running our service
+const { init } = require('../../app/server.js')
+
+describe('Global Notifier plugin', () => {
+  beforeEach(async () => {
+    // Create server before each test
+    await init()
+  })
+
+  describe('Global Notifier Plugin', () => {
+    describe('when the server is initialised', () => {
+      it('makes an instance of GlobalNotifierLib available globally', async () => {
+        const result = global.GlobalNotifier
+
+        expect(result).to.be.an.instanceOf(GlobalNotifierLib)
+      })
+    })
+  })
+})