From ce3ce72c037a54c93420bd7093d9aa11235e92b4 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Mon, 2 Feb 2026 12:13:07 +0000 Subject: [PATCH 1/8] feat(oban): add :skip_error_report_callback option The callback receives the worker module and job struct, allowing for per-worker or per-job decision logic on whether to skip error reporting. --- lib/sentry/config.ex | 30 ++++++ .../integrations/oban/error_reporter.ex | 32 +++++- .../config_oban_tags_to_sentry_tags_test.exs | 35 +++++++ .../integrations/oban/error_reporter_test.exs | 97 +++++++++++++++++++ 4 files changed, 193 insertions(+), 1 deletion(-) diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index b55f20ef..ce5b303f 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -78,6 +78,25 @@ defmodule Sentry.Config do with `oban_tags.` and with a value of `true`. *Available since 12.0.0*. """ ], + skip_error_report_callback: [ + type: {:custom, __MODULE__, :__validate_skip_error_report_callback__, []}, + default: nil, + type_doc: "`(module(), Oban.Job.t() -> boolean())` or `nil`", + doc: """ + A function that determines whether to skip reporting errors for Oban job retries. + The function receives the worker module and the `Oban.Job` struct and should return + `true` to skip reporting or `false` to report the error. + + ```elixir + skip_error_report_callback: fn _worker, job -> + job.attempt < job.max_attempts + end + ``` + + This example skips reporting errors for all non-final retry attempts. + *Available since 12.0.0*. + """ + ], cron: [ doc: """ Configuration options for configuring [*crons*](https://docs.sentry.io/product/crons/) @@ -1077,4 +1096,15 @@ defmodule Sentry.Config do {:error, "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_skip_error_report_callback__(fun) when is_function(fun, 2) do + {:ok, fun} + end + + def __validate_skip_error_report_callback__(other) do + {:error, + "expected :skip_error_report_callback to be nil or a function with arity 2, got: #{inspect(other)}"} + end end diff --git a/lib/sentry/integrations/oban/error_reporter.ex b/lib/sentry/integrations/oban/error_reporter.ex index de4e8a73..6f267edb 100644 --- a/lib/sentry/integrations/oban/error_reporter.ex +++ b/lib/sentry/integrations/oban/error_reporter.ex @@ -31,13 +31,43 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do %{job: job, kind: kind, reason: reason, stacktrace: stacktrace} = _metadata, config ) do - if report?(reason) do + if report?(reason) and should_report?(job, config) do report(job, kind, reason, stacktrace, config) else :ok end end + defp should_report?(job, config) do + case Keyword.get(config, :skip_error_report_callback) do + callback when is_function(callback, 2) -> + not call_skip_error_report_callback(callback, job) + + _ -> + true + end + end + + defp call_skip_error_report_callback(callback, job) do + worker = + case apply(Oban.Worker, :from_string, [job.worker]) do + {:ok, mod} -> mod + :error -> nil + end + + try do + callback.(worker, job) == true + rescue + error -> + Logger.warning( + "skip_error_report_callback failed for worker #{inspect(worker)} " <> + "(job ID #{job.id}): #{inspect(error)}" + ) + + false + end + end + defp report(job, kind, reason, stacktrace, config) do stacktrace = case {apply(Oban.Worker, :from_string, [job.worker]), stacktrace} do diff --git a/test/sentry/config_oban_tags_to_sentry_tags_test.exs b/test/sentry/config_oban_tags_to_sentry_tags_test.exs index 5be75111..66489433 100644 --- a/test/sentry/config_oban_tags_to_sentry_tags_test.exs +++ b/test/sentry/config_oban_tags_to_sentry_tags_test.exs @@ -59,4 +59,39 @@ defmodule Sentry.ConfigObanTagsToSentryTagsTest do end end end + + describe "skip_error_report_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 + 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 + 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]]) + 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"]]) + end + + assert_raise ArgumentError, ~r/expected :skip_error_report_callback to be/, fn -> + put_test_config(integrations: [oban: [skip_error_report_callback: 123]]) + end + + assert_raise ArgumentError, ~r/expected :skip_error_report_callback to be/, fn -> + put_test_config(integrations: [oban: [skip_error_report_callback: []]]) + end + end + end end diff --git a/test/sentry/integrations/oban/error_reporter_test.exs b/test/sentry/integrations/oban/error_reporter_test.exs index d56f5889..19f73f7f 100644 --- a/test/sentry/integrations/oban/error_reporter_test.exs +++ b/test/sentry/integrations/oban/error_reporter_test.exs @@ -1,6 +1,8 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do use ExUnit.Case, async: true + import ExUnit.CaptureLog + alias Sentry.Integrations.Oban.ErrorReporter defmodule MyWorker do @@ -210,6 +212,101 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do assert [event] = Sentry.Test.pop_sentry_reports() assert event.tags.custom_tag == "custom_value" end + + test "skip_error_report_callback skips when callback returns true" do + job = + %{"id" => "123", "entity" => "user", "type" => "delete"} + |> MyWorker.new() + |> Ecto.Changeset.apply_action!(:validate) + + reason = %RuntimeError{message: "oops"} + + Sentry.Test.start_collecting() + + job_attempt_1 = Map.merge(job, %{attempt: 1, max_attempts: 3}) + + # Callback returns true -> 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 + ) + + assert [] = Sentry.Test.pop_sentry_reports() + + # Final attempt: callback returns false -> 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 + ) + + 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 + job = + %{"id" => "123", "entity" => "user", "type" => "delete"} + |> MyWorker.new() + |> Ecto.Changeset.apply_action!(:validate) + + reason = %RuntimeError{message: "oops"} + test_pid = self() + + Sentry.Test.start_collecting() + + assert :ok = + ErrorReporter.handle_event( + [:oban, :job, :exception], + %{}, + %{job: job, kind: :error, reason: reason, stacktrace: []}, + skip_error_report_callback: fn worker, received_job -> + send(test_pid, {:callback_args, worker, received_job}) + false + end + ) + + assert_receive {:callback_args, worker, received_job} + assert worker == MyWorker + assert received_job == job + end + + test "skip_error_report_callback reports when callback returns false" do + Sentry.Test.start_collecting() + + emit_telemetry_for_failed_job(:error, %RuntimeError{message: "oops"}, [], + skip_error_report_callback: fn _worker, _job -> false 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 + 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 + ) + end) + + assert log =~ "skip_error_report_callback failed" + assert log =~ "Sentry.Integrations.Oban.ErrorReporterTest.MyWorker" + assert log =~ "callback error" + + assert [event] = Sentry.Test.pop_sentry_reports() + assert event.original_exception == %RuntimeError{message: "oops"} + end end ## Helpers From 11125a91a95b76810eff5da33521d1f68b807497 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Mon, 2 Feb 2026 19:20:30 +0100 Subject: [PATCH 2/8] tests(oban): add integration tests for :skip_error_report_callback config --- .../integrations/oban/error_reporter.ex | 11 +- .../test/phoenix_app/oban_test.exs | 242 ++++++++++++++++++ 2 files changed, 251 insertions(+), 2 deletions(-) diff --git a/lib/sentry/integrations/oban/error_reporter.ex b/lib/sentry/integrations/oban/error_reporter.ex index 6f267edb..983d9061 100644 --- a/lib/sentry/integrations/oban/error_reporter.ex +++ b/lib/sentry/integrations/oban/error_reporter.ex @@ -51,8 +51,15 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do defp call_skip_error_report_callback(callback, job) do worker = case apply(Oban.Worker, :from_string, [job.worker]) do - {:ok, mod} -> mod - :error -> nil + {:ok, mod} -> + mod + + {:error, _} -> + Logger.warning( + "Could not resolve Oban worker module from string: #{inspect(job.worker)}" + ) + + nil end try do diff --git a/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs b/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs index 0377ed29..4ca083da 100644 --- a/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs +++ b/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs @@ -2,8 +2,11 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do use PhoenixAppWeb.ConnCase, async: false use Oban.Testing, repo: PhoenixApp.Repo + import ExUnit.CaptureLog import Sentry.TestHelpers + alias Sentry.Integrations.Oban.ErrorReporter + setup do put_test_config(dsn: "http://public:secret@localhost:8080/1", traces_sample_rate: 1.0) @@ -21,6 +24,17 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do end end + defmodule FailingWorker do + use Oban.Worker, max_attempts: 3 + + @impl Oban.Worker + def perform(%Oban.Job{args: %{"should_fail" => true}}) do + raise "intentional failure for testing" + end + + def perform(_job), do: :ok + end + test "captures Oban worker execution as transaction" do :ok = perform_job(TestWorker, %{test: "args"}) @@ -42,4 +56,232 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do assert [] = transaction.spans end + + describe "skip_error_report_callback config" do + setup do + :telemetry.detach(ErrorReporter) + + on_exit(fn -> + _ = :telemetry.detach(ErrorReporter) + ErrorReporter.attach([]) + end) + + :ok + end + + test "skips error reporting when callback returns true" do + test_pid = self() + + ErrorReporter.attach( + skip_error_report_callback: fn worker, job -> + send(test_pid, {:callback_invoked, worker, job}) + true + end + ) + + {:ok, _job} = + %{"should_fail" => true} + |> FailingWorker.new() + |> Oban.insert() + + assert %{failure: 1} = Oban.drain_queue(queue: :default) + + assert_receive {:callback_invoked, worker, received_job} + assert worker == FailingWorker + assert %Oban.Job{} = received_job + assert received_job.args == %{"should_fail" => true} + + assert [] = Sentry.Test.pop_sentry_reports() + end + + test "reports error when callback returns false" do + test_pid = self() + + ErrorReporter.attach( + skip_error_report_callback: fn worker, job -> + send(test_pid, {:callback_invoked, worker, job}) + false + end + ) + + {:ok, _job} = + %{"should_fail" => true} + |> FailingWorker.new() + |> Oban.insert() + + assert %{failure: 1} = Oban.drain_queue(queue: :default) + + assert_receive {:callback_invoked, _worker, _job} + + assert [event] = Sentry.Test.pop_sentry_reports() + assert event.original_exception == %RuntimeError{message: "intentional failure for testing"} + + assert event.tags.oban_worker == + "Sentry.Integrations.Phoenix.ObanTest.FailingWorker" + end + + test "callback receives worker module and full job struct" do + test_pid = self() + + ErrorReporter.attach( + skip_error_report_callback: fn worker, job -> + send(test_pid, {:callback_args, worker, job}) + false + end + ) + + {:ok, _job} = + %{"should_fail" => true, "user_id" => 123} + |> FailingWorker.new() + |> Oban.insert() + + Oban.drain_queue(queue: :default) + + assert_receive {:callback_args, worker, job} + + assert worker == FailingWorker + assert is_atom(worker) + + assert %Oban.Job{} = job + assert job.args == %{"should_fail" => true, "user_id" => 123} + assert job.worker == "Sentry.Integrations.Phoenix.ObanTest.FailingWorker" + assert job.queue == "default" + assert job.max_attempts == 3 + assert is_integer(job.attempt) + assert is_integer(job.id) + end + + test "callback can make decisions based on attempt number" 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 + end + ) + + {:ok, _job} = + %{"should_fail" => true} + |> FailingWorker.new() + |> Oban.insert() + + Oban.drain_queue(queue: :default) + + assert_receive {:skip_decision, attempt, max_attempts, should_skip} + assert attempt == 1 + assert max_attempts == 3 + assert should_skip == true + + assert [] = Sentry.Test.pop_sentry_reports() + end + + test "handles callback errors gracefully and defaults to reporting" do + log = + capture_log(fn -> + ErrorReporter.attach( + skip_error_report_callback: fn _worker, _job -> + raise "callback crashed!" + end + ) + + {:ok, _job} = + %{"should_fail" => true} + |> FailingWorker.new() + |> Oban.insert() + + Oban.drain_queue(queue: :default) + end) + + assert log =~ "skip_error_report_callback failed" + assert log =~ "FailingWorker" + assert log =~ "callback crashed!" + + assert [event] = Sentry.Test.pop_sentry_reports() + assert event.original_exception == %RuntimeError{message: "intentional failure for testing"} + end + + test "reports error when no callback is configured" do + ErrorReporter.attach([]) + + {:ok, _job} = + %{"should_fail" => true} + |> FailingWorker.new() + |> Oban.insert() + + Oban.drain_queue(queue: :default) + + assert [event] = Sentry.Test.pop_sentry_reports() + assert event.original_exception == %RuntimeError{message: "intentional failure for testing"} + end + + test "callback can filter based on worker type" 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 + end + ) + + {:ok, _job} = + %{"should_fail" => true} + |> FailingWorker.new() + |> Oban.insert() + + Oban.drain_queue(queue: :default) + + assert_receive {:worker_check, FailingWorker, true} + + assert [] = Sentry.Test.pop_sentry_reports() + end + + test "callback receives nil and logs warning for non-existent worker module" do + test_pid = self() + + log = + capture_log(fn -> + ErrorReporter.attach( + skip_error_report_callback: fn worker, job -> + send(test_pid, {:callback_with_unknown_worker, worker, job}) + false + end + ) + + job = %Oban.Job{ + id: 999, + args: %{"test" => true}, + worker: "NonExistent.Worker.Module", + queue: "default", + state: "executing", + attempt: 1, + max_attempts: 3 + } + + :telemetry.execute( + [:oban, :job, :exception], + %{duration: 1000}, + %{ + job: job, + kind: :error, + reason: %RuntimeError{message: "worker failed"}, + stacktrace: [] + } + ) + end) + + assert log =~ "Could not resolve Oban worker module from string" + assert log =~ "NonExistent.Worker.Module" + + assert_receive {:callback_with_unknown_worker, worker, received_job} + assert worker == nil + assert received_job.worker == "NonExistent.Worker.Module" + + assert [event] = Sentry.Test.pop_sentry_reports() + assert event.tags.oban_worker == "NonExistent.Worker.Module" + end + end end From 34eb81a47f8d507164089f6ab12b099ee535d99b Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Mon, 2 Feb 2026 12:13:29 +0000 Subject: [PATCH 3/8] docs: update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e01122f1..ca168d1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Features +- Add `should_report_error_callback` option to Oban.ErrorReporter for flexible error reporting logic ([#832](https://github.com/getsentry/sentry-elixir/pull/832)) - Support for Structured Logs ([#969](https://github.com/getsentry/sentry-elixir/pull/969)) - Support for Distributed Tracing ([957](https://github.com/getsentry/sentry-elixir/pull/957)) - Support for LiveView spans captured under single trace root ([#977](https://github.com/getsentry/sentry-elixir/pull/977)) From 2bc12a4e931ffeda4b363acad59b164d21b92215 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 3 Feb 2026 14:53:08 +0100 Subject: [PATCH 4/8] Apply suggestion from @whatyouhide Co-authored-by: Andrea Leopardi --- lib/sentry/config.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index ce5b303f..4b6fbb63 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -81,7 +81,7 @@ defmodule Sentry.Config do skip_error_report_callback: [ type: {:custom, __MODULE__, :__validate_skip_error_report_callback__, []}, default: nil, - type_doc: "`(module(), Oban.Job.t() -> boolean())` or `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. The function receives the worker module and the `Oban.Job` struct and should return From faa60b2ce038c8741a0a9217cd29ab818578df2c Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 3 Feb 2026 14:53:32 +0100 Subject: [PATCH 5/8] Apply suggestions from code review Co-authored-by: Andrea Leopardi --- lib/sentry/config.ex | 2 +- lib/sentry/integrations/oban/error_reporter.ex | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index 4b6fbb63..d68ee8b1 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -79,7 +79,7 @@ defmodule Sentry.Config do """ ], skip_error_report_callback: [ - type: {:custom, __MODULE__, :__validate_skip_error_report_callback__, []}, + type: {:or, [nil, {:fun, 2}]}, default: nil, type_doc: "`(Oban.Worker.t() | nil, Oban.Job.t() -> boolean())` or `nil`", doc: """ diff --git a/lib/sentry/integrations/oban/error_reporter.ex b/lib/sentry/integrations/oban/error_reporter.ex index 983d9061..dc7b7350 100644 --- a/lib/sentry/integrations/oban/error_reporter.ex +++ b/lib/sentry/integrations/oban/error_reporter.ex @@ -67,8 +67,12 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do rescue error -> Logger.warning( - "skip_error_report_callback failed for worker #{inspect(worker)} " <> - "(job ID #{job.id}): #{inspect(error)}" + """ + :skip_error_report_callback failed for worker #{inspect(worker)} \ + (job ID #{job.id}): + + #{Exception.format(:error, error, __STACKTRACE__)}\ + """ ) false From 4ef2ef9b1d1dbcce5e57bd7ef5e542d4cef71fa7 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 3 Feb 2026 13:59:34 +0000 Subject: [PATCH 6/8] Rename to :should_report_error_callback --- lib/sentry/config.ex | 20 ++++---- .../integrations/oban/error_reporter.ex | 12 ++--- .../config_oban_tags_to_sentry_tags_test.exs | 26 +++++----- .../integrations/oban/error_reporter_test.exs | 26 +++++----- .../test/phoenix_app/oban_test.exs | 48 +++++++++---------- 5 files changed, 66 insertions(+), 66 deletions(-) diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index d68ee8b1..ea7d4891 100644 --- a/lib/sentry/config.ex +++ b/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 diff --git a/lib/sentry/integrations/oban/error_reporter.ex b/lib/sentry/integrations/oban/error_reporter.ex index dc7b7350..e4115db8 100644 --- a/lib/sentry/integrations/oban/error_reporter.ex +++ b/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 diff --git a/test/sentry/config_oban_tags_to_sentry_tags_test.exs b/test/sentry/config_oban_tags_to_sentry_tags_test.exs index 66489433..e9bcdb75 100644 --- a/test/sentry/config_oban_tags_to_sentry_tags_test.exs +++ b/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 diff --git a/test/sentry/integrations/oban/error_reporter_test.exs b/test/sentry/integrations/oban/error_reporter_test.exs index 19f73f7f..1e359b6d 100644 --- a/test/sentry/integrations/oban/error_reporter_test.exs +++ b/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,18 +225,18 @@ 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 = @@ -244,7 +244,7 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do [: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() @@ -252,7 +252,7 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do 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" diff --git a/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs b/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs index 4ca083da..06514298 100644 --- a/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs +++ b/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 ) From 747c3d39b33a23ff645e7e6fffd178a4d698853c Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 3 Feb 2026 14:00:12 +0000 Subject: [PATCH 7/8] Remove unused validation fns --- lib/sentry/config.ex | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index ea7d4891..1357ba33 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -1096,15 +1096,4 @@ defmodule Sentry.Config do {:error, "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_should_report_error_callback__(nil), do: {:ok, nil} - - def __validate_should_report_error_callback__(fun) when is_function(fun, 2) do - {:ok, fun} - end - - def __validate_should_report_error_callback__(other) do - {:error, - "expected :should_report_error_callback to be nil or a function with arity 2, got: #{inspect(other)}"} - end end From 1eb3b6d49f5ecc57bd0206edf898dc4f92f000e0 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 3 Feb 2026 14:04:47 +0000 Subject: [PATCH 8/8] mix format --- lib/sentry/integrations/oban/error_reporter.ex | 12 +++++------- .../sentry/integrations/oban/error_reporter_test.exs | 8 ++++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/sentry/integrations/oban/error_reporter.ex b/lib/sentry/integrations/oban/error_reporter.ex index e4115db8..31fce339 100644 --- a/lib/sentry/integrations/oban/error_reporter.ex +++ b/lib/sentry/integrations/oban/error_reporter.ex @@ -66,14 +66,12 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do callback.(worker, job) == true rescue error -> - Logger.warning( - """ - :should_report_error_callback failed for worker #{inspect(worker)} \ - (job ID #{job.id}): + Logger.warning(""" + :should_report_error_callback failed for worker #{inspect(worker)} \ + (job ID #{job.id}): - #{Exception.format(:error, error, __STACKTRACE__)}\ - """ - ) + #{Exception.format(:error, error, __STACKTRACE__)}\ + """) true end diff --git a/test/sentry/integrations/oban/error_reporter_test.exs b/test/sentry/integrations/oban/error_reporter_test.exs index 1e359b6d..3dda57bf 100644 --- a/test/sentry/integrations/oban/error_reporter_test.exs +++ b/test/sentry/integrations/oban/error_reporter_test.exs @@ -231,7 +231,9 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do [:oban, :job, :exception], %{}, %{job: job_attempt_1, kind: :error, reason: reason, stacktrace: []}, - should_report_error_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() @@ -244,7 +246,9 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do [:oban, :job, :exception], %{}, %{job: job_attempt_3, kind: :error, reason: reason, stacktrace: []}, - should_report_error_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()