Skip to content

Commit c54de9b

Browse files
solnicwhatyouhide
andauthored
feat(oban): add support for :should_report_error_callback option (#832)
* 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. * tests(oban): add integration tests for :skip_error_report_callback config * docs: update CHANGELOG.md * Apply suggestion from @whatyouhide Co-authored-by: Andrea Leopardi <an.leopardi@gmail.com> * Apply suggestions from code review Co-authored-by: Andrea Leopardi <an.leopardi@gmail.com> * Rename to :should_report_error_callback * Remove unused validation fns * mix format --------- Co-authored-by: Andrea Leopardi <an.leopardi@gmail.com>
1 parent 772b6a4 commit c54de9b

6 files changed

Lines changed: 438 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#### Features
44

5+
- Add `should_report_error_callback` option to Oban.ErrorReporter for flexible error reporting logic ([#832](https://github.com/getsentry/sentry-elixir/pull/832))
56
- Support for Structured Logs ([#969](https://github.com/getsentry/sentry-elixir/pull/969))
67
- Support for Distributed Tracing ([957](https://github.com/getsentry/sentry-elixir/pull/957))
78
- Support for LiveView spans captured under single trace root ([#977](https://github.com/getsentry/sentry-elixir/pull/977))

lib/sentry/config.ex

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,25 @@ defmodule Sentry.Config do
7878
with `oban_tags.` and with a value of `true`. *Available since 12.0.0*.
7979
"""
8080
],
81+
should_report_error_callback: [
82+
type: {:or, [nil, {:fun, 2}]},
83+
default: nil,
84+
type_doc: "`(Oban.Worker.t() | nil, Oban.Job.t() -> boolean())` or `nil`",
85+
doc: """
86+
A function that determines whether to report errors for Oban jobs.
87+
The function receives the worker module and the `Oban.Job` struct and should return
88+
`true` to report the error or `false` to skip reporting.
89+
90+
```elixir
91+
should_report_error_callback: fn _worker, job ->
92+
job.attempt >= job.max_attempts
93+
end
94+
```
95+
96+
This example only reports errors on final retry attempts.
97+
*Available since 12.0.0*.
98+
"""
99+
],
81100
cron: [
82101
doc: """
83102
Configuration options for configuring [*crons*](https://docs.sentry.io/product/crons/)

lib/sentry/integrations/oban/error_reporter.ex

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,52 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do
3131
%{job: job, kind: kind, reason: reason, stacktrace: stacktrace} = _metadata,
3232
config
3333
) do
34-
if report?(reason) do
34+
if report?(reason) and should_report?(job, config) do
3535
report(job, kind, reason, stacktrace, config)
3636
else
3737
:ok
3838
end
3939
end
4040

41+
defp should_report?(job, config) do
42+
case Keyword.get(config, :should_report_error_callback) do
43+
callback when is_function(callback, 2) ->
44+
call_should_report_error_callback(callback, job)
45+
46+
_ ->
47+
true
48+
end
49+
end
50+
51+
defp call_should_report_error_callback(callback, job) do
52+
worker =
53+
case apply(Oban.Worker, :from_string, [job.worker]) do
54+
{:ok, mod} ->
55+
mod
56+
57+
{:error, _} ->
58+
Logger.warning(
59+
"Could not resolve Oban worker module from string: #{inspect(job.worker)}"
60+
)
61+
62+
nil
63+
end
64+
65+
try do
66+
callback.(worker, job) == true
67+
rescue
68+
error ->
69+
Logger.warning("""
70+
:should_report_error_callback failed for worker #{inspect(worker)} \
71+
(job ID #{job.id}):
72+
73+
#{Exception.format(:error, error, __STACKTRACE__)}\
74+
""")
75+
76+
true
77+
end
78+
end
79+
4180
defp report(job, kind, reason, stacktrace, config) do
4281
stacktrace =
4382
case {apply(Oban.Worker, :from_string, [job.worker]), stacktrace} do

test/sentry/config_oban_tags_to_sentry_tags_test.exs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,39 @@ defmodule Sentry.ConfigObanTagsToSentryTagsTest do
5959
end
6060
end
6161
end
62+
63+
describe "should_report_error_callback configuration validation" do
64+
test "accepts nil" do
65+
assert :ok = put_test_config(integrations: [oban: [should_report_error_callback: nil]])
66+
assert Sentry.Config.integrations()[:oban][:should_report_error_callback] == nil
67+
end
68+
69+
test "accepts function with arity 2" do
70+
fun = fn _worker, _job -> true end
71+
assert :ok = put_test_config(integrations: [oban: [should_report_error_callback: fun]])
72+
assert Sentry.Config.integrations()[:oban][:should_report_error_callback] == fun
73+
end
74+
75+
test "rejects function with wrong arity" do
76+
fun = fn _job -> true end
77+
78+
assert_raise ArgumentError, ~r/invalid value for :should_report_error_callback/, fn ->
79+
put_test_config(integrations: [oban: [should_report_error_callback: fun]])
80+
end
81+
end
82+
83+
test "rejects invalid types" do
84+
assert_raise ArgumentError, ~r/invalid value for :should_report_error_callback/, fn ->
85+
put_test_config(integrations: [oban: [should_report_error_callback: "invalid"]])
86+
end
87+
88+
assert_raise ArgumentError, ~r/invalid value for :should_report_error_callback/, fn ->
89+
put_test_config(integrations: [oban: [should_report_error_callback: 123]])
90+
end
91+
92+
assert_raise ArgumentError, ~r/invalid value for :should_report_error_callback/, fn ->
93+
put_test_config(integrations: [oban: [should_report_error_callback: []]])
94+
end
95+
end
96+
end
6297
end

test/sentry/integrations/oban/error_reporter_test.exs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
defmodule Sentry.Integrations.Oban.ErrorReporterTest do
22
use ExUnit.Case, async: true
33

4+
import ExUnit.CaptureLog
5+
46
alias Sentry.Integrations.Oban.ErrorReporter
57

68
defmodule MyWorker do
@@ -210,6 +212,105 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
210212
assert [event] = Sentry.Test.pop_sentry_reports()
211213
assert event.tags.custom_tag == "custom_value"
212214
end
215+
216+
test "should_report_error_callback skips when callback returns false" do
217+
job =
218+
%{"id" => "123", "entity" => "user", "type" => "delete"}
219+
|> MyWorker.new()
220+
|> Ecto.Changeset.apply_action!(:validate)
221+
222+
reason = %RuntimeError{message: "oops"}
223+
224+
Sentry.Test.start_collecting()
225+
226+
job_attempt_1 = Map.merge(job, %{attempt: 1, max_attempts: 3})
227+
228+
# Callback returns false -> skip reporting
229+
assert :ok =
230+
ErrorReporter.handle_event(
231+
[:oban, :job, :exception],
232+
%{},
233+
%{job: job_attempt_1, kind: :error, reason: reason, stacktrace: []},
234+
should_report_error_callback: fn _worker, job ->
235+
job.attempt >= job.max_attempts
236+
end
237+
)
238+
239+
assert [] = Sentry.Test.pop_sentry_reports()
240+
241+
# Final attempt: callback returns true -> report
242+
job_attempt_3 = Map.merge(job, %{attempt: 3, max_attempts: 3})
243+
244+
assert :ok =
245+
ErrorReporter.handle_event(
246+
[:oban, :job, :exception],
247+
%{},
248+
%{job: job_attempt_3, kind: :error, reason: reason, stacktrace: []},
249+
should_report_error_callback: fn _worker, job ->
250+
job.attempt >= job.max_attempts
251+
end
252+
)
253+
254+
assert [event] = Sentry.Test.pop_sentry_reports()
255+
assert event.original_exception == %RuntimeError{message: "oops"}
256+
assert event.tags.oban_worker == "Sentry.Integrations.Oban.ErrorReporterTest.MyWorker"
257+
end
258+
259+
test "should_report_error_callback receives worker module and job" do
260+
job =
261+
%{"id" => "123", "entity" => "user", "type" => "delete"}
262+
|> MyWorker.new()
263+
|> Ecto.Changeset.apply_action!(:validate)
264+
265+
reason = %RuntimeError{message: "oops"}
266+
test_pid = self()
267+
268+
Sentry.Test.start_collecting()
269+
270+
assert :ok =
271+
ErrorReporter.handle_event(
272+
[:oban, :job, :exception],
273+
%{},
274+
%{job: job, kind: :error, reason: reason, stacktrace: []},
275+
should_report_error_callback: fn worker, received_job ->
276+
send(test_pid, {:callback_args, worker, received_job})
277+
true
278+
end
279+
)
280+
281+
assert_receive {:callback_args, worker, received_job}
282+
assert worker == MyWorker
283+
assert received_job == job
284+
end
285+
286+
test "should_report_error_callback reports when callback returns true" do
287+
Sentry.Test.start_collecting()
288+
289+
emit_telemetry_for_failed_job(:error, %RuntimeError{message: "oops"}, [],
290+
should_report_error_callback: fn _worker, _job -> true end
291+
)
292+
293+
assert [event] = Sentry.Test.pop_sentry_reports()
294+
assert event.original_exception == %RuntimeError{message: "oops"}
295+
end
296+
297+
test "should_report_error_callback handles errors gracefully and defaults to reporting" do
298+
Sentry.Test.start_collecting()
299+
300+
log =
301+
capture_log(fn ->
302+
emit_telemetry_for_failed_job(:error, %RuntimeError{message: "oops"}, [],
303+
should_report_error_callback: fn _worker, _job -> raise "callback error" end
304+
)
305+
end)
306+
307+
assert log =~ "should_report_error_callback failed"
308+
assert log =~ "Sentry.Integrations.Oban.ErrorReporterTest.MyWorker"
309+
assert log =~ "callback error"
310+
311+
assert [event] = Sentry.Test.pop_sentry_reports()
312+
assert event.original_exception == %RuntimeError{message: "oops"}
313+
end
213314
end
214315

215316
## Helpers

0 commit comments

Comments
 (0)