Clean up Sentry.ClientError

Author: whatyouhide

lib/sentry/client_error.ex

@@ -4,10 +4,10 @@ defmodule Sentry.ClientError do
   reporting an error or a message.
 
   This struct is designed to manage and handle errors originating from operations
-  in the Sentry client. The `:reason` field signifies the cause of the error
-  as an atom or tuple.
+  in the Sentry client. The `:reason` field contains the cause of the error
+  as an atom or tuple (see `t:reason/0`).
 
-  To raise instances of this exception, you can use `Kernel.raise/1`. When crafting
+  To raise instances of this exception, you can use `raise/1`. When crafting
   formatted error messages for purposes such as logging or presentation, consider
   leveraging `Exception.message/1`.
   """
@@ -28,14 +28,15 @@ defmodule Sentry.ClientError do
             nil | {status :: 100..599, headers :: [{String.t(), String.t()}], body :: binary()}
         }
 
+  @typedoc """
+  The reason for a Sentry error exception.
+  """
   @type reason() ::
           :too_many_retries
           | :server_error
           | {:invalid_json, Exception.t()}
-          | {:request_failure, String.t()}
-          | {:request_failure, atom}
-          | {:request_failure, term()}
-          | {atom(), term(), [term()]}
+          | {:request_failure, reason :: :inet.posix() | term()}
+          | {Exception.kind(), reason :: term(), Exception.stacktrace()}
 
   @doc false
   @spec new(reason()) :: t
@@ -44,63 +45,62 @@ defmodule Sentry.ClientError do
   end
 
   @doc false
-  @spec server_error(
-          status :: 100..599,
-          headers ::
-            [{String.t(), String.t()}],
-          body :: binary()
-        ) :: t
+  @spec server_error(status :: 100..599, headers :: [{String.t(), String.t()}], body :: binary()) ::
+          t
   def server_error(status, headers, body) do
     %__MODULE__{reason: :server_error, http_response: {status, headers, body}}
   end
 
   @impl true
-  def message(%__MODULE__{reason: reason, http_response: http_response})
-      when is_nil(http_response) do
-    "request failure reason: #{format(reason)}"
-  end
-
   def message(%__MODULE__{reason: reason, http_response: http_response}) do
-    "request failure reason: #{format(reason, http_response)}"
+    "Sentry failed to report event: #{format(reason, http_response)}"
   end
 
-  defp format(:too_many_retries) do
-    "Sentry responded with status 429 - Too Many Requests"
+  defp format(:server_error, {status, headers, body}) do
+    """
+    the Sentry server responded with an error, the details are below.
+    HTTP Status: #{status}
+    Response Headers: #{inspect(headers)}
+    Response Body: #{inspect(body)}
+    """
   end
 
-  defp format({:invalid_json, reason}) do
-    "Invalid JSON -> #{Exception.message(reason)}"
+  defp format(reason, nil) do
+    format(reason)
   end
 
-  defp format({:request_failure, reason}) when is_binary(reason) do
-    "#{reason}"
+  defp format(:too_many_retries) do
+    "Sentry responded with status 429 - Too Many Requests"
   end
 
-  defp format({:request_failure, reason}) when is_atom(reason) do
-    case :inet.format_error(reason) do
-      ~c"unknown POSIX error" -> inspect(reason)
-      formatted -> List.to_string(formatted)
-    end
+  defp format({:invalid_json, reason}) do
+    "the Sentry SDK could not encode the event to JSON: #{Exception.message(reason)}"
   end
 
   defp format({:request_failure, reason}) do
-    inspect(reason)
+    "there was a request failure: #{format_request_failure(reason)}"
   end
 
   defp format({kind, data, stacktrace}) do
     """
-    Sentry failed to report event due to an unexpected error:
+    there was an unexpected error:
 
     #{Exception.format(kind, data, stacktrace)}\
     """
   end
 
-  defp format(:server_error, {status, headers, body}) do
-    """
-    Sentry failed to report the event due to a server error.
-    HTTP Status: #{status}
-    Response Headers: #{inspect(headers)}
-    Response Body: #{inspect(body)}
-    """
+  defp format_request_failure(reason) when is_binary(reason) do
+    reason
+  end
+
+  defp format_request_failure(reason) when is_atom(reason) do
+    case :inet.format_error(reason) do
+      ~c"unknown POSIX error" -> inspect(reason)
+      formatted -> List.to_string(formatted)
+    end
+  end
+
+  defp format_request_failure(reason) do
+    inspect(reason)
   end
 end

test/sentry/client_error_test.exs

@@ -2,56 +2,61 @@ defmodule Sentry.ClientErrorTest do
   use Sentry.Case
   alias Sentry.ClientError
 
-  describe "message/1" do
-    test "with atom - returns message" do
-      assert "request failure reason: Sentry responded with status 429 - Too Many Requests" =
-               result_msg(:too_many_retries)
+  describe "c:Exception.message/1" do
+    test "with an atom reason" do
+      assert message_for_reason(:too_many_retries) ==
+               "Sentry responded with status 429 - Too Many Requests"
     end
 
-    test "with tuple {:invalid_json, _} - returns message" do
-      assert "request failure reason: Invalid JSON -> unexpected byte at position 0: 0x69 (\"i\")" =
-               result_msg(
-                 {:invalid_json,
-                  %Jason.DecodeError{position: 0, token: nil, data: "invalid JSON"}}
-               )
+    test "with {:invalid_json, _} reason" do
+      assert message_for_reason(
+               {:invalid_json, %Jason.DecodeError{position: 0, token: nil, data: "invalid JSON"}}
+             ) ==
+               "the Sentry SDK could not encode the event to JSON: unexpected byte at position 0: 0x69 (\"i\")"
     end
 
-    test "with tuple {:request_failure, _} and binary - returns message" do
-      assert "request failure reason: Received 400 from Sentry server: some error" =
-               result_msg({:request_failure, "Received 400 from Sentry server: some error"})
-    end
+    test "with {:request_failure, reason} reason" do
+      assert message_for_reason({:request_failure, "some error"}) ==
+               "there was a request failure: some error"
 
-    test "with tuple {:request_failure, _} and atom - returns message" do
-      assert "request failure reason: connection refused" =
-               result_msg({:request_failure, :econnrefused})
-    end
+      assert message_for_reason({:request_failure, :econnrefused}) ==
+               "there was a request failure: connection refused"
+
+      assert message_for_reason({:request_failure, :whatever}) ==
+               "there was a request failure: unknown POSIX error: whatever"
 
-    test "with tuple {:request_failure, _} and anything else - returns message" do
-      assert "request failure reason: {:error, %RuntimeError{message: \"I'm a really bad HTTP client\"}}" =
-               result_msg(
-                 {:request_failure,
-                  {:error, %RuntimeError{message: "I'm a really bad HTTP client"}}}
-               )
+      assert message_for_reason({:request_failure, 123}) == "there was a request failure: 123"
     end
 
-    test "with Exception- returns message" do
-      {kind, data, stacktrace} =
-        {:error, %RuntimeError{message: "I'm a really bad HTTP client"}, []}
+    test "with {kind, reason, stacktrace} reason" do
+      {:current_stacktrace, stacktrace} = Process.info(self(), :current_stacktrace)
 
-      assert "request failure reason: Sentry failed to report event due to an unexpected error:\n\n** (RuntimeError) I'm a really bad HTTP client" =
-               result_msg({kind, data, stacktrace})
+      assert message_for_reason(
+               {:error, %RuntimeError{message: "I'm a really bad HTTP client"}, stacktrace}
+             ) =~ """
+             there was an unexpected error:
+
+             ** (RuntimeError) I'm a really bad HTTP client
+                 (elixir\
+             """
     end
 
-    test "with server_error- returns message" do
-      {status, headers, body} =
-        {400, "Rate limiting.", "{}"}
+    test "with :server_error and HTTP response as the reason" do
+      exception = ClientError.server_error(400, [{"X-Foo", "true"}], "{}")
 
-      assert "request failure reason: Sentry failed to report the event due to a server error.\nHTTP Status: 400\nResponse Headers: \"Rate limiting.\"\nResponse Body: \"{}\"\n" =
-               ClientError.server_error(status, headers, body) |> ClientError.message()
+      assert Exception.message(exception) == """
+             Sentry failed to report event: the Sentry server responded with an error, the details are below.
+             HTTP Status: 400
+             Response Headers: [{"X-Foo", "true"}]
+             Response Body: "{}"
+             """
     end
   end
 
-  defp result_msg(reason) do
-    reason |> ClientError.new() |> ClientError.message()
+  defp message_for_reason(reason) do
+    assert "Sentry failed to report event: " <> rest =
+             reason |> ClientError.new() |> Exception.message()
+
+    rest
   end
 end