Fix Airbrake integration #231
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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](airbrake/airbrake-js#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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We use the Airbrake package to handle sending error notifications to our Errbit instance. This is how we can see and track errors without having to scan production logs.
A notification can be sent
omfg()methodsWell, that's what it should have been doing! We spotted when calling our
/health/airbrakeendpoint that nothing was reaching Errbit. After some investigation, we found 2 issues.The first was a new config item
errorNotificationsintroduced in Add support for remote notifier config.This should default to
trueif 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'snotify()method. So, we would see the following in our logsSetting it to
truewhen instantiating the Notifier instance resolved this issue (N.B. whilst there we also setremoteConfigtofalsejust 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 from being fired. Because of this, our logic in the Airbrake plugin wasn't being triggered.
We fix this by using the
RequestNotifierwe add to every request within the error pages plugin instead of usingserver.logger.error()directly. Using that gets us both a log message and an Airbrake notification.