Deprecate MaintenanceTasks.error_handler. Instead use the Rails.error…

….reporter.

README.md

@@ -74,9 +74,9 @@ The generator creates and runs a migration to add the necessary table to your
 database. It also mounts Maintenance Tasks in your `config/routes.rb`. By
 default the web UI can be accessed in the new `/maintenance_tasks` path.
 
-In case you use an exception reporting service (e.g. Bugsnag) you might want to
-define an error handler. See [Customizing the error
-handler](#customizing-the-error-handler) for more information.
+This gem using the Rails Error Reporter to report errors. If you are using a bug
+tracking service you may want to subscribe to the reporter. See [Reporting Errors](#reporting-errors)
+for more information.
 
 ### Active Job Dependency
 
@@ -988,44 +988,32 @@ If you are stuck in `pausing` and wish to preserve your tasks's position
 There are a few configurable options for the gem. Custom configurations should
 be placed in a `maintenance_tasks.rb` initializer.
 
-#### Customizing the error handler
+#### Reporting errors
 
 Exceptions raised while a Task is performing are rescued and information about
 the error is persisted and visible in the UI.
 
-If you want to integrate with an exception monitoring service (e.g. Bugsnag),
-you can define an error handler:
+Errors are also sent to the `Rails.error.reporter` we can be configured by your
+application. See the [Error Reporting in Rails Applications](https://guides.rubyonrails.org/error_reporting.html) guide for more details.
 
-```ruby
-# config/initializers/maintenance_tasks.rb
-
-MaintenanceTasks.error_handler = ->(error, task_context, _errored_element) do
-  Bugsnag.notify(error) do |notification|
-    notification.add_metadata(:task, task_context)
-  end
-end
-```
-
-The error handler should be a lambda that accepts three arguments:
+Reports to the error reporter will contain the following data:
 
 * `error`: The exception that was raised.
-* `task_context`: A hash with additional information about the Task and the
+* `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
-
-  Note that `task_context` may be empty if the Task produced an error before any
-  context could be gathered (for example, if deserializing the job to process
-  your Task failed).
-* `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 Active
-  Record 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.
+  * `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.
+* `source`: This will be `maintenance-tasks`
+
+Note that `context` may be empty if the Task produced an error before any
+context could be gathered (for example, if deserializing the job to process
+your Task failed).
 
 #### Customizing the maintenance tasks module
 

app/jobs/concerns/maintenance_tasks/task_job_concern.rb

@@ -183,9 +183,14 @@ def on_error(error)
           ended_at: @run.ended_at,
         }
       end
-      errored_element = @errored_element if defined?(@errored_element)
+      task_context[:errored_element] = @errored_element if defined?(@errored_element)
     ensure
-      MaintenanceTasks.error_handler.call(error, task_context, errored_element)
+      if MaintenanceTasks.instance_variable_get(:@error_handler)
+        errored_element = task_context.delete(:errored_element)
+        MaintenanceTasks.error_handler.call(error, task_context, errored_element)
+      else
+        Rails.error.report(error, context: task_context, source: "maintenance-tasks")
+      end
     end
   end
 end

lib/maintenance_tasks.rb

@@ -61,16 +61,6 @@ module MaintenanceTasks
   #     use when cleaning a Run's backtrace.
   mattr_accessor :backtrace_cleaner
 
-  # @!attribute error_handler
-  #   @scope class
-  #
-  #   The callback to perform when an error occurs in the Task.  See the
-  #   {file:README#label-Customizing+the+error+handler} for details.
-  #
-  #   @return [Proc] the callback to perform when an error occurs in the Task.
-  mattr_accessor :error_handler, default:
-    ->(_error, _task_context, _errored_element) {}
-
   # @!attribute parent_controller
   #   @scope class
   #
@@ -94,4 +84,29 @@ module MaintenanceTasks
   #
   #  @return [ActiveSupport::Duration] the threshold in seconds after which a task is considered stuck.
   mattr_accessor :stuck_task_duration, default: 5.minutes
+
+  class << self
+    DEPRECATION_MESSAGE = "MaintenanceTasks.error_handler is deprecated and will be removed in the next release. " \
+      "Instead, reports will be sent to the Rails error reporter. Do not set a handler and subscribe" \
+      "to the error reporter instead."
+
+    # no-doc
+    def error_handler
+      deprecator.warn(DEPRECATION_MESSAGE)
+
+      @error_handler
+    end
+
+    # no-doc
+    def error_handler=(proc)
+      deprecator.warn(DEPRECATION_MESSAGE)
+
+      @error_handler = proc
+    end
+
+    # no-doc
+    def deprecator
+      @deprecator ||= ActiveSupport::Deprecation.new
+    end
+  end
 end

test/jobs/maintenance_tasks/task_job_test.rb

@@ -344,64 +344,70 @@ class << self
       end
     end
 
-    test ".perform_now calls the error handler when there was an Error" do
-      error_handler_before = MaintenanceTasks.error_handler
-      handled_error = nil
-      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
+    test ".perform_now calls the error handler if one is set" do
+      MaintenanceTasks.deprecator.with(behavior: :silence) do
+        error_handler_before = MaintenanceTasks.error_handler
+        handled_error = nil
+        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")
+        run = Run.create!(task_name: "Maintenance::ErrorTask")
 
-      TaskJob.perform_now(run)
+        TaskJob.perform_now(run)
 
-      assert_equal(ArgumentError, handled_error.class)
-      assert_equal("Maintenance::ErrorTask", handled_task_context[:task_name])
-      assert_equal(3, handled_errored_element)
-    ensure
-      MaintenanceTasks.error_handler = error_handler_before
+        assert_equal(ArgumentError, handled_error.class)
+        assert_equal("Maintenance::ErrorTask", handled_task_context[:task_name])
+        assert_equal(3, handled_errored_element)
+      ensure
+        MaintenanceTasks.error_handler = error_handler_before
+      end
     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, _task_context, _errored_el) do
-        raise error
+      MaintenanceTasks.deprecator.with(behavior: :silence) do
+        error_handler_before = MaintenanceTasks.error_handler
+        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) }
+        run.reload
+
+        assert_predicate(run, :errored?)
+        assert_equal(2, run.tick_count)
+      ensure
+        MaintenanceTasks.error_handler = error_handler_before
       end
+    end
+
+    test ".perform_now reports errors raised by the task" do
       run = Run.create!(task_name: "Maintenance::ErrorTask")
 
-      assert_raises { TaskJob.perform_now(run) }
+      assert_error_reported do
+        TaskJob.perform_now(run)
+      end
       run.reload
 
       assert_predicate(run, :errored?)
       assert_equal(2, run.tick_count)
-    ensure
-      MaintenanceTasks.error_handler = error_handler_before
     end
 
-    test ".perform_now handles case where run is not set and calls error handler" do
-      error_handler_before = MaintenanceTasks.error_handler
-      handled_error = nil
-      handled_task_context = nil
-      MaintenanceTasks.error_handler = ->(error, task_context, _errored_el) do
-        handled_error = error
-        handled_task_context = task_context
-      end
-
+    test ".perform_now handles case where run is not set and reports error" do
+      TaskError = StandardError.new
       RaisingTaskJob = Class.new(TaskJob) do
-        before_perform(prepend: true) { raise "Uh oh!" }
+        before_perform(prepend: true) { raise TaskError }
       end
 
-      RaisingTaskJob.perform_now(@run)
-
-      assert_equal("Uh oh!", handled_error.message)
-      assert_empty(handled_task_context)
-    ensure
-      MaintenanceTasks.error_handler = error_handler_before
+      assert_error_reported(TaskError) do
+        RaisingTaskJob.perform_now(@run)
+      end
     end
 
     test ".perform_now throttles when running Task that uses throttle_on" do