honeybadger-ruby icon indicating copy to clipboard operation
honeybadger-ruby copied to clipboard

Creating another agent for seperate project stuff is difficult and cumbersome

Open Senjai opened this issue 7 years ago • 6 comments

Here's an example on how I've accomplished this for our rails project. (sanitized some NDA stuff out)

Problem 1 - Configuration

module ProjectNamespace
  module Netsuite
    Honeybadger = ::Honeybadger::Agent.new
  end
end

This is the reporter I will be using for our Netsuite interactions. The documentation tells me I should then configure it. Easily done, here's what is done in config/initializers/netsuite.rb (after NS config)

ProjectNamespace::Netsuite::Honeybadger.configure do |config|   
  config.api_key = Figaro.env.honeybadger_erp_key       
end                                                     

Ideally this would be it. But this gives me a notifier without the rails dressing like Honeybadger does. I get it, but in order to find out how to replace it I had to clone down the project and dig into the railtie to see what it was doing differently. This is as of yet undocumented (possibly unsupported). I ended up with this:

ProjectNamespace::Netsuite::Honeybadger.init!({
  root:      ::Rails.root.to_s,
  env:       ::Rails.env,
  logger:    ::Honeybadger::Logging::FormattedLogger.new(::Rails.logger),
  framework: :rails
})

ProjectNamespace::Netsuite::Honeybadger.configure do |config|
  config.api_key = Figaro.env.honeybadger_erp_key
end

Done. This gives me the exact same behavior as the global agent. I ignore the yaml configuration here on purpose since I don't need it.

Problem 2 - Usage

Now, when I use my error reporter, I have something that looks like the following pseudocode:

def do_things
  attempt_thing
rescue StandardError => e
  ProjectNamespace::Netsuite::Honeybadger.notify(e)
  raise e
end

We re raise because we want the exception to actually occur here. The problem is that the re raised exception then gets caught by the global honeybadger middleware and is reported to the main project.

This means I now have to add context or wrap my notify calls to include something special so I can tell the main honeybadger via ::Honeybadger.exception_filters to ignore the ones that are handled by the Netsuite exception reporter.

This is a poor experience :(

Suggestion

def do_things
  ::Honeybadger.with_agent(ProjectNamespace::Netsuite::Honeybadger) do
    attempt_thing
  end
end

I think sentry exposes something similar to this via Raven.capture do ... end Is this possible at all? Ideally honeybadger would use the appropriate agent to capture any exceptions raised within the block.

Senjai avatar Jul 05 '17 21:07 Senjai

Hey @Senjai, thanks for the feedback/suggestions here. Just wanted to let you know that I'm in the process of evaluating this and will get back to you.

joshuap avatar Jul 17 '17 20:07 joshuap

@joshuap Thanks!

In case you're curious, this is largely what we've settled on, if you wanted to see what it's kind of like to have a seperate agent coexist with a global one.

module ProjectNamespace
  module Netsuite
    class Honeybadger < SimpleDelegator
      class NSCaughtException < StandardError
        def initialize(*data)
          @exception_data = data
        end

        def message
          "NS Exception caught and reported: #{@exception_data}"
        end
      end

      def self.agent
        return @agent if @agent

        @agent = ::Honeybadger::Agent.new
        @agent.init!(
          root:      ::Rails.root.to_s,
          env:       ::Rails.env,
          logger:    ::Honeybadger::Logging::FormattedLogger.new(::Rails.logger),
          framework: :rails
        )

        @agent.configure do |config|
          config.api_key = Figaro.env.honeybadger_erp_key
        end

        @agent.exception_filter do |notice|
          notice[:exception].class == ProjectNamespace::Netsuite::Honeybadger::NSCaughtException
        end

        @agent
      end

      def initialize
        @agent = self.class.agent
        super(@agent)
      end

      def notify(exception, opts = {})
        opts = opts.dup

        if exception.respond_to?(:honeybadger_context)
          opts[:context] ||= {}
          opts[:context].merge!(exception.honeybadger_context)
        end

        super(exception, opts)
      end

      # See https://github.com/honeybadger-io/honeybadger-ruby/issues/231
      def notify_with_reraise!(exception, opts = {})
        notify(exception, opts)

        raise NSCaughtException.new(exception, opts)
      end
    end
  end
end

Senjai avatar Jul 17 '17 21:07 Senjai

An update on this:

I have played with adding something like Honeybadger.with_agent(other_agent) (which swaps out the global agent instance during its block's execution). The issue I've run into is that it doesn't always work as expected; for instance, if you use it in a Rails controller in the around_action callback and an exception is raised in the controller, the exception will be reported to the default agent because the Honeybadger.with_agent block will have already returned by the time the Rack middleware calls Honeybadger.notify.

We could add something like:

Honeybadger.capture(agent) do
  # Any exceptions here will be reported to `agent`
end

That's different, though, because you'd still need to handle the exception afterwards so that it isn't reported to the default agent later in the stack. So I don't think that's an ideal solution to multiple agents, either.

Another option could be to store the alternate agent in the thread local Context store that we use for the context hash and rack environment. Not quite sure how that would work.

I don't love any of these ideas, but I'm hoping that thinking about them will help the right interface evolve. :)

joshuap avatar Oct 10 '17 00:10 joshuap

I'm also interested in this as part of what I'm trying to do in #246.

Since it seems like the API key is really what matters for swapping agents, would it make sense to make it so you can override the API key within Honeybadger.context or Honeybadger.notify? For my use case, I could get by with just switching the API key instead of the whole agent.

Would that give you enough of a handle for your use case, @Senjai?

michaelherold avatar Nov 06 '17 16:11 michaelherold

@michaelherold we actually do support api_key as an option to Honeybadger.notify, so you can do: Honeybadger.notify(err, api_key: 'alternate').

joshuap avatar Nov 06 '17 16:11 joshuap

Oo, I did not know that - I hadn't got that far into a code dive. Good to know, thank you!

michaelherold avatar Nov 06 '17 17:11 michaelherold