Issue with "on_thread_error" override about solid_queue HOT 8 OPEN

@rosa : Thanks for clarification and for this wonderful gem. Your explanation clears up the confusion I had.

@kylekeesling : Thanks for the above snippet. I used the same and got it working for ApplicationJob by directly calling Honeybadger.notify.

class ApplicationJob < ActiveJob::Base
  around_perform do |job, block|
    capture_and_record_errors(job, block)
  end

  def capture_and_record_errors(job, block)
    block.call
  rescue => exception
    context = {
      error_class: job.class.name,
      args: job.arguments,
      scheduled_at: job.scheduled_at,
      job_id: job.job_id
    }
    Honeybadger.notify(exception, context:)
    raise exception
  end
end

I also added similar code to the around block for ActionMailer::MailDeliveryJob to make it work for Mailers.

# application_mailer.rb

ActionMailer::MailDeliveryJob.around_perform do |job, block|
  block.call
rescue => exception
  context = {
    error_class: job.class.name,
    args: job.arguments,
    scheduled_at: job.scheduled_at,
    job_id: job.job_id
  }
  Honeybadger.notify(exception, context:)
  raise exception
end

While searching for ways on why just adding it to application_job.rb wasn't working for mailers, I came across this section in the readme of good_job gem which helped clear up the confusion and provide the above solution for mailers.

@rosa : Do you think it would be a good thing to add both of the above in the readme? If yes, then I would be happy to raise a PR.

from solid_queue.

Using the Sentry example Rosa provided, I was able to get the following working in my app by adding it to ApplicationJob:

class ApplicationJob < ActiveJob::Base
  around_perform do |job, block|
    capture_and_record_errors(job, block)
  end

  def capture_and_record_errors(job, block)
    block.call
  # I had to use rescue here instead of a `Rails.error` block because Honeybadger ignores the `Rails.error.report` call
  # in favor of their own error handler, which is fine in most cases, but unfortunately doesn't work here. Report would be
  # great here because it re-raises the error, but instead I have to do that manually
  rescue Exception => e 
    Rails.error.set_context(**error_context(job))
    Rails.error.report(e)
    raise e
  end

  def error_context(job)
    {
      active_job: job.class.name,
      arguments: job.arguments,
      scheduled_at: job.scheduled_at,
      job_id: job.job_id
    }
  end
end

It's important to note that you must include Exception in the error handler as opposed to omitting it or defining StandardError. You also don't have to include the context, but figured it couldn't hurt and was easy to add.

Thanks again @rosa for the wonderful gem and the assistance in figuring this out!

from solid_queue.

@rosa : I have raised a PR #139

from solid_queue.

Doesn't seem like on_thread_error is ever called. If I bundle open solid_queue and throw a bunch of puts "here" into the default on_thread_error proc, nothing gets outputted to development.log

from solid_queue.

Likewise, I have the following config and it does not notify Honeybadger when an error occurs:

config.solid_queue.on_thread_error = ->(exception) {
  Honeybadger.notify(exception, context: {error_message: exception.message, source: "solid_queue"})
}

from solid_queue.

Hey @chiraggshah, @npezza93, @kylekeesling thanks for reporting this and sorry for the delay. I've been working mostly on unrelated stuff this past week.

on_thread_error wasn't quite intended for errors on the job itself, but rather errors in the thread that's executing the job, but around the job itself. For example, if you had an Active Record's thread pool too small for your number of threads and you got an error when trying to check out a new connection, on_thread_error would be called with that.

For errors in the job itself, you could try to hook into Active Job's itself, similarly to what Sentry's client does here.

from solid_queue.

@rosa I started to wonder that based on the wording thread instead of job. Thanks for the clarification!

from solid_queue.

@rosa : Do you think it would be a good thing to add both of the above in the readme? If yes, then I would be happy to raise a PR.

Oops, sorry for the delay! I missed this mention. I think that'd be super helpful, please do raise a PR if you have time 🙏 If not, I'll address it soon.

from solid_queue.