Skip to content

Commit

Permalink
Call inspect on cleaned map values
Browse files Browse the repository at this point in the history
Some values that are left "as-is" when cleaned do not implement
`String.Chars`. Since the `Type` struct implements both `Inspect`
and `String.Chars`, call `inspect` on the cleaned value.
  • Loading branch information
unflxw committed Nov 29, 2022
1 parent a4f7618 commit a5c810d
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 8 deletions.
6 changes: 6 additions & 0 deletions .changesets/fix-argument-cleaner-pid-issue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "fix"
---

Fix an issue where reporting an exception for a function call whose arguments contain a map of PID would raise a second exception instead.
2 changes: 1 addition & 1 deletion lib/appsignal/utils/argument_cleaner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ defmodule Appsignal.Utils.ArgumentCleaner do
def clean(argument) when is_map(argument) do
{struct, map} = Map.pop(argument, :__struct__)

"%#{Appsignal.Utils.module_name(struct)}{#{Enum.map_join(map, ", ", fn {key, value} -> "#{inspect(key)} => #{clean(value)}" end)}}"
"%#{Appsignal.Utils.module_name(struct)}{#{Enum.map_join(map, ", ", fn {key, value} -> "#{inspect(key)} => #{inspect(clean(value))}" end)}}"
end

def clean(argument), do: Type.from(argument)
Expand Down
6 changes: 3 additions & 3 deletions test/appsignal/stacktrace_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ defmodule Appsignal.StacktraceTest do

describe "get/0, with an exception with included arguments" do
setup do
String.to_atom("string", :extra_argument)
String.to_atom("string", :extra_argument, 123, :erlang.list_to_pid('<0.0.0>'))
catch
:error, _ -> %{stack: Stacktrace.get()}
end

test "replaces arguments with types", %{stack: stack} do
test "replaces sensitive arguments with types", %{stack: stack} do
[line | _] = Stacktrace.format(stack)
assert line =~ ~r{\(elixir( [\w.-]+)?\) String.to_atom\(binary, atom\)}
assert line =~ ~r{\(elixir( [\w.-]+)?\) String.to_atom\(binary, atom, 123, #PID<0.0.0>\)}
end
end

Expand Down
24 changes: 20 additions & 4 deletions test/appsignal/utils/argument_cleaner_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,45 @@ defmodule Appsignal.Utils.ArgumentCleanerTest do
alias Appsignal.Utils.{ArgumentCleaner, Type}
use ExUnit.Case

test "cleaned types" do
test "sensitive types are converted to type structs" do
assert ArgumentCleaner.clean(:foo) == %Type{type: "atom"}
assert ArgumentCleaner.clean("bar") == %Type{type: "binary"}
assert ArgumentCleaner.clean(<<1::1>>) == %Type{type: "bitstring"}
assert ArgumentCleaner.clean(fn -> nil end) == %Type{type: "function"}
end

test "untouched types" do
test "simple types are kept as-is" do
# is_boolean
assert ArgumentCleaner.clean(true) == true
assert ArgumentCleaner.clean(false) == false
# is_integer
assert ArgumentCleaner.clean(1) == 1
# is_float
assert ArgumentCleaner.clean(1.2) == 1.2
# is_pid
pid = :erlang.list_to_pid('<0.0.0>')
assert ArgumentCleaner.clean(pid) == pid
port = Port.open({:spawn, "echo foo"}, [])
# is_port
port = Port.open({:spawn, "true"}, [])
assert ArgumentCleaner.clean(port) == port
# is_reference
reference = make_ref()
assert ArgumentCleaner.clean(reference) == reference
end

test "map" do
test "values inside composite types are converted to type structs" do
assert ArgumentCleaner.clean({1, :foo}) == %Type{type: "{integer, atom}"}

assert ArgumentCleaner.clean([:erlang.list_to_pid('<0.0.0>'), "bar"]) ==
%Type{type: "[pid, binary]"}
end

test "map keys are preserved" do
assert ArgumentCleaner.clean(%{foo: 1}) == "%{:foo => 1}"
assert ArgumentCleaner.clean(%{foo: "bar"}) == "%{:foo => binary}"
assert ArgumentCleaner.clean(%NonEmptyStruct{foo: "bar"}) == "%NonEmptyStruct{:foo => binary}"

assert ArgumentCleaner.clean(%{foo: :erlang.list_to_pid('<0.0.0>')}) ==
"%{:foo => #PID<0.0.0>}"
end
end

0 comments on commit a5c810d

Please sign in to comment.