From 618c918320ba678c4633a26b1fde0d1789664704 Mon Sep 17 00:00:00 2001 From: Gonzalo <456459+grzuy@users.noreply.github.com> Date: Mon, 13 Jan 2025 16:01:08 -0300 Subject: [PATCH 1/2] fix: logs normalized erlang error when unwrapping Plug.Conn.WrapperError --- lib/bandit/pipeline.ex | 10 +++++++++- test/bandit/http1/plug_test.exs | 12 +++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/bandit/pipeline.ex b/lib/bandit/pipeline.ex index 8d772595..1365c955 100644 --- a/lib/bandit/pipeline.ex +++ b/lib/bandit/pipeline.ex @@ -209,7 +209,15 @@ defmodule Bandit.Pipeline do ) :: {:ok, Bandit.HTTPTransport.t()} | {:error, term()} defp handle_error(:error, %Plug.Conn.WrapperError{} = error, _, transport, span, opts, metadata) do # Unwrap the inner error and handle it - handle_error(error.kind, error.reason, error.stack, transport, span, opts, metadata) + handle_error( + error.kind, + Exception.normalize(:error, error.reason, error.stack), + error.stack, + transport, + span, + opts, + metadata + ) end defp handle_error(:error, %type{} = error, stacktrace, transport, span, opts, metadata) diff --git a/test/bandit/http1/plug_test.exs b/test/bandit/http1/plug_test.exs index d2973ca5..538dd925 100644 --- a/test/bandit/http1/plug_test.exs +++ b/test/bandit/http1/plug_test.exs @@ -112,7 +112,7 @@ defmodule HTTP1PlugTest do get "/" do # Quiet the compiler _ = conn - raise "boom" + 1 = 0 end end @@ -134,9 +134,15 @@ defmodule HTTP1PlugTest do SimpleHTTP1Client.send(client, "GET", "/hello_world", ["host: banana"]) assert SimpleHTTP1Client.connection_closed_for_reading?(client) - assert_receive {:log, %{level: :error, msg: {:string, msg}}}, 500 + assert_receive {:log, %{level: :error, msg: {:string, msg}, meta: meta}}, 500 refute msg =~ "(Plug.Conn.WrapperError)" - assert msg =~ "** (RuntimeError) boom" + assert msg =~ "** (MatchError)" + + assert %{ + domain: [:elixir, :bandit], + crash_reason: {%MatchError{}, [_ | _] = _stacktrace}, + conn: %Plug.Conn{} + } = meta end end From 7453f5c541c94d1ca83ba9c4d89f66c0a91d4e68 Mon Sep 17 00:00:00 2001 From: Gonzalo <456459+grzuy@users.noreply.github.com> Date: Tue, 14 Jan 2025 15:06:41 -0300 Subject: [PATCH 2/2] generalize exception normalization --- lib/bandit/pipeline.ex | 12 +++--------- test/bandit/http1/plug_test.exs | 6 +++--- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/lib/bandit/pipeline.ex b/lib/bandit/pipeline.ex index 1365c955..51af6a8c 100644 --- a/lib/bandit/pipeline.ex +++ b/lib/bandit/pipeline.ex @@ -209,15 +209,7 @@ defmodule Bandit.Pipeline do ) :: {:ok, Bandit.HTTPTransport.t()} | {:error, term()} defp handle_error(:error, %Plug.Conn.WrapperError{} = error, _, transport, span, opts, metadata) do # Unwrap the inner error and handle it - handle_error( - error.kind, - Exception.normalize(:error, error.reason, error.stack), - error.stack, - transport, - span, - opts, - metadata - ) + handle_error(error.kind, error.reason, error.stack, transport, span, opts, metadata) end defp handle_error(:error, %type{} = error, stacktrace, transport, span, opts, metadata) @@ -238,6 +230,8 @@ defmodule Bandit.Pipeline do end defp handle_error(kind, reason, stacktrace, transport, span, opts, metadata) do + reason = Exception.normalize(kind, reason, stacktrace) + Bandit.Telemetry.span_exception(span, kind, reason, stacktrace) status = reason |> Plug.Exception.status() |> Plug.Conn.Status.code() diff --git a/test/bandit/http1/plug_test.exs b/test/bandit/http1/plug_test.exs index 538dd925..d22173dd 100644 --- a/test/bandit/http1/plug_test.exs +++ b/test/bandit/http1/plug_test.exs @@ -46,19 +46,19 @@ defmodule HTTP1PlugTest do assert SimpleHTTP1Client.connection_closed_for_reading?(client) assert_receive {:log, %{level: :error, msg: {:string, msg}, meta: meta}}, 500 - assert msg =~ "(RuntimeError) boom" + assert msg =~ "(ArithmeticError)" assert msg =~ "lib/bandit/pipeline.ex:" assert %{ domain: [:elixir, :bandit], - crash_reason: {%RuntimeError{message: "boom"}, [_ | _] = _stacktrace}, + crash_reason: {%ArithmeticError{}, [_ | _] = _stacktrace}, conn: %Plug.Conn{}, plug: {__MODULE__, []} } = meta end def unknown_crasher(_conn) do - raise "boom" + 1 / 0 end @tag :capture_log