Make Sentry.Interfaces.Request a struct (#611)

Author: whatyouhide

lib/sentry/client.ex

@@ -111,6 +111,7 @@ defmodule Sentry.Client do
     |> update_if_present(:message, &String.slice(&1, 0, @max_message_length))
     |> update_if_present(:breadcrumbs, fn bcs -> Enum.map(bcs, &Map.from_struct/1) end)
     |> update_if_present(:sdk, &Map.from_struct/1)
+    |> update_if_present(:request, &(&1 |> Map.from_struct() |> remove_nils()))
     |> update_if_present(:extra, &sanitize_non_jsonable_values(&1, json_library))
     |> update_if_present(:user, &sanitize_non_jsonable_values(&1, json_library))
     |> update_if_present(:tags, &sanitize_non_jsonable_values(&1, json_library))
@@ -125,6 +126,10 @@ defmodule Sentry.Client do
     end)
   end
 
+  defp remove_nils(map) when is_map(map) do
+    :maps.filter(fn _key, value -> not is_nil(value) end, map)
+  end
+
   defp sanitize_non_jsonable_values(map, json_library) do
     # We update the existing map instead of building a new one from scratch
     # due to performance reasons. See the docs for :maps.map/2.

lib/sentry/context.ex

@@ -82,6 +82,23 @@ defmodule Sentry.Context do
           optional(atom()) => term()
         }
 
+  @typedoc """
+  Request context.
+
+  See `set_request_context/1`. This map gets eventually converted
+  into a `Sentry.Interfaces.Request` struct.
+  """
+  @typedoc since: "9.0.0"
+  @type request_context() :: %{
+          optional(:method) => String.t() | nil,
+          optional(:url) => String.t() | nil,
+          optional(:query_string) => String.t() | map() | [{String.t(), String.t()}] | nil,
+          optional(:data) => term(),
+          optional(:cookies) => String.t() | map() | [{String.t(), String.t()}] | nil,
+          optional(:headers) => map() | nil,
+          optional(:env) => map() | nil
+        }
+
   @typedoc """
   Breadcrumb info.
 
@@ -154,9 +171,9 @@ defmodule Sentry.Context do
   """
   @spec get_all() :: %{
           user: user_context(),
+          request: request_context(),
           tags: tags(),
           extra: extra(),
-          request: Interfaces.request(),
           breadcrumbs: list()
         }
   def get_all do
@@ -296,7 +313,7 @@ defmodule Sentry.Context do
       }
 
   """
-  @spec set_request_context(Interfaces.request()) :: :ok
+  @spec set_request_context(request_context()) :: :ok
   def set_request_context(request_context) when is_map(request_context) do
     set_context(@request_key, request_context)
   end

lib/sentry/event.ex

@@ -70,7 +70,7 @@ defmodule Sentry.Event do
           contexts: Interfaces.context(),
           exception: [Interfaces.Exception.t()],
           message: String.t() | nil,
-          request: Interfaces.request(),
+          request: Interfaces.Request.t() | nil,
           sdk: Interfaces.SDK.t() | nil,
           user: Interfaces.user() | nil,
 
@@ -113,7 +113,7 @@ defmodule Sentry.Event do
     modules: %{},
     platform: :elixir,
     release: nil,
-    request: %{},
+    request: %Interfaces.Request{},
     sdk: nil,
     server_name: nil,
     tags: %{},
@@ -212,7 +212,7 @@ defmodule Sentry.Event do
   @spec create_event([option]) :: t()
         when option:
                {:user, Interfaces.user()}
-               | {:request, Interfaces.request()}
+               | {:request, map()}
                | {:extra, Context.extra()}
                | {:breadcrumbs, Context.breadcrumb()}
                | {:tags, Context.tags()}
@@ -274,7 +274,7 @@ defmodule Sentry.Event do
       modules: Map.new(@deps, &{&1, to_string(Application.spec(&1, :vsn))}),
       original_exception: exception,
       release: Config.release(),
-      request: request,
+      request: coerce_request(request),
       sdk: @sdk,
       server_name: Config.server_name() || to_string(:net_adm.localhost()),
       source: source,
@@ -321,6 +321,18 @@ defmodule Sentry.Event do
     end
   end
 
+  @request_fields %Interfaces.Request{} |> Map.from_struct() |> Map.keys() |> MapSet.new()
+
+  defp coerce_request(request) do
+    Enum.reduce(request, %Interfaces.Request{}, fn {key, value}, acc ->
+      if key in @request_fields do
+        Map.replace!(acc, key, value)
+      else
+        raise ArgumentError, "unknown field for the request interface: #{inspect(key)}"
+      end
+    end)
+  end
+
   @doc """
   Transforms an exception to a Sentry event.
 

lib/sentry/interfaces.ex

@@ -33,22 +33,6 @@ defmodule Sentry.Interfaces do
           optional(atom()) => term()
         }
 
-  @typedoc """
-  The **request** interface.
-
-  See <https://develop.sentry.dev/sdk/event-payloads/request>.
-  """
-  @typedoc since: "9.0.0"
-  @type request() :: %{
-          optional(:method) => String.t() | nil,
-          optional(:url) => String.t() | nil,
-          optional(:query_string) => String.t() | map() | [{String.t(), String.t()}] | nil,
-          optional(:data) => term(),
-          optional(:cookies) => String.t() | map() | [{String.t(), String.t()}] | nil,
-          optional(:headers) => map() | nil,
-          optional(:env) => map() | nil
-        }
-
   @typedoc """
   The generic **context** interface.
 
@@ -57,6 +41,37 @@ defmodule Sentry.Interfaces do
   @typedoc since: "9.0.0"
   @type context() :: map()
 
+  defmodule Request do
+    @moduledoc """
+    The struct for the **request** interface.
+
+    See <https://develop.sentry.dev/sdk/event-payloads/request>.
+    """
+
+    @moduledoc since: "9.0.0"
+
+    @typedoc since: "9.0.0"
+    @type t() :: %__MODULE__{
+            method: String.t() | nil,
+            url: String.t() | nil,
+            query_string: String.t() | map() | [{String.t(), String.t()}] | nil,
+            data: term(),
+            cookies: String.t() | map() | [{String.t(), String.t()}] | nil,
+            headers: map() | nil,
+            env: map() | nil
+          }
+
+    defstruct [
+      :method,
+      :url,
+      :query_string,
+      :data,
+      :cookies,
+      :headers,
+      :env
+    ]
+  end
+
   defmodule SDK do
     @moduledoc """
     The struct for the **SDK** interface.

test/context_test.exs

@@ -3,7 +3,7 @@ defmodule Sentry.ContextTest do
 
   import Sentry.TestEnvironmentHelper
 
-  alias Sentry.{Context, Event}
+  alias Sentry.{Context, Event, Interfaces}
 
   doctest Context, except: [add_breadcrumb: 1]
 
@@ -64,7 +64,7 @@ defmodule Sentry.ContextTest do
 
     event = Event.create_event(request: %{method: "GET"})
 
-    assert event.request == %{url: "https://wow", method: "GET"}
+    assert event.request == %Interfaces.Request{url: "https://wow", method: "GET"}
   end
 
   test "passing in extra context as option overrides Context" do

test/envelope_test.exs

@@ -1,7 +1,7 @@
 defmodule Sentry.EnvelopeTest do
   use ExUnit.Case, async: true
 
-  alias Sentry.{Envelope, Event}
+  alias Sentry.{Envelope, Event, Interfaces}
 
   describe "from_binary/1" do
     test "parses envelope with empty headers" do
@@ -62,7 +62,7 @@ defmodule Sentry.EnvelopeTest do
         original_exception: nil,
         platform: :elixir,
         release: nil,
-        request: %{},
+        request: %Interfaces.Request{},
         server_name: "john-linux",
         tags: %{},
         timestamp: "2021-10-09T03:53:22",

test/event_test.exs

@@ -165,7 +165,7 @@ defmodule Sentry.EventTest do
 
       assert event.user == %{id: 2, username: "foo", email: "[email protected]"}
 
-      assert event.request == %{
+      assert event.request == %Interfaces.Request{
                method: "POST",
                url: "https://a.com",
                data: "yes"
@@ -259,6 +259,12 @@ defmodule Sentry.EventTest do
       assert event.source == :plug
       assert event.original_exception == exception
     end
+
+    test "raises if the :request option contains non-request fields" do
+      assert_raise ArgumentError, ~r/unknown field for the request interface: :bad_key/, fn ->
+        Event.create_event(request: %{method: "GET", bad_key: :indeed})
+      end
+    end
   end
 
   test "respects the max_breadcrumbs configuration" do