Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
subzero10 opened this issue Jan 5, 2024 · 10 comments · Fixed by #526 or #527
Closed

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

subzero10 opened this issue Jan 5, 2024 · 10 comments · Fixed by #526 or #527
Labels

Comments

@subzero10
Copy link
Member

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
Copy link
Member Author

It looks like the issue is here.

@joshuap
Copy link
Member

joshuap commented Jan 5, 2024

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
Copy link
Contributor

shalvah commented Jan 6, 2024

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
Copy link
Member Author

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
Copy link
Contributor

shalvah commented Jan 10, 2024

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
Copy link
Member Author

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
Copy link
Contributor

shalvah commented Jan 12, 2024

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

@joshuap joshuap added the bug label Jan 16, 2024
@searls
Copy link

searls commented Jan 28, 2024

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
Copy link

searls commented Jan 28, 2024

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
Copy link
Member

stympy commented Mar 5, 2024

Here’s another example of working around the issue.

stympy added a commit that referenced this issue Mar 5, 2024
And any other ActiveJob adapters that come along :)

Fixes #518
stympy added a commit that referenced this issue Mar 5, 2024
And any other ActiveJob adapters that come along :)

Fixes #518
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants