diff --git a/README.md b/README.md index d035be5e8..c1e341d98 100644 --- a/README.md +++ b/README.md @@ -319,9 +319,31 @@ you can define an error handler: ```ruby # config/initializers/maintenance_tasks.rb -MaintenanceTasks.error_handler = ->(error) { Bugsnag.notify(error) } +MaintenanceTasks.error_handler = ->(error, task_context, _errored_element) do + Bugsnag.notify(error) do |notification| + notification.add_tab(:task, task_context) + end +end ``` +The error handler should be a lambda that accepts three arguments: + +* `error`: The object containing the exception that was raised. +* `task_context`: A hash with additional information about the Task and the + error: + * `task_name`: The name of the Task that errored + * `started_at`: The time the Task started + * `ended_at`: The time the Task errored +* `errored_element`: The element, if any, that was being processed when the + Task raised an exception. If you would like to pass this object to your + exception monitoring service, make sure you **sanitize the object** to avoid + leaking sensitive data and **convert it to a format** that is compatible with + your bug tracker. For example, Bugsnag only sends the id and class name of + ActiveRecord objects in order to protect sensitive data. CSV rows, on the + other hand, are converted to strings and passed raw to Bugsnag, so make sure + to filter any personal data from these objects before adding them to a + report. + #### Customizing the maintenance tasks module `MaintenanceTasks.tasks_module` can be configured to define the module in which diff --git a/app/jobs/maintenance_tasks/task_job.rb b/app/jobs/maintenance_tasks/task_job.rb index 1440e4b57..7ad599711 100644 --- a/app/jobs/maintenance_tasks/task_job.rb +++ b/app/jobs/maintenance_tasks/task_job.rb @@ -49,11 +49,18 @@ def build_enumerator(_run, cursor:) # @param _run [Run] the current Run, passed as an argument by Job Iteration. def each_iteration(input, _run) throw(:abort, :skip_complete_callbacks) if @run.stopping? - @task.process(input) + task_iteration(input) @ticker.tick @run.reload_status end + def task_iteration(input) + @task.process(input) + rescue => error + @errored_element = input + raise error + end + def before_perform @run = arguments.first @task = Task.named(@run.task_name).new @@ -97,7 +104,14 @@ def after_perform def on_error(error) @ticker.persist if defined?(@ticker) @run.persist_error(error) - MaintenanceTasks.error_handler.call(error) + + task_context = { + task_name: @run.task_name, + started_at: @run.started_at, + ended_at: @run.ended_at, + } + errored_element = @errored_element if defined?(@errored_element) + MaintenanceTasks.error_handler.call(error, task_context, errored_element) end end end diff --git a/lib/maintenance_tasks.rb b/lib/maintenance_tasks.rb index e2126ac7b..ad0964cd1 100644 --- a/lib/maintenance_tasks.rb +++ b/lib/maintenance_tasks.rb @@ -32,6 +32,23 @@ module MaintenanceTasks # the ticker during Task iterations. mattr_accessor :ticker_delay, default: 1.second + # Retrieves the callback to be performed when an error occurs in the task. + def self.error_handler + return @error_handler if defined?(@error_handler) + @error_handler = ->(_error, _task_context, _errored_element) {} + end + # Defines a callback to be performed when an error occurs in the task. - mattr_accessor :error_handler, default: ->(_error) {} + def self.error_handler=(error_handler) + unless error_handler.arity == 3 + ActiveSupport::Deprecation.warn( + 'MaintenanceTasks.error_handler should be a lambda that takes three '\ + 'arguments: error, task_context, and errored_element.' + ) + @error_handler = ->(error, _task_context, _errored_element) do + error_handler.call(error) + end + end + @error_handler = error_handler + end end diff --git a/test/jobs/maintenance_tasks/task_job_test.rb b/test/jobs/maintenance_tasks/task_job_test.rb index 7b11472ec..51ed5b7d2 100644 --- a/test/jobs/maintenance_tasks/task_job_test.rb +++ b/test/jobs/maintenance_tasks/task_job_test.rb @@ -262,19 +262,31 @@ class TaskJobTest < ActiveJob::TestCase test '.perform_now calls the error handler when there was an Error' do error_handler_before = MaintenanceTasks.error_handler handled_error = nil - MaintenanceTasks.error_handler = ->(error) { handled_error = error } + handled_task_context = nil + handled_errored_element = nil + + MaintenanceTasks.error_handler = ->(error, task_context, errored_el) do + handled_error = error + handled_task_context = task_context + handled_errored_element = errored_el + end + run = Run.create!(task_name: 'Maintenance::ErrorTask') TaskJob.perform_now(run) assert_equal(ArgumentError, handled_error.class) + assert_equal('Maintenance::ErrorTask', handled_task_context[:task_name]) + assert_equal(2, handled_errored_element) ensure MaintenanceTasks.error_handler = error_handler_before end test '.perform_now still persists the error properly if the error handler raises' do error_handler_before = MaintenanceTasks.error_handler - MaintenanceTasks.error_handler = ->(error) { raise error } + MaintenanceTasks.error_handler = ->(error, _task_context, _errored_el) do + raise error + end run = Run.create!(task_name: 'Maintenance::ErrorTask') assert_raises { TaskJob.perform_now(run) } diff --git a/test/lib/maintenance_tasks_test.rb b/test/lib/maintenance_tasks_test.rb index 6e7a47f8a..19617a398 100644 --- a/test/lib/maintenance_tasks_test.rb +++ b/test/lib/maintenance_tasks_test.rb @@ -19,4 +19,14 @@ class MaintenanceTasksTest < ActiveSupport::TestCase end assert_equal expected_public_constants.sort, public_constants.sort end + + test 'deprecation warning raised when error_handler does not accept three arguments' do + error_handler_before = MaintenanceTasks.error_handler + + dep_msg = 'MaintenanceTasks.error_handler should be a lambda that takes '\ + 'three arguments: error, task_context, and errored_element.' + assert_deprecated(dep_msg) { MaintenanceTasks.error_handler = ->(error) {} } + ensure + MaintenanceTasks.error_handler = error_handler_before + end end