diff --git a/.changesets/replace-arguments-in-stack-traces-with-sanitized-versions-instead-of-stripping-them-out-completely.md b/.changesets/replace-arguments-in-stack-traces-with-sanitized-versions-instead-of-stripping-them-out-completely.md new file mode 100644 index 000000000..d9510cd3b --- /dev/null +++ b/.changesets/replace-arguments-in-stack-traces-with-sanitized-versions-instead-of-stripping-them-out-completely.md @@ -0,0 +1,6 @@ +--- +bump: "patch" +type: "change" +--- + +Replace arguments in stack traces with sanitized versions instead of stripping them out completely diff --git a/lib/appsignal/stacktrace.ex b/lib/appsignal/stacktrace.ex index f368df0f1..e3ece81c8 100644 --- a/lib/appsignal/stacktrace.ex +++ b/lib/appsignal/stacktrace.ex @@ -24,11 +24,15 @@ defmodule Appsignal.Stacktrace do defp format_stacktrace_entry(entry) when is_binary(entry), do: entry - defp format_stacktrace_entry({module, function, arity, location}) when is_list(arity) do - format_stacktrace_entry({module, function, length(arity), location}) + defp format_stacktrace_entry({module, function, arguments, location}) when is_list(arguments) do + Exception.format_stacktrace_entry({module, function, to_types(arguments), location}) end defp format_stacktrace_entry(entry) do Exception.format_stacktrace_entry(entry) end + + defp to_types(arguments) do + Enum.map(arguments, &Appsignal.Utils.ArgumentCleaner.clean/1) + end end diff --git a/lib/appsignal/utils/argument_cleaner.ex b/lib/appsignal/utils/argument_cleaner.ex new file mode 100644 index 000000000..1602bd5ef --- /dev/null +++ b/lib/appsignal/utils/argument_cleaner.ex @@ -0,0 +1,17 @@ +defmodule Appsignal.Utils.ArgumentCleaner do + alias Appsignal.Utils.Type + + def clean(argument) + when is_boolean(argument) or is_integer(argument) or is_float(argument) or is_pid(argument) or + is_port(argument) or is_reference(argument) do + argument + end + + 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)}}" + end + + def clean(argument), do: Type.from(argument) +end diff --git a/lib/appsignal/utils/type.ex b/lib/appsignal/utils/type.ex new file mode 100644 index 000000000..c3d3367bb --- /dev/null +++ b/lib/appsignal/utils/type.ex @@ -0,0 +1,53 @@ +defmodule Appsignal.Utils.Type do + defstruct [:type] + + def from(term) do + %__MODULE__{type: of(term)} + end + + defp of(term) when is_boolean(term), do: "boolean" + + defp of(nil), do: "nil" + + defp of(term) when is_integer(term), do: "integer" + + defp of(term) when is_float(term), do: "float" + + defp of(term) when is_pid(term), do: "pid" + + defp of(term) when is_atom(term), do: "atom" + + defp of(term) when is_bitstring(term) and not is_binary(term), do: "bitstring" + + defp of(term) when is_binary(term), do: "binary" + + defp of(term) when is_function(term), do: "function" + + defp of(term) when is_port(term), do: "port" + + defp of(term) when is_reference(term), do: "reference" + + defp of(term) when is_tuple(term) do + "{#{Enum.map_join(Tuple.to_list(term), ", ", &of/1)}}" + end + + defp of(term) when is_list(term) do + "[#{Enum.map_join(term, ", ", &of/1)}]" + end + + defp of(term) when is_map(term) do + {struct, map} = Map.pop(term, :__struct__) + + "%#{Appsignal.Utils.module_name(struct)}{#{Enum.map_join(map, ", ", fn {key, value} -> "#{of(key)} => #{of(value)}" end)}}" + end + + defp of(_term), do: "unknown" +end + +defimpl Inspect, for: Appsignal.Utils.Type do + def inspect(%Appsignal.Utils.Type{type: type}, _opts), do: type +end + +defimpl String.Chars, for: Appsignal.Utils.Type do + def to_string(%Appsignal.Utils.Type{type: type}), do: type +end diff --git a/test/appsignal/stacktrace_test.exs b/test/appsignal/stacktrace_test.exs index eba6b5e2c..6447d082b 100644 --- a/test/appsignal/stacktrace_test.exs +++ b/test/appsignal/stacktrace_test.exs @@ -33,26 +33,24 @@ defmodule Appsignal.StacktraceTest do all_location = expected_location ++ [error_info: %{module: Exception}] assert Enum.all?(location, &Enum.member?(all_location, &1)) end - end - - test "formats stacktrace lines" do - stacktrace = - [line] = [ - {:elixir_translator, :guard_op, 2, [file: 'src/elixir_translator.erl', line: 317]} - ] - assert Stacktrace.format(stacktrace) == [ - Exception.format_stacktrace_entry(line) - ] + test "formats stacktrace lines", %{stack: stack} do + [line | _] = stack + assert Stacktrace.format([line]) == [Exception.format_stacktrace_entry(line)] + end end - test "replaces arguments with arities" do - stacktrace = [ - {:erl_internal, :op_type, [:get_stacktrace, 0], [file: 'erl_internal.erl', line: 212]} - ] + describe "get/0, with an exception with included arguments" do + setup do + String.to_atom("string", :extra_argument) + catch + :error, _ -> %{stack: Stacktrace.get()} + end - [line] = Stacktrace.format(stacktrace) - assert line =~ ~r{\(stdlib( [\w.-]+)?\) erl_internal.erl:212: :erl_internal.op_type/2} + test "replaces arguments with types", %{stack: stack} do + [line | _] = Stacktrace.format(stack) + assert line =~ ~r{\(elixir( [\w.-]+)?\) String.to_atom\(binary, atom\)} + end end test "handles lists of binaries" do diff --git a/test/appsignal/utils/argument_cleaner_test.exs b/test/appsignal/utils/argument_cleaner_test.exs new file mode 100644 index 000000000..c6d43862b --- /dev/null +++ b/test/appsignal/utils/argument_cleaner_test.exs @@ -0,0 +1,34 @@ +defmodule NonEmptyStruct do + defstruct [:foo] +end + +defmodule Appsignal.Utils.ArgumentCleanerTest do + alias Appsignal.Utils.{ArgumentCleaner, Type} + use ExUnit.Case + + test "cleaned types" 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 + assert ArgumentCleaner.clean(true) == true + assert ArgumentCleaner.clean(false) == false + assert ArgumentCleaner.clean(1) == 1 + assert ArgumentCleaner.clean(1.2) == 1.2 + pid = :erlang.list_to_pid('<0.0.0>') + assert ArgumentCleaner.clean(pid) == pid + port = Port.open({:spawn, "echo foo"}, []) + assert ArgumentCleaner.clean(port) == port + reference = make_ref() + assert ArgumentCleaner.clean(reference) == reference + end + + test "map" do + assert ArgumentCleaner.clean(%{foo: 1}) == "%{:foo => 1}" + assert ArgumentCleaner.clean(%{foo: "bar"}) == "%{:foo => binary}" + assert ArgumentCleaner.clean(%NonEmptyStruct{foo: "bar"}) == "%NonEmptyStruct{:foo => binary}" + end +end diff --git a/test/appsignal/utils/type_test.exs b/test/appsignal/utils/type_test.exs new file mode 100644 index 000000000..2f9de44f4 --- /dev/null +++ b/test/appsignal/utils/type_test.exs @@ -0,0 +1,97 @@ +defmodule EmptyStruct do + defstruct [] +end + +defmodule NonEmptyStruct do + defstruct [:foo] +end + +defmodule Appsignal.Utils.TypeTest do + use ExUnit.Case + alias Appsignal.Utils.Type + + test "atom" do + assert Type.from(:foo).type == "atom" + end + + test "booleans" do + assert Type.from(true).type == "boolean" + assert Type.from(false).type == "boolean" + end + + test "nils" do + assert Type.from(nil).type == "nil" + end + + test "binary" do + assert Type.from("bar").type == "binary" + end + + test "bitstring" do + assert Type.from(<<1::1>>).type == "bitstring" + end + + test "function" do + assert Type.from(fn -> nil end).type == "function" + end + + test "float" do + assert Type.from(1.2).type == "float" + end + + test "integer" do + assert Type.from(1).type == "integer" + end + + test "pid" do + assert Type.from(:erlang.list_to_pid('<0.0.0>')).type == "pid" + end + + test "port" do + assert Type.from(Port.open({:spawn, "echo foo"}, [])).type == "port" + end + + test "reference" do + assert Type.from(make_ref()).type == "reference" + end + + test "empty tuple" do + assert Type.from({}).type == "{}" + end + + test "non-empty tuple" do + assert Type.from({:foo, "bar"}).type == "{atom, binary}" + end + + test "empty list" do + assert Type.from([]).type == "[]" + end + + test "non-empty list" do + assert Type.from([:foo]).type == "[atom]" + end + + test "keyword list" do + assert Type.from(foo: "bar").type == "[{atom, binary}]" + end + + test "empty map" do + assert Type.from(%{}).type == "%{}" + end + + test "non-empty map" do + assert Type.from(%{foo: "bar"}).type == "%{atom => binary}" + end + + test "empty struct" do + assert Type.from(%EmptyStruct{}).type == "%EmptyStruct{}" + end + + test "non-empty struct" do + assert Type.from(%NonEmptyStruct{foo: 1}).type == "%NonEmptyStruct{atom => integer}" + end + + test "inspect" do + assert inspect(Type.from("string")) == "binary" + end +end