Better handle errors in Notifiers

[Cruikshanks]

We spotted this issue when working in another service where we have copied the app/lib/base-notifier.lib.js and app/lib/global-notifier.lib.js across. At the points we wanted to log an error we were calling global.GlobalNotifier.omfg('import.charge-versions: errored', error). When we got an error though, this is what we saw.

In the logs, it would just be

0|import  | [15:44:40.837] ERROR (8616):
0|import  |     message: "import.charge-versions: errored"

Errbit would get the notification but if you tried to view it you'd get an error.

The main issue with the logs was we were treating instances of Error as if they were just a POJO when they are not.

const myError = new Error('boom')

const myObject = {
  ...myError
}

console.log(myObject) // {}

We also went back to the pino docs and learnt that the log packet we were generating no longer matched what pino was expecting. Specifically, we refreshed our understanding of the mergingObject and realised that we would be better placed to keep any data that needs to be logged separately from the message.

We also learnt that pino knows how to log errors, including custom ones with additional properties as long as either

• the error instance is passed into the log call as the mergingObject
• we mimic what pino does and wrap the error instance in a new object with the property err:

For Airbrake and Errbit uncaught errors were coming through and displaying fine. It was those caught in our logic and passed through as the data argument to omfg(). The key difference was app/plugins/airbrake.plugin.js when calling notify() passes in the error instance as the error: property, not session:.

// When Hapi emits a request event with an error we capture the details and use Airbrake to send a request to our
    // Errbit instance
    server.events.on({ name: 'request', channels: 'error' }, (request, event, _tags) => {
      server.app.airbrake
        .notify({
          error: event.error,
          session: {
            route: request.route.path,
            method: request.method,
            url: request.url.href
          }
        })

If we did the same then the error would always display in Errbit. We also found the grouping of errors vastly improved.

All this means we are making the following changes

• to ensure things are passed through to the logger and errbit correctly, formatting of the arguments for both is moved to BaseNotifier rather than forcing extending classes to do their own thing
• callers still have the flexibility to just set a message for both omg() and omfg(). But behind the scenes we always generate an Error instance and pass this to the Pino and Airbrake to ensure consistency

The result is this. Given the following calls to omg() and omfg()

const myError = new ErrorWithDate('Thing go pop')
const myData = { thingyId: 'ece18f30-1a73-4579-b752-5ca2607671bf', status: 'snazzy' }

global.GlobalNotifier.omg('Keep calm. I am an example of using omg()', myData)
global.GlobalNotifier.omfg('Panic! I am an example of using omfg()', myData, myError)

We see the following log output (pretty-printed local version)

[15:57:10.672] INFO (10292): Keep calm. I am an example of using omg()
    thingyId: "ece18f30-1a73-4579-b752-5ca2607671bf"
    status: "snazzy"
[15:57:10.673] ERROR (10292): Panic! I am an example of using omfg()
    thingyId: "ece18f30-1a73-4579-b752-5ca2607671bf"
    status: "snazzy"
    err: {
      "type": "ErrorWithDate",
      "message": "Thing go pop",
      "stack":
          Error: Thing go pop
              at index (/home/repos/water-abstraction-system/app/controllers/root.controller.js:16:19)
              at exports.Manager.execute (/home/repos/water-abstraction-system/node_modules/@hapi/hapi/lib/toolkit.js:57:29)
              at Object.internals.handler (/home/repos/water-abstraction-system/node_modules/@hapi/hapi/lib/handler.js:46:48)
              at exports.execute (/home/repos/water-abstraction-system/node_modules/@hapi/hapi/lib/handler.js:31:36)
              at Request._lifecycle (/home/repos/water-abstraction-system/node_modules/@hapi/hapi/lib/request.js:370:68)
              at processTicksAndRejections (node:internal/process/task_queues:96:5)
              at async Request._execute (/home/repos/water-abstraction-system/node_modules/@hapi/hapi/lib/request.js:280:9)
      "dateItHappened": "2023-06-20T15:57:10.672Z"
    }

In Errbit all errors are viewable. Also, it better groups the notifications.

Screenshot of app errors page

Screenshot of error summary page

Screenshot of error session page

[Jozzey]