honeybadger-io / honeybadger-ruby

Ruby gem for reporting errors to honeybadger.io
https://docs.honeybadger.io/lib/ruby/
MIT License
250 stars 146 forks source link

Exceptions raised in jobs from Solid Queue are not automatically reported #518

Closed subzero10 closed 8 months ago

subzero10 commented 10 months ago

First reported here:

It turns out that Honeybadger by default hooks into the error reporter, but for some reason it does not report errors that report handled: false.

Reproducible example here.

subzero10 commented 10 months ago

It looks like the issue is here.

joshuap commented 10 months ago

It looks like the issue is here.

Based on the comment in that file, I think the issue is that we let our rack middleware catch unhandled errors in web requests, but we have no such catchall middleware for ActiveJob. Does that sound right, @shalvah?

Does anyone have an opinion on the best way to fix it?

shalvah commented 10 months ago

Yeah, that's the case. To elaborate, we could decide to enable the Rails error reporter for all cases, but when errors are raised in third-party systems like Sidekiq, the Rails error reporter doesn't have access to any useful context about the job (as at then, at least). By contrast, our middleware integrates with Sidekiq, so it has more context accessible.

One fix here would be to look at the source parameter and check it it's from Solid Queue (dunno what value it uses), and then go ahead to report it.

subzero10 commented 10 months ago

One fix here would be to look at the source parameter and check it it's from Solid Queue (dunno what value it uses), and then go ahead to report it.

Can we have a register of the middleware we have defined and report if the source is not in that register instead of just checking the handled flag? Or even a combination of these two checks?

shalvah commented 10 months ago

The source parameter has nothing to do with our middleware... It's an entirely new concept that was introduced with the error reporter, so checking what middleware is registered won't do much, unless you also maintain a mapping of middleware to sources. But also note that the source is not very reliable (an integration has to set it, or it defaults to active_support).

subzero10 commented 10 months ago

unless you also maintain a mapping of middleware to sources

Yes, that's what I was suggesting. Would you be against that? Though, if source is not reliable, it doesn't make sense to do it.

One fix here would be to look at the source parameter and check it it's from Solid Queue (dunno what value it uses), and then go ahead to report it.

I guess we are left with this solution? I can go ahead and try it if you haven't started working on it already.

shalvah commented 10 months ago

Oh no, I'm not actively contributing; I only chimed in to provide context, since I was the original implementer.

searls commented 9 months ago

Hello! Was just about to email Josh before checking to see if an issue was open for this. I just spent a few hours feeling REALLY DUMB trying to figure out why activejob (backed by solid queue in my case) wasn't being reported by honeybadger default (especially since I couldn't find any mention in the documentation about a manual setup step for reporting errors from failed jobs, I assumed this was being done by the gem)

searls commented 9 months ago

For anyone stumbling on this via Google, this is what I did to trigger notifications manually (and to print errors to stdout outside production, which solid queue's CLI does not do by default):

class ApplicationJob < ActiveJob::Base
  rescue_from(StandardError) do |error|
    if Rails.env.production?
      Honeybadger.notify(error, {
        controller: self.class.name,
        context: {
          job_id:,
          arguments:
        }
      })
    else
      SolidQueue.logger.error "Exception occurred processing job #{job_id}: #{error.message}"
      SolidQueue.logger.error error.backtrace.join("\n")
    end
  end
end
stympy commented 8 months ago

Here’s another example of working around the issue.