From 6f4288d6ea17c6f11e0e748dd08bc767747e5756 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 17 May 2023 19:16:28 +0100 Subject: [PATCH 1/3] Fix Airbrake integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We use the [Airbrake package](https://github.com/airbrake/airbrake-js/tree/master/packages/node) to handle sending error notifications to our [Errbit](https://github.com/errbit/errbit) instance. This is how we can see and track errors without having to scan production logs. A notification can be sent - manually - using one of our notifier modules `omfg()` methods - by default when hapi sees an error event Well that's what it should have been doing. We spotted when calling our `/health/airbrake` endpoint that nothing was reaching Errbit. After some investigation we found 2 issues The first was a new config item `errorNotifications` introduced in [Add support for remote notifier config](https://github.com/airbrake/airbrake-js/pull/940). This should default to `true` if not set when instantiating your Airbrake notifier. But for reasons we admittedly didn't get to the bottom of, this was not the case when we manually called Airbrake's `notify()` method. So, we would see the following in our logs ``` notify failed Error: airbrake: not sending this error, errorNotifications is disabled err="Manual error message" at Notifier.BaseNotifier.notify (/home/repos/water-abstraction-system/node_modules/@airbrake/browser/dist/base_notifier.js:110:28) at Notifier.notify (/home/repos/water-abstraction-system/node_modules/@airbrake/node/dist/notifier.js:126:40) at index (/home/repos/water-abstraction-system/app/controllers/health/airbrake.controller.js:11:47) 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) ``` Setting it to `true` when instantiating the Notifier instance resolved this issue (N.B. whilst there we also set `remoteConfig` to `false` just in case 😁) The second was our error pages plugin. In the event of an error on a route we haven't configured to use plain output the plugin ensures our error page is returned. But this stopped a Hapi error event being fired. Because of this our logic in the Airbrake plugin wasn't being triggered. We fix this by using the `RequestNotifier` we add to every request within the error pages plugin instead of using `server.logger.error()` directly. Using that gets us both a log message and an Airbrake notification. From 36891c88740d5c7a60154f4d9ef9fa67d442414d Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 17 May 2023 20:14:34 +0100 Subject: [PATCH 2/3] Implement fix --- app/plugins/airbrake.plugin.js | 4 +++- app/plugins/error-pages.plugin.js | 17 ++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/app/plugins/airbrake.plugin.js b/app/plugins/airbrake.plugin.js index 553ccfb0ac..7dfe51f199 100644 --- a/app/plugins/airbrake.plugin.js +++ b/app/plugins/airbrake.plugin.js @@ -32,7 +32,9 @@ const AirbrakePlugin = { projectId: AirbrakeConfig.projectId, projectKey: AirbrakeConfig.projectKey, environment: AirbrakeConfig.environment, - performanceStats: false + errorNotifications: true, + performanceStats: false, + remoteConfig: false }) // When Hapi emits a request event with an error we capture the details and use Airbrake to send a request to our diff --git a/app/plugins/error-pages.plugin.js b/app/plugins/error-pages.plugin.js index 28c52b53ab..f0345e2ca9 100644 --- a/app/plugins/error-pages.plugin.js +++ b/app/plugins/error-pages.plugin.js @@ -20,8 +20,13 @@ const ErrorPagesPlugin = { server.ext('onPreResponse', (request, h) => { const { response } = request + // By adding `plugins: { errorPages: { plainOutput: true }}` to a route's `options:` property you can control + // whether we display our error handling pages or not. This is handy for API only where we would not want an + // error page to be returned as the response const { errorPages: pluginSettings } = request.route.settings.plugins + // Whether we purposely return a Boom error in our controllers or not, exceptions thrown by the controllers are + // wrapped as Boom errors. So, in this context `isBoom` can be read is `isError`. if (response.isBoom && !pluginSettings?.plainOutput) { const { statusCode } = response.output @@ -29,11 +34,13 @@ const ErrorPagesPlugin = { return h.view('404').code(statusCode) } - server.logger.error({ - statusCode, - message: response.message, - stack: response.data ? response.data.stack : response.stack - }) + request.app.notifier.omfg( + response.message, + { + statusCode, + stack: response.data ? response.data.stack : response.stack + } + ) return h.view('500').code(statusCode) } From b889a43fbcf8ab8d16f307d44f8099e966c51736 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 17 May 2023 20:22:39 +0100 Subject: [PATCH 3/3] Typo in comment --- app/plugins/error-pages.plugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/plugins/error-pages.plugin.js b/app/plugins/error-pages.plugin.js index f0345e2ca9..82fc5a4c90 100644 --- a/app/plugins/error-pages.plugin.js +++ b/app/plugins/error-pages.plugin.js @@ -26,7 +26,7 @@ const ErrorPagesPlugin = { const { errorPages: pluginSettings } = request.route.settings.plugins // Whether we purposely return a Boom error in our controllers or not, exceptions thrown by the controllers are - // wrapped as Boom errors. So, in this context `isBoom` can be read is `isError`. + // wrapped as Boom errors. So, in this context `isBoom` can be read as `isError`. if (response.isBoom && !pluginSettings?.plainOutput) { const { statusCode } = response.output