Refactor maybe_log_send_result/2 (#799)

Co-authored-by: Andrea Leopardi <[email protected]>

Author: savhappy

lib/sentry/client.ex

@@ -39,7 +39,6 @@ defmodule Sentry.Client do
       |> Envelope.from_check_in()
       |> Transport.encode_and_post_envelope(client, request_retries)
 
-    _ = maybe_log_send_result(send_result, check_in)
     send_result
   end
 
@@ -175,7 +174,6 @@ defmodule Sentry.Client do
           |> Envelope.from_event()
           |> Transport.encode_and_post_envelope(client, request_retries)
 
-        _ = maybe_log_send_result(send_result, event)
         send_result
     end
   end
@@ -296,30 +294,4 @@ defmodule Sentry.Client do
       {value, map} -> Map.put(map, key, fun.(value))
     end
   end
-
-  def maybe_log_send_result(_send_result, %Event{source: :logger}) do
-    :ok
-  end
-
-  @spec maybe_log_send_result(
-          {:error, any()} | {:ok, binary()},
-          Sentry.CheckIn.t() | Event.t() | list(Event.t())
-        ) ::
-          :ok | nil
-  def maybe_log_send_result(send_result, events) do
-    if is_list(events) && Enum.any?(events, &(&1.source == :logger)) do
-      :ok
-    else
-      message =
-        case send_result do
-          {:error, %ClientError{} = error} ->
-            "Failed to send Sentry event. #{Exception.message(error)}"
-
-          {:ok, _} ->
-            nil
-        end
-
-      if message, do: LoggerUtils.log(fn -> [message] end)
-    end
-  end
 end

lib/sentry/transport.ex

@@ -3,9 +3,7 @@ defmodule Sentry.Transport do
 
   # This module is exclusively responsible for encoding and POSTing envelopes to Sentry.
 
-  alias Sentry.ClientError
-  alias Sentry.Config
-  alias Sentry.Envelope
+  alias Sentry.{ClientError, Config, Envelope, LoggerUtils}
 
   @default_retries [1000, 2000, 4000, 8000]
   @sentry_version 5
@@ -27,14 +25,19 @@ defmodule Sentry.Transport do
           {:ok, envelope_id :: String.t()} | {:error, ClientError.t()}
   def encode_and_post_envelope(%Envelope{} = envelope, client, retries \\ @default_retries)
       when is_atom(client) and is_list(retries) do
-    case Envelope.to_binary(envelope) do
-      {:ok, body} ->
-        {endpoint, headers} = get_endpoint_and_headers()
-        post_envelope_with_retries(client, endpoint, headers, body, retries)
+    result =
+      case Envelope.to_binary(envelope) do
+        {:ok, body} ->
+          {endpoint, headers} = get_endpoint_and_headers()
+          post_envelope_with_retries(client, endpoint, headers, body, retries)
 
-      {:error, reason} ->
-        {:error, ClientError.new({:invalid_json, reason})}
-    end
+        {:error, reason} ->
+          {:error, ClientError.new({:invalid_json, reason})}
+      end
+
+    _ = maybe_log_send_result(result, envelope.items)
+
+    result
   end
 
   defp post_envelope_with_retries(client, endpoint, headers, payload, retries_left) do
@@ -130,4 +133,21 @@ defmodule Sentry.Transport do
 
     {dsn.endpoint_uri, auth_headers}
   end
+
+  defp maybe_log_send_result(send_result, events) do
+    if Enum.any?(events, &(Map.has_key?(&1, :source) && &1.source == :logger)) do
+      :ok
+    else
+      message =
+        case send_result do
+          {:error, %ClientError{} = error} ->
+            "Failed to send Sentry event. #{Exception.message(error)}"
+
+          {:ok, _} ->
+            nil
+        end
+
+      if message, do: LoggerUtils.log(fn -> [message] end)
+    end
+  end
 end

lib/sentry/transport/sender.ex

@@ -3,7 +3,7 @@ defmodule Sentry.Transport.Sender do
 
   use GenServer
 
-  alias Sentry.{Envelope, Event, Transport, Client}
+  alias Sentry.{Envelope, Event, Transport}
 
   require Logger
 
@@ -41,10 +41,10 @@ defmodule Sentry.Transport.Sender do
 
   @impl GenServer
   def handle_cast({:send, client, %Event{} = event}, %__MODULE__{} = state) do
-    event
-    |> Envelope.from_event()
-    |> Transport.encode_and_post_envelope(client)
-    |> Client.maybe_log_send_result([event])
+    _ =
+      event
+      |> Envelope.from_event()
+      |> Transport.encode_and_post_envelope(client)
 
     # We sent an event, so we can decrease the number of queued events.
     Transport.SenderPool.decrease_queued_events_counter()

test/sentry/transport_test.exs

@@ -2,6 +2,7 @@ defmodule Sentry.TransportTest do
   use Sentry.Case, async: false
 
   import Sentry.TestHelpers
+  import ExUnit.CaptureLog
 
   alias Sentry.{ClientError, Envelope, Event, HackneyClient, Transport}
 
@@ -245,6 +246,12 @@ defmodule Sentry.TransportTest do
                  Transport.encode_and_post_envelope(envelope, HackneyClient, _retries = [])
                end)
 
+      log =
+        capture_log(fn ->
+          Transport.encode_and_post_envelope(envelope, HackneyClient, _retries = [])
+        end)
+
+      assert log =~ "[warning]"
       assert_received {:request, ^ref}
     end
   end