Skip to content

Commit

Permalink
Fix Oban.PerformError grouping (#92)
Browse files Browse the repository at this point in the history
This change improves how we store data from `Oban.PerformError` errors.

Those errors don't come from exceptions, as they are delivered via
Telemetry when the worker ends up with an `:error` or an `{:error,
any()}` tuple.

As they are not exceptions, there is no stack trace, and they were been
detected as the same error by our fingerprinting algo.

This changes introduces an fix that adds the worker module as the only
stacktrace of the occurrence, so the error can populate the
`source_function` attribute and different workers generate different
error groupings.

We cannot generate different error groups per different outputs of a
worker. In that case, you should use `raise`, that generates a full
stack trace and works as expected.

Closes #87
  • Loading branch information
odarriba authored Sep 11, 2024
1 parent ac839f5 commit c0c94fa
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 5 deletions.
21 changes: 20 additions & 1 deletion lib/error_tracker/integrations/oban.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,20 @@ defmodule ErrorTracker.Integrations.Oban do
It works using Oban's Telemetry events, so you don't need to modify anything
on your application.
> #### A note on errors grouping {: .warning}
>
> All errors reported using `:error` or `{:error, any()}` as the output of
> your `perform/2` worker function are going to be grouped together (one group
> of those of errors per worker).
>
> The reason of that behaviour is that those errors do not generate an exception,
> so no stack trace is detected and they are stored as happening in the same
> place.
>
> If you want errors of your workers to be grouped as you may expect on other
> integrations, you should raise exceptions to report errors instead of gracefully
> returning an error value.
### Default context
By default we store some context for you on errors generated in an Oban
Expand Down Expand Up @@ -58,9 +72,14 @@ defmodule ErrorTracker.Integrations.Oban do
end

def handle_event([:oban, :job, :exception], _measurements, metadata, :no_config) do
%{reason: exception, stacktrace: stacktrace} = metadata
%{reason: exception, stacktrace: stacktrace, job: job} = metadata
state = Map.get(metadata, :state, :failure)

stacktrace =
if stacktrace == [],
do: [{String.to_existing_atom("Elixir." <> job.worker), :perform, 2, []}],
else: stacktrace

ErrorTracker.report(exception, stacktrace, %{state: state})
end
end
2 changes: 1 addition & 1 deletion lib/error_tracker/schemas/error.ex
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ defmodule ErrorTracker.Error do

{source_line, source_function} =
if source do
source_line = if source.line, do: "#{source.file}:#{source.line}", else: "nofile"
source_line = if source.line, do: "#{source.file}:#{source.line}", else: "(nofile)"
source_function = "#{source.module}.#{source.function}/#{source.arity}"

{source_line, source_function}
Expand Down
2 changes: 1 addition & 1 deletion lib/error_tracker/web/live/show.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
<td class="px-2 align-top"><pre>(<%= line.application || @app %>)</pre></td>
<td>
<pre><%= "#{sanitize_module(line.module)}.#{line.function}/#{line.arity}" %>
<%= if line.line, do: "#{line.file}:#{line.line}", else: "nofile" %></pre>
<%= if line.line, do: "#{line.file}:#{line.line}", else: "(nofile)" %></pre>
</td>
</tr>
</tbody>
Expand Down
4 changes: 2 additions & 2 deletions test/error_tracker_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ defmodule ErrorTrackerTest do
assert error.source_line =~ @relative_file_path
else
assert error.source_function == "erlang.+/2"
assert error.source_line == "nofile"
assert error.source_line == "(nofile)"
end
end

Expand All @@ -54,7 +54,7 @@ defmodule ErrorTrackerTest do
assert error.kind == to_string(UndefinedFunctionError)
assert error.reason =~ "is undefined or private"
assert error.source_function == Exception.format_mfa(m, f, Enum.count(a))
assert error.source_line == "nofile"
assert error.source_line == "(nofile)"
end

test "reports throws" do
Expand Down

0 comments on commit c0c94fa

Please sign in to comment.