Move logging functions out of Pipeline

mtrudel · Jan 2, 2025 · 8e90637 · 8e90637
1 parent d50a9da
commit 8e90637
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 37 deletions.
diff --git a/lib/bandit/http2/handler.ex b/lib/bandit/http2/handler.ex
@@ -157,7 +157,6 @@ defmodule Bandit.HTTP2.Handler do
         )
       end
 
-    # Reuse Pipeline's error configuration logic
-    Bandit.Pipeline.maybe_log_error(error, stacktrace, state.opts, plug: state.plug)
+    Bandit.Logger.maybe_log_protocol_error(error, stacktrace, state.opts, plug: state.plug)
   end
 end
diff --git a/lib/bandit/logger.ex b/lib/bandit/logger.ex
@@ -87,4 +87,36 @@ defmodule Bandit.Logger do
       domain: [:bandit]
     )
   end
+
+  def maybe_log_protocol_error(error, stacktrace, opts, metadata) do
+    logging_verbosity =
+      case error do
+        %Bandit.TransportError{error: :closed} ->
+          Keyword.get(opts.http, :log_client_closures, false)
+
+        _error ->
+          Keyword.get(opts.http, :log_protocol_errors, :short)
+      end
+
+    case logging_verbosity do
+      :short ->
+        logger_metadata = logger_metadata_for(:error, error, stacktrace, metadata)
+        Logger.error(Exception.format_banner(:error, error, stacktrace), logger_metadata)
+
+      :verbose ->
+        logger_metadata = logger_metadata_for(:error, error, stacktrace, metadata)
+        Logger.error(Exception.format(:error, error, stacktrace), logger_metadata)
+
+      false ->
+        :ok
+    end
+  end
+
+  def logger_metadata_for(kind, reason, stacktrace, metadata) do
+    [domain: [:bandit], crash_reason: crash_reason(kind, reason, stacktrace)]
+    |> Keyword.merge(metadata)
+  end
+
+  defp crash_reason(:throw, reason, stacktrace), do: {{:nocatch, reason}, stacktrace}
+  defp crash_reason(_, reason, stacktrace), do: {reason, stacktrace}
 end
diff --git a/lib/bandit/pipeline.ex b/lib/bandit/pipeline.ex
@@ -221,7 +221,7 @@ defmodule Bandit.Pipeline do
             ] do
     Bandit.Telemetry.stop_span(span, %{}, Enum.into(metadata, %{error: error.message}))
 
-    maybe_log_error(error, stacktrace, opts, metadata)
+    Bandit.Logger.maybe_log_protocol_error(error, stacktrace, opts, metadata)
 
     # We want to do this at the end of the function, since the HTTP2 stack may kill this process
     # in the course of handling a ConnectionError
@@ -234,7 +234,7 @@ defmodule Bandit.Pipeline do
     status = reason |> Plug.Exception.status() |> Plug.Conn.Status.code()
 
     if status in Keyword.get(opts.http, :log_exceptions_with_status_codes, 500..599) do
-      logger_metadata = logger_metadata_for(kind, reason, stacktrace, metadata)
+      logger_metadata = Bandit.Logger.logger_metadata_for(kind, reason, stacktrace, metadata)
       Logger.error(Exception.format(kind, reason, stacktrace), logger_metadata)
 
       Bandit.HTTPTransport.send_on_error(transport, reason)
@@ -244,37 +244,4 @@ defmodule Bandit.Pipeline do
       {:ok, transport}
     end
   end
-
-  # Public since it's used by transports to log errors outside of the pipeline
-  def maybe_log_error(error, stacktrace, opts, metadata) do
-    logging_verbosity =
-      case error do
-        %Bandit.TransportError{error: :closed} ->
-          Keyword.get(opts.http, :log_client_closures, false)
-
-        _error ->
-          Keyword.get(opts.http, :log_protocol_errors, :short)
-      end
-
-    case logging_verbosity do
-      :short ->
-        logger_metadata = logger_metadata_for(:error, error, stacktrace, metadata)
-        Logger.error(Exception.format_banner(:error, error, stacktrace), logger_metadata)
-
-      :verbose ->
-        logger_metadata = logger_metadata_for(:error, error, stacktrace, metadata)
-        Logger.error(Exception.format(:error, error, stacktrace), logger_metadata)
-
-      false ->
-        :ok
-    end
-  end
-
-  defp logger_metadata_for(kind, reason, stacktrace, metadata) do
-    [domain: [:bandit], crash_reason: crash_reason(kind, reason, stacktrace)]
-    |> Keyword.merge(metadata)
-  end
-
-  defp crash_reason(:throw, reason, stacktrace), do: {{:nocatch, reason}, stacktrace}
-  defp crash_reason(_, reason, stacktrace), do: {reason, stacktrace}
 end