Improve option validation in functions (#671)

Author: whatyouhide

lib/sentry.ex

@@ -171,7 +171,7 @@ defmodule Sentry do
   > with `:source_code_exclude_patterns`.
   """
 
-  alias Sentry.{Config, Event, LoggerUtils}
+  alias Sentry.{Client, Config, Event, LoggerUtils}
 
   require Logger
 
@@ -227,13 +227,14 @@ defmodule Sentry do
   def capture_exception(exception, opts \\ []) do
     filter_module = Config.filter()
     event_source = Keyword.get(opts, :event_source)
+    {send_opts, create_event_opts} = Client.split_send_event_opts(opts)
 
     if filter_module.exclude_exception?(exception, event_source) do
       :excluded
     else
       exception
-      |> Event.transform_exception(opts)
-      |> send_event(opts)
+      |> Event.transform_exception(create_event_opts)
+      |> send_event(send_opts)
     end
   end
 
@@ -263,10 +264,13 @@ defmodule Sentry do
   """
   @spec capture_message(String.t(), keyword()) :: send_result
   def capture_message(message, opts \\ []) when is_binary(message) do
-    opts
-    |> Keyword.put(:message, message)
-    |> Event.create_event()
-    |> send_event(opts)
+    {send_opts, create_event_opts} =
+      opts
+      |> Keyword.put(:message, message)
+      |> Client.split_send_event_opts()
+
+    event = Event.create_event(create_event_opts)
+    send_event(event, send_opts)
   end
 
   @doc """
@@ -278,30 +282,7 @@ defmodule Sentry do
 
   ## Options
 
-  The supported options are:
-
-    * `:result` - Allows specifying how the result should be returned. The possible values are:
-
-      * `:sync` - Sentry will make an API call synchronously (including retries) and will
-        return `{:ok, event_id}` if successful.
-
-      * `:none` - Sentry will send the event in the background, in a *fire-and-forget*
-        fashion. The function will return `{:ok, ""}` regardless of whether the API
-        call ends up being successful or not.
-
-      * `:async` - **Not supported anymore**, see the information below.
-
-    * `:sample_rate` - same as the global `:sample_rate` configuration, but applied only to
-      this call. See the module documentation. *Available since v10.0.0*.
-
-    * `:before_send` - same as the global `:before_send` configuration, but
-      applied only to this call. See the module documentation. *Available since v10.0.0*.
-
-    * `:after_send_event` - same as the global `:after_send_event` configuration, but
-      applied only to this call. See the module documentation. *Available since v10.0.0*.
-
-    * `:client` - same as the global `:client` configuration, but
-      applied only to this call. See the module documentation. *Available since v10.0.0*.
+  #{NimbleOptions.docs(Client.send_events_opts_schema())}
 
   > #### Async Send {: .error}
   >
@@ -331,7 +312,7 @@ defmodule Sentry do
         :ignored
 
       included_envs == :all or to_string(Config.environment_name()) in included_envs ->
-        Sentry.Client.send_event(event, opts)
+        Client.send_event(event, opts)
 
       true ->
         :ignored

lib/sentry/client.ex

@@ -12,11 +12,79 @@ defmodule Sentry.Client do
   # Max message length per https://github.com/getsentry/sentry/blob/0fcec33ac94ad81a205f86f208072b0f57b39ff4/src/sentry/conf/server.py#L1021
   @max_message_length 8_192
 
+  # The docs for the options here are generated in the Sentry module, so you can refer to types
+  # and functions and so on like if you were writing these docs in the Sentry module itself.
+  send_event_opts_schema = [
+    result: [
+      type: {:in, [:sync, :none]},
+      doc: """
+      Allows specifying how the result should be returned. The possible values are:
+
+      * `:sync` - Sentry will make an API call synchronously (including retries) and will
+        return `{:ok, event_id}` if successful.
+
+      * `:none` - Sentry will send the event in the background, in a *fire-and-forget*
+        fashion. The function will return `{:ok, ""}` regardless of whether the API
+        call ends up being successful or not.
+      """
+    ],
+    sample_rate: [
+      type: :float,
+      doc: """
+      Same as the global `:sample_rate` configuration, but applied only to
+      this call. See the module documentation. *Available since v10.0.0*.
+      """
+    ],
+    before_send: [
+      type: {:or, [{:fun, 1}, {:tuple, [:atom, :atom]}]},
+      type_doc: "`t:before_send_event_callback/0`",
+      doc: """
+      Same as the global `:before_send` configuration, but
+      applied only to this call. See the module documentation. *Available since v10.0.0*.
+      """
+    ],
+    after_send_event: [
+      type: {:or, [{:fun, 2}, {:tuple, [:atom, :atom]}]},
+      type_doc: "`t:after_send_event_callback/1`",
+      doc: """
+      Same as the global `:after_send_event` configuration, but
+      applied only to this call. See the module documentation. *Available since v10.0.0*.
+      """
+    ],
+    client: [
+      type: :atom,
+      type_doc: "`t:module/0`",
+      doc: """
+      Same as the global `:client` configuration, but
+      applied only to this call. See the module documentation. *Available since v10.0.0*.
+      """
+    ],
+
+    # Private options, only used in testing.
+    request_retries: [
+      type: {:list, :integer},
+      doc: false
+    ]
+  ]
+
+  @send_event_opts_schema NimbleOptions.new!(send_event_opts_schema)
+  @send_event_opts_keys Keyword.keys(send_event_opts_schema)
+
+  @spec send_events_opts_schema() :: NimbleOptions.t()
+  def send_events_opts_schema, do: @send_event_opts_schema
+
+  @spec split_send_event_opts(keyword()) :: {keyword(), keyword()}
+  def split_send_event_opts(options) when is_list(options) do
+    Keyword.split(options, @send_event_opts_keys)
+  end
+
   # This is what executes the "Event Pipeline".
   # See: https://develop.sentry.dev/sdk/unified-api/#event-pipeline
   @spec send_event(Event.t(), keyword()) ::
           {:ok, event_id :: String.t()} | {:error, term()} | :unsampled | :excluded
   def send_event(%Event{} = event, opts) when is_list(opts) do
+    opts = NimbleOptions.validate!(opts, @send_event_opts_schema)
+
     result_type = Keyword.get_lazy(opts, :result, &Config.send_result/0)
     sample_rate = Keyword.get_lazy(opts, :sample_rate, &Config.sample_rate/0)
     before_send = Keyword.get_lazy(opts, :before_send, &Config.before_send/0)

lib/sentry/event.ex

@@ -135,61 +135,129 @@ defmodule Sentry.Event do
     |> Map.drop([:original_exception, :source])
   end
 
-  @doc """
-  Creates an event struct out of collected context and options.
-
-  > #### Merging Options with Context and Config {: .info}
-  >
-  > Some of the options documented below are **merged** with the Sentry context, or
-  > with the Sentry context *and* the configuration. The option you pass here always
-  > has higher precedence, followed by the context and finally by the configuration.
-  >
-  > See also `Sentry.Context` for information on the Sentry context and `Sentry` for
-  > information on configuration.
-
-  ## Options
-
-    * `:exception` - an `t:Exception.t/0`. This is the exception that gets reported in the
+  create_event_opts_schema = [
+    exception: [
+      type: {:custom, __MODULE__, :__validate_exception__, [:exception]},
+      type_doc: "`t:Exception.t/0`",
+      doc: """
+      This is the exception that gets reported in the
       `:exception` field of `t:t/0`. The term passed here also ends up unchanged in the
       `:original_exception` field of `t:t/0`. This option is **required** unless the
-      `:message` option is present. This is not present by default.
-
-    * `:stacktrace` - a stacktrace, as in `t:Exception.stacktrace/0`. This is not present
-      by default.
-
-    * `:message` - a message (`t:String.t/0`). This is not present by default.
-
-    * `:extra` - map of extra context, which gets merged with the current context
+      `:message` option is present. Not present by default.
+      """
+    ],
+    stacktrace: [
+      type: {:list, :any},
+      type_doc: "`t:Exception.stacktrace/0`",
+      doc: """
+      The exception's stacktrace. This can also be used with messages (`:message`). Not
+      present by default.
+      """
+    ],
+    message: [
+      type: :string,
+      doc: """
+      A message to report. The string can contain interpolation markers (`%s`). In that
+      case, you can pass the `:interpolation_parameters` option as well to fill
+      in those parameters. See `Sentry.capture_message/2` for more information on
+      message interpolation. Not present by default.
+      """
+    ],
+    extra: [
+      type: {:map, {:or, [:atom, :string]}, :any},
+      type_doc: "`t:Sentry.Context.extra/0`",
+      default: %{},
+      doc: """
+      Map of extra context, which gets merged with the current context
       (see `Sentry.Context.set_extra_context/1`). If fields collide, the ones
       in the map passed through this option have precedence over the ones in
-      the context. Defaults to `%{}`.
-
-    * `:user` - map of user context, which gets merged with the current context
+      the context.
+      """
+    ],
+    user: [
+      type: :map,
+      type_doc: "`t:Sentry.Context.user_context/0`",
+      default: %{},
+      doc: """
+      Map of user context, which gets merged with the current context
       (see `Sentry.Context.set_user_context/1`). If fields collide, the ones
       in the map passed through this option have precedence over the ones in
-      the context. Defaults to `%{}`.
-
-    * `:tags` - map of tags context, which gets merged with the current context (see
+      the context.
+      """
+    ],
+    tags: [
+      type: {:map, {:or, [:atom, :string]}, :any},
+      type_doc: "`t:Sentry.Context.tags/0`",
+      default: %{},
+      doc: """
+      Map of tags context, which gets merged with the current context (see
       `Sentry.Context.set_tags_context/1`) and with the `:tags` option in the global
       Sentry configuration. If fields collide, the ones in the map passed through
       this option have precedence over the ones in the context, which have precedence
-      over the ones in the configuration. Defaults to `%{}`.
-
-    * `:request` - map of request context, which gets merged with the current context
+      over the ones in the configuration.
+      """
+    ],
+    request: [
+      type: :map,
+      type_doc: "`t:Sentry.Context.request_context/0`",
+      default: %{},
+      doc: """
+      Map of request context, which gets merged with the current context
       (see `Sentry.Context.set_request_context/1`). If fields collide, the ones
       in the map passed through this option have precedence over the ones in
-      the context. Defaults to `%{}`.
+      the context.
+      """
+    ],
+    breadcrumbs: [
+      type: {:list, {:or, [:keyword_list, :map]}},
+      type_doc: "list of `t:keyword/0` or `t:Sentry.Context.breadcrumb/0`",
+      default: [],
+      doc: """
+      List of breadcrumbs. This list gets **prepended** to the list
+      in the context (see `Sentry.Context.add_breadcrumb/1`).
+      """
+    ],
+    level: [
+      type: {:in, [:fatal, :error, :warning, :info, :debug]},
+      type_doc: "`t:level/0`",
+      default: :error,
+      doc: """
+      The level of the event.
+      """
+    ],
+    fingerprint: [
+      type: {:list, :string},
+      default: ["{{ default }}"],
+      doc: """
+      List of the fingerprint for grouping this event.
+      """
+    ],
+    event_source: [
+      type: :atom,
+      doc: """
+      The source of the event. This fills in the `:source` field of the
+      returned struct. This is not present by default.
+      """
+    ]
+  ]
 
-    * `:breadcrumbs` - list of breadcrumbs. This list gets **prepended** to the list
-      in the context (see `Sentry.Context.add_breadcrumb/1`). Defaults to `[]`.
+  @create_event_opts_schema NimbleOptions.new!(create_event_opts_schema)
 
-    * `:level` - error level (see `t:t/0`). Defaults to `:error`.
+  @doc """
+  Creates an event struct out of collected context and options.
 
-    * `:fingerprint` - list of the fingerprint for grouping this event (a list
-      of `t:String.t/0`). Defaults to `["{{ default }}"]`.
+  > #### Merging Options with Context and Config {: .info}
+  >
+  > Some of the options documented below are **merged** with the Sentry context, or
+  > with the Sentry context *and* the configuration. The option you pass here always
+  > has higher precedence, followed by the context and finally by the configuration.
+  >
+  > See also `Sentry.Context` for information on the Sentry context and `Sentry` for
+  > information on configuration.
 
-    * `:event_source` - the source of the event. This fills in the `:source` field of the
-      returned struct. This is not present by default.
+  ## Options
+
+  #{NimbleOptions.docs(@create_event_opts_schema)}
 
   ## Examples
 
@@ -220,6 +288,8 @@ defmodule Sentry.Event do
                | {:exception, Exception.t()}
                | {:stacktrace, Exception.stacktrace()}
   def create_event(opts) when is_list(opts) do
+    opts = NimbleOptions.validate!(opts, @create_event_opts_schema)
+
     timestamp =
       DateTime.utc_now()
       |> DateTime.truncate(:microsecond)
@@ -234,20 +304,18 @@ defmodule Sentry.Event do
       request: request_context
     } = Sentry.Context.get_all()
 
-    level = Keyword.get(opts, :level, :error)
-    fingerprint = Keyword.get(opts, :fingerprint, ["{{ default }}"])
-
-    extra = Map.merge(extra_context, Keyword.get(opts, :extra, %{}))
-    user = Map.merge(user_context, Keyword.get(opts, :user, %{}))
-    request = Map.merge(request_context, Keyword.get(opts, :request, %{}))
+    extra = Map.merge(extra_context, Keyword.fetch!(opts, :extra))
+    user = Map.merge(user_context, Keyword.fetch!(opts, :user))
+    request = Map.merge(request_context, Keyword.fetch!(opts, :request))
 
     tags =
       Config.tags()
       |> Map.merge(tags_context)
-      |> Map.merge(Keyword.get(opts, :tags, %{}))
+      |> Map.merge(Keyword.fetch!(opts, :tags))
 
     breadcrumbs =
-      Keyword.get(opts, :breadcrumbs, [])
+      opts
+      |> Keyword.fetch!(:breadcrumbs)
       |> Kernel.++(breadcrumbs_context)
       |> Enum.take(-1 * Config.max_breadcrumbs())
       |> Enum.map(&struct(Interfaces.Breadcrumb, &1))
@@ -265,8 +333,8 @@ defmodule Sentry.Event do
       event_id: UUID.uuid4_hex(),
       exception: List.wrap(coerce_exception(exception, stacktrace, message)),
       extra: extra,
-      fingerprint: fingerprint,
-      level: level,
+      fingerprint: Keyword.fetch!(opts, :fingerprint),
+      level: Keyword.fetch!(opts, :level),
       message: message,
       modules: :persistent_term.get({:sentry, :loaded_applications}),
       original_exception: exception,
@@ -491,4 +559,13 @@ defmodule Sentry.Event do
       event.fingerprint
     ])
   end
+
+  @doc false
+  def __validate_exception__(term, key) do
+    if is_exception(term) do
+      {:ok, term}
+    else
+      {:error, "expected #{inspect(key)} to be an exception, got: #{inspect(term)}"}
+    end
+  end
 end