Skip to content

Commit

Permalink
Actually fix empty frames in stacktraces
Browse files Browse the repository at this point in the history
See #787.
  • Loading branch information
whatyouhide committed Sep 9, 2024
1 parent 211a29f commit d79919f
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 20 deletions.
28 changes: 14 additions & 14 deletions lib/sentry/client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -287,24 +287,27 @@ defmodule Sentry.Client do
defp render_exception(%Interfaces.Exception{} = exception) do
exception
|> Map.from_struct()
|> render_stacktrace()
|> render_or_delete_stacktrace()
|> update_if_present(:mechanism, &Map.from_struct/1)
end

defp render_thread(%Interfaces.Thread{} = thread) do
thread
|> Map.from_struct()
|> render_stacktrace()
|> render_or_delete_stacktrace()
end

defp render_stacktrace(map) do
case map do
%{stacktrace: %{frames: %Interfaces.Stacktrace{frames: [_ | _]} = stacktrace}} ->
%{stacktrace | frames: Enum.map(stacktrace.frames, &Map.from_struct/1)}
# If there are frames, render the stacktrace, otherwise delete it altogether from the map.
defp render_or_delete_stacktrace(
%{stacktrace: %Interfaces.Stacktrace{frames: [_ | _]}} = exception_or_thread
) do
exception_or_thread
|> Map.update!(:stacktrace, &Map.from_struct/1)
|> update_in([:stacktrace, :frames, Access.all()], &Map.from_struct/1)
end

map_without_stacktrace ->
Map.delete(map_without_stacktrace, :stacktrace)
end
defp render_or_delete_stacktrace(exception_or_thread) do
Map.delete(exception_or_thread, :stacktrace)
end

defp remove_nils(map) when is_map(map) do
Expand Down Expand Up @@ -362,11 +365,8 @@ defmodule Sentry.Client do

defp update_if_present(map, key, fun) do
case Map.pop(map, key) do
{nil, _} ->
map

{value, map} ->
Map.put(map, key, fun.(value))
{nil, _} -> map
{value, map} -> Map.put(map, key, fun.(value))
end
end

Expand Down
53 changes: 47 additions & 6 deletions test/sentry/client_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -84,22 +84,63 @@ defmodule Sentry.ClientTest do
:code.purge(RaisingJSONClient)
end

test "renders threads with stacktrace property deleted if :frames field set to nil if empty" do
test "renders stacktrace for threads" do
event =
Event.create_event(
message: "Stacktrace for threads",
stacktrace: [
{Kernel, :apply, 3, [file: "lib/kernel.ex", line: 1]},
{URI, :new!, 1, [file: "lib/uri.ex", line: 2]}
]
)

assert %{threads: [thread]} = Client.render_event(event)

refute is_struct(thread.stacktrace),
":stacktrace shouldn't be a struct, got: #{inspect(thread.stacktrace)}"

assert [
%{module: URI, function: "URI.new!/1", filename: "lib/uri.ex", lineno: 2},
%{module: Kernel, function: "Kernel.apply/3", filename: "lib/kernel.ex", lineno: 1}
] = thread.stacktrace.frames
end

test "renders threads with :stacktrace property deleted if :frames field set to nil if empty" do
event =
Event.create_event(message: "No frames in stacktrace", stacktrace: [])

client = Client.render_event(event)
assert %{threads: [thread]} = Client.render_event(event)
refute Map.has_key?(thread, :stacktrace)
end

test "renders stacktrace for exceptions" do
event =
Event.transform_exception(
%RuntimeError{message: "foo"},
stacktrace: [
{Kernel, :apply, 3, [file: "lib/kernel.ex", line: 1]},
{URI, :new!, 1, [file: "lib/uri.ex", line: 2]}
]
)

assert %{exception: [exception]} = Client.render_event(event)

refute is_struct(exception.stacktrace),
":stacktrace shouldn't be a struct, got: #{inspect(exception.stacktrace)}"

assert is_nil(get_in(client.threads, [Access.at(0), :stacktrace]))
assert [
%{module: URI, function: "URI.new!/1", filename: "lib/uri.ex", lineno: 2},
%{module: Kernel, function: "Kernel.apply/3", filename: "lib/kernel.ex", lineno: 1}
] = exception.stacktrace.frames
end

test "renders exception with stacktrace property deleted if :frames field set to nil if empty" do
test "renders exception with :stacktrace property deleted if :frames field set to nil if empty" do
event =
Event.transform_exception(%RuntimeError{message: "foo"}, stacktrace: [])

client = Client.render_event(event)
assert %{exception: [exception]} = Client.render_event(event)

assert is_nil(get_in(client.exception, [Access.at(0), :stacktrace]))
refute Map.has_key?(exception, :stacktrace)
end

test "removes non-payload fields" do
Expand Down

0 comments on commit d79919f

Please sign in to comment.