Add threads interface and fix messages with stacktraces (#668)

Author: whatyouhide

lib/sentry/client.ex

@@ -154,6 +154,7 @@ defmodule Sentry.Client do
     |> update_if_present(:user, &sanitize_non_jsonable_values(&1, json_library))
     |> update_if_present(:tags, &sanitize_non_jsonable_values(&1, json_library))
     |> update_if_present(:exception, fn list -> Enum.map(list, &render_exception/1) end)
+    |> update_if_present(:threads, fn list -> Enum.map(list, &render_thread/1) end)
   end
 
   defp render_exception(%Interfaces.Exception{} = exception) do
@@ -164,6 +165,14 @@ defmodule Sentry.Client do
     end)
   end
 
+  defp render_thread(%Interfaces.Thread{} = thread) do
+    thread
+    |> Map.from_struct()
+    |> update_if_present(:stacktrace, fn %Interfaces.Stacktrace{frames: frames} ->
+      %{frames: Enum.map(frames, &Map.from_struct/1)}
+    end)
+  end
+
   defp remove_nils(map) when is_map(map) do
     :maps.filter(fn _key, value -> not is_nil(value) end, map)
   end

lib/sentry/event.ex

@@ -67,6 +67,7 @@ defmodule Sentry.Event do
           message: String.t() | nil,
           request: Interfaces.Request.t() | nil,
           sdk: Interfaces.SDK.t() | nil,
+          threads: [Interfaces.Thread.t()] | nil,
           user: Interfaces.user() | nil,
 
           # Non-payload fields.
@@ -113,6 +114,7 @@ defmodule Sentry.Event do
     server_name: nil,
     tags: %{},
     transaction: nil,
+    threads: nil,
     user: %{},
 
     # "Culprit" is not documented anymore and we should move to transactions at some point.
@@ -255,7 +257,7 @@ defmodule Sentry.Event do
     stacktrace = Keyword.get(opts, :stacktrace)
     source = Keyword.get(opts, :event_source)
 
-    %__MODULE__{
+    event = %__MODULE__{
       breadcrumbs: breadcrumbs,
       contexts: generate_contexts(),
       culprit: culprit_from_stacktrace(Keyword.get(opts, :stacktrace, [])),
@@ -277,23 +279,25 @@ defmodule Sentry.Event do
       timestamp: timestamp,
       user: user
     }
-  end
 
-  defp coerce_exception(_exception = nil, _stacktrace = nil, _message) do
-    nil
+    # If we have a message *and* a stacktrace, but no exception, we need to store the stacktrace
+    # information within a "thread" interface. This is how the Python SDK also does it. An issue
+    # was opened in the sentry-elixir repo about this, but this is also a Sentry issue (if there
+    # is an exception of type "message" with a stacktrace *and* a "message" attribute, it should
+    # still show properly). This issue is now tracked in Sentry itself:
+    # https://github.com/getsentry/sentry/issues/61239
+    if message && stacktrace && is_nil(exception) do
+      add_thread_with_stacktrace(event, stacktrace)
+    else
+      event
+    end
   end
 
-  defp coerce_exception(_exception = nil, stacktrace_or_nil, message) when is_binary(message) do
-    stacktrace =
-      if is_list(stacktrace_or_nil) do
-        %Interfaces.Stacktrace{frames: stacktrace_to_frames(stacktrace_or_nil)}
-      end
-
-    %Interfaces.Exception{
-      type: "message",
-      value: message,
-      stacktrace: stacktrace
-    }
+  # If we have a message with a stacktrace, but no exceptions, for now we store the stacktrace in
+  # the "threads" interface and we don't fill in the "exception" interface altogether. This might
+  # be eventually fixed in Sentry itself: https://github.com/getsentry/sentry/issues/61239
+  defp coerce_exception(_exception = nil, _stacktrace_or_nil, message) when is_binary(message) do
+    nil
   end
 
   defp coerce_exception(exception, stacktrace_or_nil, _message) when is_exception(exception) do
@@ -328,6 +332,15 @@ defmodule Sentry.Event do
     end)
   end
 
+  defp add_thread_with_stacktrace(%__MODULE__{} = event, stacktrace) when is_list(stacktrace) do
+    thread = %Interfaces.Thread{
+      id: UUID.uuid4_hex(),
+      stacktrace: %Interfaces.Stacktrace{frames: stacktrace_to_frames(stacktrace)}
+    }
+
+    %__MODULE__{event | threads: [thread]}
+  end
+
   @doc """
   Transforms an exception to a Sentry event.
 

lib/sentry/interfaces.ex

@@ -190,4 +190,38 @@ defmodule Sentry.Interfaces do
 
     defstruct [:type, :category, :message, :data, :level, :timestamp]
   end
+
+  defmodule Thread do
+    @moduledoc """
+    The struct for the **thread** interface.
+
+    See <https://develop.sentry.dev/sdk/event-payloads/threads>.
+    """
+
+    @moduledoc since: "10.1.0"
+
+    @typedoc since: "10.1.0"
+    @type t() :: %__MODULE__{
+            id: term(),
+            crashed: boolean() | nil,
+            current: boolean() | nil,
+            main: boolean() | nil,
+            name: String.t() | nil,
+            state: term(),
+            held_locks: [term()],
+            stacktrace: Sentry.Interfaces.Stacktrace.t() | nil
+          }
+
+    @enforce_keys [:id]
+    defstruct [
+      :id,
+      :crashed,
+      :current,
+      :main,
+      :name,
+      :state,
+      :held_locks,
+      :stacktrace
+    ]
+  end
 end

test/event_test.exs

@@ -210,6 +210,41 @@ defmodule Sentry.EventTest do
              } = Event.create_event(message: "Test message")
     end
 
+    test "fills in the message and threads interfaces when passing the :message option with :stacktrace" do
+      {:current_stacktrace, stacktrace} = Process.info(self(), :current_stacktrace)
+      put_test_config(environment_name: "my_env")
+
+      assert %Event{
+               breadcrumbs: [],
+               environment: "my_env",
+               exception: [],
+               extra: %{},
+               level: :error,
+               message: "Test message",
+               platform: :elixir,
+               release: nil,
+               request: %{},
+               tags: %{},
+               user: %{},
+               contexts: %{os: %{name: _, version: _}, runtime: %{name: _, version: _}},
+               threads: [%Interfaces.Thread{id: thread_id, stacktrace: thread_stacktrace}]
+             } = Event.create_event(message: "Test message", stacktrace: stacktrace)
+
+      assert is_binary(thread_id) and byte_size(thread_id) > 0
+
+      assert [
+               %Interfaces.Stacktrace.Frame{
+                 context_line: nil,
+                 in_app: false,
+                 lineno: _,
+                 post_context: [],
+                 pre_context: [],
+                 vars: %{}
+               }
+               | _rest
+             ] = thread_stacktrace.frames
+    end
+
     test "fills in private (:__...__) fields" do
       exception = %RuntimeError{message: "foo"}
 

test/logger_backend_test.exs

@@ -37,12 +37,13 @@ defmodule Sentry.LoggerBackendTest do
     pid = start_supervised!(TestGenServer)
     TestGenServer.run_async(pid, fn _state -> throw("I am throwing") end)
     assert_receive {^ref, event}
-    assert [exception] = event.exception
-    assert exception.value =~ ~s<GenServer #{inspect(pid)} terminating\n>
-    assert exception.value =~ ~s<** (stop) bad return value: "I am throwing"\n>
-    assert exception.value =~ ~s<Last message: {:"$gen_cast",>
-    assert exception.value =~ ~s<State: []>
-    assert exception.stacktrace.frames == []
+    assert [] = event.exception
+    assert [thread] = event.threads
+    assert event.message =~ ~s<GenServer #{inspect(pid)} terminating\n>
+    assert event.message =~ ~s<** (stop) bad return value: "I am throwing"\n>
+    assert event.message =~ ~s<Last message: {:"$gen_cast",>
+    assert event.message =~ ~s<State: []>
+    assert thread.stacktrace.frames == []
   end
 
   test "abnormal GenServer exit is reported" do
@@ -51,12 +52,12 @@ defmodule Sentry.LoggerBackendTest do
     pid = start_supervised!(TestGenServer)
     TestGenServer.run_async(pid, fn state -> {:stop, :bad_exit, state} end)
     assert_receive {^ref, event}
-    assert [exception] = event.exception
-    assert exception.type == "message"
-    assert exception.value =~ ~s<GenServer #{inspect(pid)} terminating\n>
-    assert exception.value =~ ~s<** (stop) :bad_exit\n>
-    assert exception.value =~ ~s<Last message: {:"$gen_cast",>
-    assert exception.value =~ ~s<State: []>
+    assert [] = event.exception
+    assert [_thread] = event.threads
+    assert event.message =~ ~s<GenServer #{inspect(pid)} terminating\n>
+    assert event.message =~ ~s<** (stop) :bad_exit\n>
+    assert event.message =~ ~s<Last message: {:"$gen_cast",>
+    assert event.message =~ ~s<State: []>
   end
 
   test "bad function call causing GenServer crash is reported" do
@@ -96,16 +97,15 @@ defmodule Sentry.LoggerBackendTest do
 
     assert_receive {^ref, event}
 
-    assert [exception] = event.exception
-
-    assert exception.type == "message"
+    assert [] = event.exception
+    assert [thread] = event.threads
 
-    assert exception.value =~
+    assert event.message =~
              "Task #{inspect(task_pid)} started from #{inspect(self())} terminating\n"
 
-    assert exception.value =~ "** (stop) exited in: GenServer.call("
-    assert exception.value =~ "** (EXIT) time out"
-    assert length(exception.stacktrace.frames) > 0
+    assert event.message =~ "** (stop) exited in: GenServer.call("
+    assert event.message =~ "** (EXIT) time out"
+    assert length(thread.stacktrace.frames) > 0
   end
 
   test "captures errors from spawn/0 in Plug app" do

test/sentry/logger_handler_test.exs

@@ -175,7 +175,8 @@ defmodule Sentry.LoggerHandlerTest do
       assert event.message =~ "** (stop) :bad_exit"
 
       if System.otp_release() >= "26" do
-        assert hd(event.exception).type == "message"
+        assert [] = event.exception
+        assert [_thread] = event.threads
       end
     end
 
@@ -273,13 +274,12 @@ defmodule Sentry.LoggerHandlerTest do
 
       assert_receive {^ref, event}
 
-      assert [exception] = event.exception
-
-      assert exception.type == "message"
+      assert [] = event.exception
+      assert [thread] = event.threads
 
-      assert exception.value =~ "** (stop) exited in: GenServer.call("
-      assert exception.value =~ "** (EXIT) time out"
-      assert length(exception.stacktrace.frames) > 0
+      assert event.message =~ "** (stop) exited in: GenServer.call("
+      assert event.message =~ "** (EXIT) time out"
+      assert length(thread.stacktrace.frames) > 0
     end
 
     test "reports crashes on c:GenServer.init/1", %{sender_ref: ref} do