diff --git a/.changesets/fix-argument-cleaner-pid-issue.md b/.changesets/fix-argument-cleaner-pid-issue.md new file mode 100644 index 000000000..6325b04a8 --- /dev/null +++ b/.changesets/fix-argument-cleaner-pid-issue.md @@ -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. diff --git a/lib/appsignal/utils/argument_cleaner.ex b/lib/appsignal/utils/argument_cleaner.ex index 1602bd5ef..69aa1ab16 100644 --- a/lib/appsignal/utils/argument_cleaner.ex +++ b/lib/appsignal/utils/argument_cleaner.ex @@ -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) diff --git a/test/appsignal/stacktrace_test.exs b/test/appsignal/stacktrace_test.exs index 6447d082b..5a2e0b80f 100644 --- a/test/appsignal/stacktrace_test.exs +++ b/test/appsignal/stacktrace_test.exs @@ -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 diff --git a/test/appsignal/utils/argument_cleaner_test.exs b/test/appsignal/utils/argument_cleaner_test.exs index c6d43862b..de1db3c87 100644 --- a/test/appsignal/utils/argument_cleaner_test.exs +++ b/test/appsignal/utils/argument_cleaner_test.exs @@ -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