exception_notification icon indicating copy to clipboard operation
exception_notification copied to clipboard

enhance ignore_if to allow "by-notifier" customization

Open fursich opened this issue 7 years ago • 1 comments

Hello, thanks a million maintaining this great gem!

Here's something I hope this gem to have as a new feature - I'm very happy to PR if that makes sense.

OPTION PROPOSAL (no breaking changes)

Just wanted to propose a new API that enables us to selectively ignore exceptions on notifier basis - something like:

Rails.application.config.middleware.use ExceptionNotification::Rack,
  :ignore_notifier_if => ->(env, exception, notifier) {
    exception.message =~ /^A Not-So-Important-Yet-Needs-To-Be-Logged Exception/ &&  \
     notifier == :slack_notifier
  },
  :email => {
    :email_prefix         => "[PREFIX] ",
    :sender_address       => %{"notifier" <[email protected]>},
    :exception_recipients => %w{[email protected]},
  },
  :slack => {
    :webhook_url => "[Your webhook url]",
    :channel => "#major_exceptions",
  }

  # Keep ignore_if option as it is now, so that it continues to be available for
  # lamda's with only two arguments

Or more simply, by using config

ExceptionNotification.configure do |config|
  config.ignore_if do |exception, options, notifier|
    exception.message =~ /^A Not-So-Important-Yet-Needs-To-Be-Logged Error/ &&  \
    notifier == :slack_notifier
  end

  # this also accepts two arguments as it used to do (notifier optional)
  config.ignore_if do |exception, options|
    exception.message =~ /^Another Error That Needs To Be Totally Ignored/
  end
end

Again, there is no breaking changes (back compatible) - current API is available as it is. Just wanted to make a new argument available.

Do you have any thoughts/advises on this..? I'll be submitting a PR for this to show the details above with some decent tests attached - hope that makes sense!

USE CASE

We need this as we have multiple notifiers to serve for different purposes - we need our slack to alert only major incidents, but we still want to collect all the error logs just in case using other notifiers.

fursich avatar May 25 '18 15:05 fursich

I'd really like to see some way to do this. My first idea was to add ignore_if on a per-notifier basis. Global ignore_if for all notifiers + local ignore_if just for the current notifier:

Rails.application.config.middleware.use ExceptionNotification::Rack,
  :ignore_if => ->(env, exception) { exception.message =~ /^A Just-Ignore-Me-Everywhere Exception/ },
  :email => {
    :email_prefix         => "[PREFIX] ",
    :sender_address       => %{"notifier" <[email protected]>},
    :exception_recipients => %w{[email protected]},
  },
  :slack => {
    :webhook_url => "[Your webhook url]",
    :channel => "#major_exceptions",
    :ignore_if => ->(env, exception) { exception.message =~ /^A Not-So-Important-Yet-Needs-To-Be-Logged Exception/ },
  }

LeEnno avatar Aug 14 '18 13:08 LeEnno