Error handling and logging improvements via re-raising

lib/bandit/application.ex

@@ -8,6 +8,7 @@ defmodule Bandit.Application do
           {:ok, pid}
           | {:error, {:already_started, pid} | {:shutdown, term} | term}
   def start(_type, _args) do
+    Logger.add_translator({Bandit.LoggerTranslator, :translate})
     children = [Bandit.Clock]
     Supervisor.start_link(children, strategy: :one_for_one)
   end

lib/bandit/logger_translator.ex

@@ -0,0 +1,30 @@
+defmodule Bandit.LoggerTranslator do
+  def translate(_min_level, :error, :report, {:logger, %{reason: {:bad_return_value, value}}}) do
+    stacktrace = []
+    {
+      :ok,
+      Exception.format(:throw, value, stacktrace),
+      crash_reason: {{:nocatch, value}, stacktrace}, domain: :bandit
+    }
+  end
+
+  def translate(_min_level, :error, :report, {:logger, %{reason: {exception, stacktrace}}}) when is_exception(exception) do
+    {
+      :ok,
+      Exception.format(:error, exception, stacktrace),
+      crash_reason: {exception, stacktrace}, domain: :bandit
+    }
+  end
+
+  def translate(_min_level, :error, :report, {:logger, %{reason: {reason, stacktrace}}}) do
+    {
+      :ok,
+      Exception.format(:exit, reason, stacktrace),
+      crash_reason: {reason, stacktrace}, domain: :bandit
+    }
+  end
+
+  def translate(_min_level, _level, _kind, _message) do
+    :none
+  end
+end

lib/bandit/pipeline.ex

@@ -9,6 +9,12 @@ defmodule Bandit.Pipeline do
   @type scheme :: String.t() | nil
   @type path :: String.t() | :*
 
+  @protocol_errors [
+    Bandit.HTTPError,
+    Bandit.HTTP2.Errors.StreamError,
+    Bandit.HTTP2.Errors.ConnectionError
+  ]
+
   require Logger
 
   @spec run(
@@ -45,13 +51,20 @@ defmodule Bandit.Pipeline do
             Bandit.Telemetry.stop_span(span, adapter.metrics, %{conn: conn})
             {:upgrade, adapter.transport, protocol, opts}
         end
-      rescue
-        error -> handle_error(error, __STACKTRACE__, transport, span, opts)
+      catch
+        kind, reason ->
+          handle_error(kind, reason, __STACKTRACE__, transport, span, opts)
       end
     rescue
-      error ->
-        span = Bandit.Telemetry.start_span(:request, measurements, metadata)
-        handle_error(error, __STACKTRACE__, transport, span, opts)
+      error in @protocol_errors ->
+        handle_error(
+          :error,
+          error,
+          __STACKTRACE__,
+          transport,
+          Bandit.Telemetry.start_span(:request, measurements, metadata),
+          opts
+        )
     end
   end
 
@@ -194,18 +207,15 @@ defmodule Bandit.Pipeline do
   end
 
   @spec handle_error(
+          :error | :throw | :exit,
           Exception.t(),
           Exception.stacktrace(),
           Bandit.HTTPTransport.t(),
           Bandit.Telemetry.t(),
           map()
         ) :: {:ok, Bandit.HTTPTransport.t()} | {:error, term()}
-  defp handle_error(%type{} = error, stacktrace, transport, span, opts)
-       when type in [
-              Bandit.HTTPError,
-              Bandit.HTTP2.Errors.StreamError,
-              Bandit.HTTP2.Errors.ConnectionError
-            ] do
+  defp handle_error(:error, %type{} = error, stacktrace, transport, span, opts)
+       when type in @protocol_errors do
     Bandit.Telemetry.stop_span(span, %{}, %{error: error.message})
 
     maybe_log_error(error, stacktrace, opts.http)
@@ -216,16 +226,17 @@ defmodule Bandit.Pipeline do
     {:error, error}
   end
 
-  defp handle_error(error, stacktrace, transport, span, opts) do
-    Bandit.Telemetry.span_exception(span, :exit, error, stacktrace)
-    status = error |> Plug.Exception.status() |> Plug.Conn.Status.code()
+  defp handle_error(kind, reason, stacktrace, transport, span, opts) do
+    # TODO: Fix this telementry kind
+    Bandit.Telemetry.span_exception(span, :exit, reason, stacktrace)
+    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.error(Exception.format(:error, error, stacktrace), domain: [:bandit])
-      Bandit.HTTPTransport.send_on_error(transport, error)
-      {:error, error}
+      Bandit.HTTPTransport.send_on_error(transport, reason)
+
+      :erlang.raise(kind, reason, stacktrace)
     else
-      Bandit.HTTPTransport.send_on_error(transport, error)
+      Bandit.HTTPTransport.send_on_error(transport, reason)
       {:ok, transport}
     end
   end

test/bandit/http1/request_test.exs

@@ -140,19 +140,50 @@ defmodule HTTP1RequestTest do
   describe "plug error logging" do
     test "it should return 500 and log when unknown exceptions are raised", context do
       output =
-        capture_log(fn ->
+        capture_log([metadata: [:domain]], fn ->
           {:ok, response} = Req.get(context.req, url: "/unknown_crasher")
           assert response.status == 500
           Process.sleep(100)
         end)
 
       assert output =~ "(RuntimeError) boom"
+      assert output =~ "domain=bandit"
     end
 
     def unknown_crasher(_conn) do
       raise "boom"
     end
 
+    test "it should return 500 and log when throws", context do
+      output =
+        capture_log(fn ->
+          {:ok, response} = Req.get(context.req, url: "/uncaught_throw")
+          assert response.status == 500
+          Process.sleep(100)
+        end)
+
+      assert output =~ "(throw) \"something\""
+    end
+
+    def uncaught_throw(_conn) do
+      throw "something"
+    end
+
+    test "it should return 500 and log when abnormal exit", context do
+      output =
+        capture_log(fn ->
+          {:ok, response} = Req.get(context.req, url: "/abnormal_exit")
+          assert response.status == 500
+          Process.sleep(100)
+        end)
+
+      assert output =~ "(exit) :abnormal"
+    end
+
+    def abnormal_exit(_conn) do
+      exit(:abnormal)
+    end
+
     test "it should return the code and not log when known exceptions are raised", context do
       output =
         capture_log(fn ->