Rename to :should_report_error_callback

Author: solnic

lib/sentry/config.ex

@@ -78,22 +78,22 @@ defmodule Sentry.Config do
           with `oban_tags.` and with a value of `true`. *Available since 12.0.0*.
           """
         ],
-        skip_error_report_callback: [
+        should_report_error_callback: [
           type: {:or, [nil, {:fun, 2}]},
           default: nil,
           type_doc: "`(Oban.Worker.t() | nil, Oban.Job.t() -> boolean())` or `nil`",
           doc: """
-          A function that determines whether to skip reporting errors for Oban job retries.
+          A function that determines whether to report errors for Oban jobs.
           The function receives the worker module and the `Oban.Job` struct and should return
-          `true` to skip reporting or `false` to report the error.
+          `true` to report the error or `false` to skip reporting.
 
           ```elixir
-          skip_error_report_callback: fn _worker, job ->
-            job.attempt < job.max_attempts
+          should_report_error_callback: fn _worker, job ->
+            job.attempt >= job.max_attempts
           end
           ```
 
-          This example skips reporting errors for all non-final retry attempts.
+          This example only reports errors on final retry attempts.
           *Available since 12.0.0*.
           """
         ],
@@ -1097,14 +1097,14 @@ defmodule Sentry.Config do
      "expected :oban_tags_to_sentry_tags to be nil, a function with arity 1, or a {module, function} tuple, got: #{inspect(other)}"}
   end
 
-  def __validate_skip_error_report_callback__(nil), do: {:ok, nil}
+  def __validate_should_report_error_callback__(nil), do: {:ok, nil}
 
-  def __validate_skip_error_report_callback__(fun) when is_function(fun, 2) do
+  def __validate_should_report_error_callback__(fun) when is_function(fun, 2) do
     {:ok, fun}
   end
 
-  def __validate_skip_error_report_callback__(other) do
+  def __validate_should_report_error_callback__(other) do
     {:error,
-     "expected :skip_error_report_callback to be nil or a function with arity 2, got: #{inspect(other)}"}
+     "expected :should_report_error_callback to be nil or a function with arity 2, got: #{inspect(other)}"}
   end
 end

lib/sentry/integrations/oban/error_reporter.ex

@@ -39,16 +39,16 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do
   end
 
   defp should_report?(job, config) do
-    case Keyword.get(config, :skip_error_report_callback) do
+    case Keyword.get(config, :should_report_error_callback) do
       callback when is_function(callback, 2) ->
-        not call_skip_error_report_callback(callback, job)
+        call_should_report_error_callback(callback, job)
 
       _ ->
         true
     end
   end
 
-  defp call_skip_error_report_callback(callback, job) do
+  defp call_should_report_error_callback(callback, job) do
     worker =
       case apply(Oban.Worker, :from_string, [job.worker]) do
         {:ok, mod} ->
@@ -68,14 +68,14 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do
       error ->
         Logger.warning(
           """
-          :skip_error_report_callback failed for worker #{inspect(worker)} \
+          :should_report_error_callback failed for worker #{inspect(worker)} \
           (job ID #{job.id}):
-          
+
           #{Exception.format(:error, error, __STACKTRACE__)}\
           """
         )
 
-        false
+        true
     end
   end
 

test/sentry/config_oban_tags_to_sentry_tags_test.exs

@@ -60,37 +60,37 @@ defmodule Sentry.ConfigObanTagsToSentryTagsTest do
     end
   end
 
-  describe "skip_error_report_callback configuration validation" do
+  describe "should_report_error_callback configuration validation" do
     test "accepts nil" do
-      assert :ok = put_test_config(integrations: [oban: [skip_error_report_callback: nil]])
-      assert Sentry.Config.integrations()[:oban][:skip_error_report_callback] == nil
+      assert :ok = put_test_config(integrations: [oban: [should_report_error_callback: nil]])
+      assert Sentry.Config.integrations()[:oban][:should_report_error_callback] == nil
     end
 
     test "accepts function with arity 2" do
       fun = fn _worker, _job -> true end
-      assert :ok = put_test_config(integrations: [oban: [skip_error_report_callback: fun]])
-      assert Sentry.Config.integrations()[:oban][:skip_error_report_callback] == fun
+      assert :ok = put_test_config(integrations: [oban: [should_report_error_callback: fun]])
+      assert Sentry.Config.integrations()[:oban][:should_report_error_callback] == fun
     end
 
     test "rejects function with wrong arity" do
       fun = fn _job -> true end
 
-      assert_raise ArgumentError, ~r/expected :skip_error_report_callback to be/, fn ->
-        put_test_config(integrations: [oban: [skip_error_report_callback: fun]])
+      assert_raise ArgumentError, ~r/invalid value for :should_report_error_callback/, fn ->
+        put_test_config(integrations: [oban: [should_report_error_callback: fun]])
       end
     end
 
     test "rejects invalid types" do
-      assert_raise ArgumentError, ~r/expected :skip_error_report_callback to be/, fn ->
-        put_test_config(integrations: [oban: [skip_error_report_callback: "invalid"]])
+      assert_raise ArgumentError, ~r/invalid value for :should_report_error_callback/, fn ->
+        put_test_config(integrations: [oban: [should_report_error_callback: "invalid"]])
       end
 
-      assert_raise ArgumentError, ~r/expected :skip_error_report_callback to be/, fn ->
-        put_test_config(integrations: [oban: [skip_error_report_callback: 123]])
+      assert_raise ArgumentError, ~r/invalid value for :should_report_error_callback/, fn ->
+        put_test_config(integrations: [oban: [should_report_error_callback: 123]])
       end
 
-      assert_raise ArgumentError, ~r/expected :skip_error_report_callback to be/, fn ->
-        put_test_config(integrations: [oban: [skip_error_report_callback: []]])
+      assert_raise ArgumentError, ~r/invalid value for :should_report_error_callback/, fn ->
+        put_test_config(integrations: [oban: [should_report_error_callback: []]])
       end
     end
   end

test/sentry/integrations/oban/error_reporter_test.exs

@@ -213,7 +213,7 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
       assert event.tags.custom_tag == "custom_value"
     end
 
-    test "skip_error_report_callback skips when callback returns true" do
+    test "should_report_error_callback skips when callback returns false" do
       job =
         %{"id" => "123", "entity" => "user", "type" => "delete"}
         |> MyWorker.new()
@@ -225,34 +225,34 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
 
       job_attempt_1 = Map.merge(job, %{attempt: 1, max_attempts: 3})
 
-      # Callback returns true -> skip reporting
+      # Callback returns false -> skip reporting
       assert :ok =
                ErrorReporter.handle_event(
                  [:oban, :job, :exception],
                  %{},
                  %{job: job_attempt_1, kind: :error, reason: reason, stacktrace: []},
-                 skip_error_report_callback: fn _worker, job -> job.attempt < job.max_attempts end
+                 should_report_error_callback: fn _worker, job -> job.attempt >= job.max_attempts end
                )
 
       assert [] = Sentry.Test.pop_sentry_reports()
 
-      # Final attempt: callback returns false -> report
+      # Final attempt: callback returns true -> report
       job_attempt_3 = Map.merge(job, %{attempt: 3, max_attempts: 3})
 
       assert :ok =
                ErrorReporter.handle_event(
                  [:oban, :job, :exception],
                  %{},
                  %{job: job_attempt_3, kind: :error, reason: reason, stacktrace: []},
-                 skip_error_report_callback: fn _worker, job -> job.attempt < job.max_attempts end
+                 should_report_error_callback: fn _worker, job -> job.attempt >= job.max_attempts end
                )
 
       assert [event] = Sentry.Test.pop_sentry_reports()
       assert event.original_exception == %RuntimeError{message: "oops"}
       assert event.tags.oban_worker == "Sentry.Integrations.Oban.ErrorReporterTest.MyWorker"
     end
 
-    test "skip_error_report_callback receives worker module and job" do
+    test "should_report_error_callback receives worker module and job" do
       job =
         %{"id" => "123", "entity" => "user", "type" => "delete"}
         |> MyWorker.new()
@@ -268,9 +268,9 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
                  [:oban, :job, :exception],
                  %{},
                  %{job: job, kind: :error, reason: reason, stacktrace: []},
-                 skip_error_report_callback: fn worker, received_job ->
+                 should_report_error_callback: fn worker, received_job ->
                    send(test_pid, {:callback_args, worker, received_job})
-                   false
+                   true
                  end
                )
 
@@ -279,28 +279,28 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
       assert received_job == job
     end
 
-    test "skip_error_report_callback reports when callback returns false" do
+    test "should_report_error_callback reports when callback returns true" do
       Sentry.Test.start_collecting()
 
       emit_telemetry_for_failed_job(:error, %RuntimeError{message: "oops"}, [],
-        skip_error_report_callback: fn _worker, _job -> false end
+        should_report_error_callback: fn _worker, _job -> true end
       )
 
       assert [event] = Sentry.Test.pop_sentry_reports()
       assert event.original_exception == %RuntimeError{message: "oops"}
     end
 
-    test "skip_error_report_callback handles errors gracefully and defaults to reporting" do
+    test "should_report_error_callback handles errors gracefully and defaults to reporting" do
       Sentry.Test.start_collecting()
 
       log =
         capture_log(fn ->
           emit_telemetry_for_failed_job(:error, %RuntimeError{message: "oops"}, [],
-            skip_error_report_callback: fn _worker, _job -> raise "callback error" end
+            should_report_error_callback: fn _worker, _job -> raise "callback error" end
           )
         end)
 
-      assert log =~ "skip_error_report_callback failed"
+      assert log =~ "should_report_error_callback failed"
       assert log =~ "Sentry.Integrations.Oban.ErrorReporterTest.MyWorker"
       assert log =~ "callback error"
 

test_integrations/phoenix_app/test/phoenix_app/oban_test.exs

@@ -57,7 +57,7 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
     assert [] = transaction.spans
   end
 
-  describe "skip_error_report_callback config" do
+  describe "should_report_error_callback config" do
     setup do
       :telemetry.detach(ErrorReporter)
 
@@ -69,13 +69,13 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
       :ok
     end
 
-    test "skips error reporting when callback returns true" do
+    test "skips error reporting when callback returns false" do
       test_pid = self()
 
       ErrorReporter.attach(
-        skip_error_report_callback: fn worker, job ->
+        should_report_error_callback: fn worker, job ->
           send(test_pid, {:callback_invoked, worker, job})
-          true
+          false
         end
       )
 
@@ -94,13 +94,13 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
       assert [] = Sentry.Test.pop_sentry_reports()
     end
 
-    test "reports error when callback returns false" do
+    test "reports error when callback returns true" do
       test_pid = self()
 
       ErrorReporter.attach(
-        skip_error_report_callback: fn worker, job ->
+        should_report_error_callback: fn worker, job ->
           send(test_pid, {:callback_invoked, worker, job})
-          false
+          true
         end
       )
 
@@ -124,9 +124,9 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
       test_pid = self()
 
       ErrorReporter.attach(
-        skip_error_report_callback: fn worker, job ->
+        should_report_error_callback: fn worker, job ->
           send(test_pid, {:callback_args, worker, job})
-          false
+          true
         end
       )
 
@@ -155,10 +155,10 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
       test_pid = self()
 
       ErrorReporter.attach(
-        skip_error_report_callback: fn _worker, job ->
-          should_skip = job.attempt < job.max_attempts
-          send(test_pid, {:skip_decision, job.attempt, job.max_attempts, should_skip})
-          should_skip
+        should_report_error_callback: fn _worker, job ->
+          should_report = job.attempt >= job.max_attempts
+          send(test_pid, {:report_decision, job.attempt, job.max_attempts, should_report})
+          should_report
         end
       )
 
@@ -169,10 +169,10 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
 
       Oban.drain_queue(queue: :default)
 
-      assert_receive {:skip_decision, attempt, max_attempts, should_skip}
+      assert_receive {:report_decision, attempt, max_attempts, should_report}
       assert attempt == 1
       assert max_attempts == 3
-      assert should_skip == true
+      assert should_report == false
 
       assert [] = Sentry.Test.pop_sentry_reports()
     end
@@ -181,7 +181,7 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
       log =
         capture_log(fn ->
           ErrorReporter.attach(
-            skip_error_report_callback: fn _worker, _job ->
+            should_report_error_callback: fn _worker, _job ->
               raise "callback crashed!"
             end
           )
@@ -194,7 +194,7 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
           Oban.drain_queue(queue: :default)
         end)
 
-      assert log =~ "skip_error_report_callback failed"
+      assert log =~ "should_report_error_callback failed"
       assert log =~ "FailingWorker"
       assert log =~ "callback crashed!"
 
@@ -220,10 +220,10 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
       test_pid = self()
 
       ErrorReporter.attach(
-        skip_error_report_callback: fn worker, _job ->
-          should_skip = worker == FailingWorker
-          send(test_pid, {:worker_check, worker, should_skip})
-          should_skip
+        should_report_error_callback: fn worker, _job ->
+          should_report = worker != FailingWorker
+          send(test_pid, {:worker_check, worker, should_report})
+          should_report
         end
       )
 
@@ -234,7 +234,7 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
 
       Oban.drain_queue(queue: :default)
 
-      assert_receive {:worker_check, FailingWorker, true}
+      assert_receive {:worker_check, FailingWorker, false}
 
       assert [] = Sentry.Test.pop_sentry_reports()
     end
@@ -245,9 +245,9 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do
       log =
         capture_log(fn ->
           ErrorReporter.attach(
-            skip_error_report_callback: fn worker, job ->
+            should_report_error_callback: fn worker, job ->
               send(test_pid, {:callback_with_unknown_worker, worker, job})
-              false
+              true
             end
           )