Normalize Oban exception reasons for better reports (#878)

Author: whatyouhide

lib/sentry/integrations/oban/error_reporter.ex

@@ -23,17 +23,20 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do
           %{required(:job) => struct(), optional(term()) => term()},
           :no_config
         ) :: :ok
-  def handle_event([:oban, :job, :exception], _measurements, %{job: job} = _metadata, :no_config) do
-    %{reason: reason, stacktrace: stacktrace} = job.unsaved_error
-
+  def handle_event(
+        [:oban, :job, :exception],
+        _measurements,
+        %{job: job, kind: kind, reason: reason, stacktrace: stacktrace} = _metadata,
+        :no_config
+      ) do
     if report?(reason) do
-      report(job, reason, stacktrace)
+      report(job, kind, reason, stacktrace)
     else
       :ok
     end
   end
 
-  defp report(job, reason, stacktrace) do
+  defp report(job, kind, reason, stacktrace) do
     stacktrace =
       case {apply(Oban.Worker, :from_string, [job.worker]), stacktrace} do
         {{:ok, atom_worker}, []} -> [{atom_worker, :process, 1, []}]
@@ -51,15 +54,19 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do
       ]
 
     _ =
-      case maybe_unwrap_exception(reason) do
+      case maybe_unwrap_exception(kind, reason, stacktrace) do
         exception when is_exception(exception) ->
           Sentry.capture_exception(exception, opts)
 
         _other ->
-          Sentry.capture_message(
-            "Oban job #{job.worker} errored out: %s",
-            opts ++ [interpolation_parameters: [inspect(reason)]]
-          )
+          message =
+            case kind do
+              :exit -> "Oban job #{job.worker} exited: %s"
+              :throw -> "Oban job #{job.worker} exited with an uncaught throw: %s"
+              _other -> "Oban job #{job.worker} errored out: %s"
+            end
+
+          Sentry.capture_message(message, opts ++ [interpolation_parameters: [inspect(reason)]])
       end
 
     :ok
@@ -75,12 +82,16 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do
     true
   end
 
-  defp maybe_unwrap_exception(%{reason: {:error, error}} = perform_error)
+  defp maybe_unwrap_exception(
+         :error = _kind,
+         %{reason: {:error, error}} = perform_error,
+         _stacktrace
+       )
        when is_exception(perform_error, Oban.PerformError) and is_exception(error) do
     error
   end
 
-  defp maybe_unwrap_exception(reason) do
-    reason
+  defp maybe_unwrap_exception(kind, reason, stacktrace) do
+    Exception.normalize(kind, reason, stacktrace)
   end
 end

test/sentry/integrations/oban/error_reporter_test.exs

@@ -69,21 +69,21 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
       assert event.fingerprint == [@worker_as_string, "{{ default }}"]
     end
 
-    test "reports non-exception errors to Sentry" do
+    test "reports normalized non-exception errors to Sentry" do
       Sentry.Test.start_collecting()
 
       emit_telemetry_for_failed_job(:error, :undef, [])
 
       assert [event] = Sentry.Test.pop_sentry_reports()
       assert %{job: %Oban.Job{}} = event.integration_meta.oban
 
-      assert event.message == %Sentry.Interfaces.Message{
-               formatted: "Oban job #{@worker_as_string} errored out: :undef",
-               message: "Oban job #{@worker_as_string} errored out: %s",
-               params: [":undef"]
-             }
+      assert event.message == nil
 
-      assert [%Sentry.Interfaces.Thread{stacktrace: %{frames: [stacktrace]}}] = event.threads
+      assert [%{stacktrace: %{frames: [stacktrace]}} = exception] = event.exception
+
+      assert exception.type == "UndefinedFunctionError"
+      assert exception.value == "function #{@worker_as_string}.process/1 is undefined or private"
+      assert exception.mechanism.handled == true
       assert stacktrace.module == MyWorker
       assert stacktrace.function == "#{@worker_as_string}.process/1"
 
@@ -94,6 +94,53 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
       assert event.fingerprint == [@worker_as_string, "{{ default }}"]
     end
 
+    test "reports exits to Sentry" do
+      Sentry.Test.start_collecting()
+
+      emit_telemetry_for_failed_job(:exit, :oops, [])
+
+      assert [event] = Sentry.Test.pop_sentry_reports()
+      assert %{job: %Oban.Job{}} = event.integration_meta.oban
+
+      assert event.message == %Sentry.Interfaces.Message{
+               message: "Oban job #{@worker_as_string} exited: %s",
+               params: [":oops"],
+               formatted: "Oban job #{@worker_as_string} exited: :oops"
+             }
+
+      assert event.exception == []
+
+      assert event.tags.oban_queue == "default"
+      assert event.tags.oban_state == "available"
+      assert event.tags.oban_worker == @worker_as_string
+
+      assert event.fingerprint == [@worker_as_string, "{{ default }}"]
+    end
+
+    test "reports throws to Sentry" do
+      Sentry.Test.start_collecting()
+
+      emit_telemetry_for_failed_job(:throw, :this_was_not_caught, [])
+
+      assert [event] = Sentry.Test.pop_sentry_reports()
+      assert %{job: %Oban.Job{}} = event.integration_meta.oban
+
+      assert event.message == %Sentry.Interfaces.Message{
+               message: "Oban job #{@worker_as_string} exited with an uncaught throw: %s",
+               params: [":this_was_not_caught"],
+               formatted:
+                 "Oban job #{@worker_as_string} exited with an uncaught throw: :this_was_not_caught"
+             }
+
+      assert event.exception == []
+
+      assert event.tags.oban_queue == "default"
+      assert event.tags.oban_state == "available"
+      assert event.tags.oban_worker == @worker_as_string
+
+      assert event.fingerprint == [@worker_as_string, "{{ default }}"]
+    end
+
     for reason <- [:cancel, :discard] do
       test "doesn't report Oban.PerformError with reason #{inspect(reason)}" do
         Sentry.Test.start_collecting()
@@ -116,13 +163,12 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
       %{"id" => "123", "entity" => "user", "type" => "delete"}
       |> MyWorker.new()
       |> Ecto.Changeset.apply_action!(:validate)
-      |> Map.replace!(:unsaved_error, %{kind: kind, reason: reason, stacktrace: stacktrace})
 
     assert :ok =
              ErrorReporter.handle_event(
                [:oban, :job, :exception],
                %{},
-               %{job: job},
+               %{job: job, kind: kind, reason: reason, stacktrace: stacktrace},
                :no_config
              )