Skip to content
This repository has been archived by the owner on Feb 20, 2021. It is now read-only.

Add an option for notify-environments #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sheelc
Copy link
Contributor

@sheelc sheelc commented Feb 15, 2016

Allows notifying bugsnag in only certain environments. Helpful for
avoiding exception reporting in development environment for example.
Parallels the semantics in other bugsnag libraries, such as the JS
library that has a "notifyReleaseStages"

Does this seem like a reasonable feature for the Clojure library to implement? It parallels the functionality of the JS and Java bugsnag libraries. At the moment we can wrap up the notifier function in a should-notify in our own projects, but across a lot of projects it becomes a little tedious.

@sheelc
Copy link
Contributor Author

sheelc commented Feb 19, 2016

@andreiursan Would love to get some feedback on this PR when you have a chance!

@@ -54,6 +54,9 @@
thing
(str thing)))

(defn- environment [options]
(or (:environment options) "production"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was like this before, but better code style would be to use
(:key map "defualt")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! I totally missed that, thanks. I'll update the commit with that fix

@andreiursan
Copy link
Contributor

Thanks for working on this.
I think there might be a better way to do this. I was reading the Bugsnag API.
There is this additional key that we would be able to set.

notifyReleaseStages
A list of release stages that the notifier will capture and send errors for. If the current release stage is not in this list, errors should not be sent to Bugsnag.

If we just extend the post to have this additional field, then the implementation with (defn should-notify? [{:keys [notify-environments] :as options}]) would not be necessary.

@sheelc
Copy link
Contributor Author

sheelc commented Feb 22, 2016

@andreiursan maybe I'm misreading their documentation, but notifyReleaseStages is under the section:

Notifier Configuration
When writing a notifier, you should consider providing methods to allow users to configure how errors are sent to bugsnag.

On our official notifiers, we provide the following interfaces:

As in, they like if client libraries implement this functionality. Looking at the JSON payload, they don't support a parameter called notifyReleaseStages as I think the idea is to avoid the POST entirely.

(As a bit of sidenote, I called it notify-environments instead of notify-release-stages because currently in the options the :environment option is set to releaseStage in the post, so I thought the parallel would be notify-environments?)

Please let me know what you think or if I'm missing something in your idea, thanks!

Allows notifying bugsnag in only certain environments. Helpful for
avoiding exception reporting in development environment for example.
Parallels the semantics in other bugsnag libraries, such as the JS
library that has a "notifyReleaseStages"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants