-
Notifications
You must be signed in to change notification settings - Fork 280
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
Fix ActionMailer plugin to add rescue_from
to the correct class
#1143
Changes from all commits
d3e0029
4d4f391
abd5156
eea33a5
2dcaa5a
ea734f7
752d68b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,35 +1,44 @@ | ||
module Rollbar | ||
# Report any uncaught errors in a job to Rollbar and reraise | ||
module ActiveJob | ||
def self.included(base) | ||
base.send :rescue_from, Exception do |exception| | ||
args = if self.class.respond_to?(:log_arguments?) && !self.class.log_arguments? | ||
arguments.map(&Rollbar::Scrubbers.method(:scrub_value)) | ||
else | ||
arguments | ||
end | ||
Rollbar.plugins.define('active_job') do | ||
dependency { !configuration.disable_monkey_patch } | ||
dependency { !configuration.disable_action_mailer_monkey_patch } | ||
|
||
Rollbar.error(exception, | ||
:job => self.class.name, | ||
:job_id => job_id, | ||
:use_exception_level_filters => true, | ||
:arguments => args) | ||
raise exception | ||
execute do | ||
module Rollbar | ||
# Report any uncaught errors in a job to Rollbar and reraise | ||
module ActiveJob | ||
def self.included(base) | ||
base.send :rescue_from, Exception do |exception| | ||
args = if self.class.respond_to?(:log_arguments?) && !self.class.log_arguments? | ||
arguments.map(&Rollbar::Scrubbers.method(:scrub_value)) | ||
else | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @waltjones, I noticed that the rollbar update to 3.5.0 broke one of the mailers of our app and tracked it down here: I assume this was fixed on the tests here with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lucasmazza Thank you for the report. I'm working to get a fix out today. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lucasmazza Fix is released in 3.5.1. https://github.com/rollbar/rollbar-gem/releases/tag/v3.5.1 |
||
arguments | ||
end | ||
|
||
job_data = { | ||
:job => self.class.name, | ||
:use_exception_level_filters => true, | ||
:arguments => args | ||
} | ||
|
||
# job_id isn't present when this is a mailer class. | ||
job_data[:job_id] = job_id if defined?(job_id) | ||
|
||
Rollbar.error(exception, job_data) | ||
raise exception | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end | ||
|
||
if defined?(ActiveSupport) && ActiveSupport.respond_to?(:on_load) | ||
ActiveSupport.on_load(:action_mailer) do | ||
# Automatically add to ActionMailer::DeliveryJob | ||
if defined?(ActionMailer::DeliveryJob) | ||
ActionMailer::DeliveryJob.send(:include, | ||
Rollbar::ActiveJob) | ||
end | ||
# Rails >= 6.0 | ||
if defined?(ActionMailer::MailDeliveryJob) | ||
ActionMailer::MailDeliveryJob.send(:include, Rollbar::ActiveJob) | ||
if defined?(ActiveSupport) && ActiveSupport.respond_to?(:on_load) | ||
ActiveSupport.on_load(:action_mailer) do | ||
if defined?(ActionMailer::MailDeliveryJob) # Rails >= 6.0 | ||
ActionMailer::Base.send(:include, Rollbar::ActiveJob) | ||
elsif defined?(ActionMailer::DeliveryJob) # Rails < 6.0 | ||
ActionMailer::DeliveryJob.send(:include, | ||
Rollbar::ActiveJob) | ||
end | ||
end | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋 - this change produces:
which means that https://docs.rollbar.com/docs/activejob-integration is no longer true. Is there a change required to the documentation?
https://docs.rollbar.com/docs/gem-plugins is unclear on how plugins are loaded, so we were unable to experiment with a solution.