Merge pull request #400 from josevalim/jv-thin-http-client

Make hackney an optional dependency

Author: mitchellhenke

README.md

@@ -17,14 +17,15 @@ If you would like to upgrade a project to use Sentry 7.x, see [here](https://gis
 
 ## Installation
 
-To use Sentry with your projects, edit your mix.exs file and add it as a dependency.  Sentry does not install a JSON library itself, and requires users to have one available.  Sentry will default to trying to use Jason for JSON operations, but can be configured to use other ones.
+To use Sentry with your projects, edit your mix.exs file and add it as a dependency. Sentry does not install a JSON library nor HTTP client by itself.  Sentry will default to trying to use Jason for JSON operations and Hackney for HTTP requests, but can be configured to use other ones. To use the default ones, do:
 
 ```elixir
 defp deps do
   [
     # ...
     {:sentry, "~> 7.0"},
     {:jason, "~> 1.1"},
+    {:hackney, "~> 1.8"}
   ]
 end
 ```
@@ -116,13 +117,15 @@ The full range of options is the following:
 | `tags` | False  | `%{}` | |
 | `release` | False  | None | |
 | `server_name` | False  | None | |
-| `client` | False  | `Sentry.Client` | If you need different functionality for the HTTP client, you can define your own module that implements the `Sentry.HTTPClient` behaviour and set `client` to that module |
+| `client` | False  | `Sentry.HackneyClient` | If you need different functionality for the HTTP client, you can define your own module that implements the `Sentry.HTTPClient` behaviour and set `client` to that module |
 | `hackney_opts` | False  | `[pool: :sentry_pool]` | |
 | `hackney_pool_max_connections` | False  | 50 | |
 | `hackney_pool_timeout` | False  | 5000 | |
 | `before_send_event` | False | | |
 | `after_send_event` | False | | |
 | `sample_rate` | False | 1.0 | |
+| `send_result` | False | `:none` | You may want to set it to `:sync` if testing your Sentry integration. See "Testing with Sentry" |
+| `send_max_attempts` | False | 4 | |
 | `in_app_module_whitelist` | False | `[]` | |
 | `report_deps` | False | True | Will attempt to load Mix dependencies at compile time to report alongside events |
 | `enable_source_code_context` | False | False | |
@@ -242,10 +245,13 @@ test "add/2 does not raise but sends an event to Sentry when given bad input" do
   end)
 
   Application.put_env(:sentry, :dsn, "http://public:secret@localhost:#{bypass.port}/1")
+  Application.put_env(:sentry, :send_result, :sync)
   MyModule.add(1, "a")
 end
 ```
 
+When testing, you will also want to set the `send_result` type to `:sync`, so the request is done synchronously.
+
 ## License
 
 This project is Licensed under the [MIT License](https://github.com/getsentry/sentry-elixir/blob/master/LICENSE).

config/test.exs

@@ -3,8 +3,9 @@ use Mix.Config
 config :sentry,
   environment_name: :test,
   included_environments: [:test],
-  client: Sentry.TestClient,
-  hackney_opts: [recv_timeout: 50]
+  hackney_opts: [recv_timeout: 50],
+  send_result: :sync,
+  send_max_attempts: 1
 
 config :ex_unit,
   assert_receive_timeout: 500

lib/mix/tasks/sentry.send_test_event.ex

@@ -9,7 +9,7 @@ defmodule Mix.Tasks.Sentry.SendTestEvent do
 
   def run(args) do
     unless "--no-compile" in args do
-      Mix.Project.compile(args)
+      Mix.Task.run("compile", args)
     end
 
     Application.ensure_all_started(:sentry)

lib/sentry.ex

@@ -75,14 +75,17 @@ defmodule Sentry do
 
   ## Capturing Exceptions
 
-  Simply calling `capture_exception/2` will send the event.  By default, the event is sent asynchronously and the result can be awaited upon.  The `:result` option can be used to change this behavior.  See `Sentry.Client.send_event/2` for more information.
+  Simply calling `capture_exception/2` will send the event. By default, the event
+  is sent asynchronously and the result can be awaited upon.  The `:result` option
+  can be used to change this behavior.  See `Sentry.Client.send_event/2` for more
+  information.
 
       {:ok, task} = Sentry.capture_exception(my_exception)
       {:ok, event_id} = Task.await(task)
-
       {:ok, another_event_id} = Sentry.capture_exception(other_exception, [event_source: :my_source, result: :sync])
 
   ### Options
+
     * `:event_source` - The source passed as the first argument to `c:Sentry.EventFilter.exclude_exception?/2`
 
   ## Configuring The `Logger` Backend
@@ -95,11 +98,7 @@ defmodule Sentry do
   def start(_type, _opts) do
     children = [
       {Task.Supervisor, name: Sentry.TaskSupervisor},
-      :hackney_pool.child_spec(
-        Sentry.Client.hackney_pool_name(),
-        timeout: Config.hackney_timeout(),
-        max_connections: Config.max_hackney_connections()
-      )
+      Config.client().child_spec()
     ]
 
     validate_json_config!()
@@ -157,10 +156,9 @@ defmodule Sentry do
   def send_event(%Event{} = event, opts) do
     included_environments = Config.included_environments()
     environment_name = Config.environment_name()
-    client = Config.client()
 
     if environment_name in included_environments do
-      client.send_event(event, opts)
+      Sentry.Client.send_event(event, opts)
     else
       :ignored
     end

lib/sentry/client.ex

@@ -1,39 +1,49 @@
 defmodule Sentry.Client do
-  @behaviour Sentry.HTTPClient
-  # Max message length per https://github.com/getsentry/sentry/blob/0fcec33ac94ad81a205f86f208072b0f57b39ff4/src/sentry/conf/server.py#L1021
-  @max_message_length 8_192
-
   @moduledoc ~S"""
-  This module is the default client for sending an event to Sentry via HTTP.
+  This module interfaces directly with Sentry via HTTP.
 
-  It makes use of `Task.Supervisor` to allow sending tasks synchronously or asynchronously, and defaulting to asynchronous. See `Sentry.Client.send_event/2` for more information.
+  The client itself can be configured via the :client
+  configuration. It must implement the `Sentry.HTTPClient`
+  behaviour and it defaults to `Sentry.HackneyClient`.
+
+  It makes use of `Task.Supervisor` to allow sending tasks
+  synchronously or asynchronously, and defaulting to asynchronous.
+  See `send_event/2` for more information.
 
   ### Configuration
 
-  * `:before_send_event` - allows performing operations on the event before
-    it is sent.  Accepts an anonymous function or a {module, function} tuple, and
-    the event will be passed as the only argument.
+    * `:before_send_event` - allows performing operations on the event before
+      it is sent. Accepts an anonymous function or a `{module, function}` tuple,
+      and the event will be passed as the only argument.
 
-  * `:after_send_event` - callback that is called after attempting to send an event.
-    Accepts an anonymous function or a {module, function} tuple. The result of the HTTP call as well as the event will be passed as arguments.
-    The return value of the callback is not returned.
+    * `:after_send_event` - callback that is called after attempting to send an event.
+      Accepts an anonymous function or a `{module, function}` tuple. The result of the
+      HTTP call as well as the event will be passed as arguments. The return value of
+      the callback is not returned.
 
   Example configuration of putting Logger metadata in the extra context:
 
       config :sentry,
-        before_send_event: fn(event) ->
+        before_send_event: {MyModule, :before_send},
+        before_send_event: {MyModule, :after_send}
+
+  where:
+
+      defmodule MyModule do
+        def before_send(event) do
           metadata = Map.new(Logger.metadata)
           %{event | extra: Map.merge(event.extra, metadata)}
-        end,
+        end
 
-        after_send_event: fn(event, result) ->
+        def after_send_event(event, result) do
           case result do
             {:ok, id} ->
               Logger.info("Successfully sent event!")
             _ ->
               Logger.info(fn -> "Did not successfully send event! #{inspect(event)}" end)
           end
         end
+      end
   """
 
   alias Sentry.{Config, Event, Util}
@@ -45,27 +55,39 @@ defmodule Sentry.Client do
   @type dsn :: {String.t(), String.t(), String.t()}
   @type result :: :sync | :none | :async
   @sentry_version 5
-  @max_attempts 4
-  @hackney_pool_name :sentry_pool
+  @sentry_client "sentry-elixir/#{Mix.Project.config()[:version]}"
 
-  quote do
-    unquote(@sentry_client "sentry-elixir/#{Mix.Project.config()[:version]}")
-  end
+  # Max message length per https://github.com/getsentry/sentry/blob/0fcec33ac94ad81a205f86f208072b0f57b39ff4/src/sentry/conf/server.py#L1021
+  @max_message_length 8_192
 
   @doc """
   Attempts to send the event to the Sentry API up to 4 times with exponential backoff.
 
   The event is dropped if it all retries fail.
-  Errors will be logged unless the source is the Sentry.LoggerBackend, which can deadlock by logging within a logger.
+  Errors will be logged unless the source is the Sentry.LoggerBackend, which can
+  deadlock by logging within a logger.
 
   ### Options
-  * `:result` - Allows specifying how the result should be returned. Options include `:sync`, `:none`, and `:async`.  `:sync` will make the API call synchronously, and return `{:ok, event_id}` if successful.  `:none` sends the event from an unlinked child process under `Sentry.TaskSupervisor` and will return `{:ok, ""}` regardless of the result.  `:async` will start an unlinked task and return a tuple of `{:ok, Task.t}` on success where the Task should be awaited upon to receive the result asynchronously.  If you do not call `Task.await/2`, messages will be leaked to the inbox of the current process.  See `Task.Supervisor.async_nolink/2` for more information.  `:none` is the default.
-  * `:sample_rate` - The sampling factor to apply to events.  A value of 0.0 will deny sending any events, and a value of 1.0 will send 100% of events.
-  * Other options, such as `:stacktrace` or `:extra` will be passed to `Sentry.Event.create_event/1` downstream. See `Sentry.Event.create_event/1` for available options.
+
+  * `:result` - Allows specifying how the result should be returned. Options include
+    `:sync`, `:none`, and `:async`.  `:sync` will make the API call synchronously, and
+    return `{:ok, event_id}` if successful.  `:none` sends the event from an unlinked
+    child process under `Sentry.TaskSupervisor` and will return `{:ok, ""}` regardless
+    of the result.  `:async` will start an unlinked task and return a tuple of `{:ok, Task.t}`
+    on success where the Task should be awaited upon to receive the result asynchronously.
+    If you do not call `Task.await/2`, messages will be leaked to the inbox of the current
+    process.  See `Task.Supervisor.async_nolink/2` for more information. `:none` is the default.
+
+  * `:sample_rate` - The sampling factor to apply to events.  A value of 0.0 will deny sending
+    any events, and a value of 1.0 will send 100% of events.
+
+  * Other options, such as `:stacktrace` or `:extra` will be passed to `Sentry.Event.create_event/1`
+    downstream. See `Sentry.Event.create_event/1` for available options.
+
   """
   @spec send_event(Event.t()) :: send_event_result
   def send_event(%Event{} = event, opts \\ []) do
-    result = Keyword.get(opts, :result, :none)
+    result = Keyword.get(opts, :result, Config.send_result())
     sample_rate = Keyword.get(opts, :sample_rate) || Config.sample_rate()
     should_log = event.event_source != :logger
 
@@ -110,7 +132,7 @@ defmodule Sentry.Client do
       {endpoint, auth_headers} when is_binary(endpoint) ->
         {:ok,
          Task.Supervisor.async_nolink(Sentry.TaskSupervisor, fn ->
-           try_request(endpoint, auth_headers, {event, body})
+           try_request(endpoint, auth_headers, {event, body}, Config.send_max_attempts())
            |> maybe_call_after_send_event(event)
          end)}
 
@@ -123,7 +145,7 @@ defmodule Sentry.Client do
   defp do_send_event(event, body, :sync) do
     case get_headers_and_endpoint() do
       {endpoint, auth_headers} when is_binary(endpoint) ->
-        try_request(endpoint, auth_headers, {event, body})
+        try_request(endpoint, auth_headers, {event, body}, Config.send_max_attempts())
         |> maybe_call_after_send_event(event)
 
       {:error, :invalid_dsn} ->
@@ -137,7 +159,7 @@ defmodule Sentry.Client do
     case get_headers_and_endpoint() do
       {endpoint, auth_headers} when is_binary(endpoint) ->
         Task.Supervisor.start_child(Sentry.TaskSupervisor, fn ->
-          try_request(endpoint, auth_headers, {event, body})
+          try_request(endpoint, auth_headers, {event, body}, Config.send_max_attempts())
           |> maybe_call_after_send_event(event)
         end)
 
@@ -152,47 +174,39 @@ defmodule Sentry.Client do
           String.t(),
           list({String.t(), String.t()}),
           {Event.t(), String.t()},
+          pos_integer(),
           {pos_integer(), any()}
         ) :: {:ok, String.t()} | {:error, {:request_failure, any()}}
-  defp try_request(url, headers, event_body_tuple, current_attempt_and_error \\ {1, nil})
+  defp try_request(url, headers, event_body_tuple, max_attempts, current \\ {1, nil})
 
-  defp try_request(_url, _headers, {_event, _body}, {current_attempt, last_error})
-       when current_attempt > @max_attempts,
+  defp try_request(_url, _headers, {_event, _body}, max_attempts, {current_attempt, last_error})
+       when current_attempt > max_attempts,
        do: {:error, {:request_failure, last_error}}
 
-  defp try_request(url, headers, {event, body}, {current_attempt, _last_error}) do
+  defp try_request(url, headers, {event, body}, max_attempts, {current_attempt, _last_error}) do
     case request(url, headers, body) do
       {:ok, id} ->
         {:ok, id}
 
       {:error, error} ->
-        if current_attempt < @max_attempts, do: sleep(current_attempt)
-
-        try_request(url, headers, {event, body}, {current_attempt + 1, error})
+        if current_attempt < max_attempts, do: sleep(current_attempt)
+        try_request(url, headers, {event, body}, max_attempts, {current_attempt + 1, error})
     end
   end
 
   @doc """
-  Makes the HTTP request to Sentry using hackney.
-
-  Hackney options can be set via the `hackney_opts` configuration option.
+  Makes the HTTP request to Sentry using the configured HTTP client.
   """
   @spec request(String.t(), list({String.t(), String.t()}), String.t()) ::
           {:ok, String.t()} | {:error, term()}
   def request(url, headers, body) do
     json_library = Config.json_library()
 
-    hackney_opts =
-      Config.hackney_opts()
-      |> Keyword.put_new(:pool, @hackney_pool_name)
-
-    with {:ok, 200, _, client} <- :hackney.request(:post, url, headers, body, hackney_opts),
-         {:ok, body} <- :hackney.body(client),
+    with {:ok, 200, _, body} <- Config.client().post(url, headers, body),
          {:ok, json} <- json_library.decode(body) do
       {:ok, Map.get(json, "id")}
     else
-      {:ok, status, headers, client} ->
-        :hackney.skip_body(client)
+      {:ok, status, headers, _body} ->
         error_header = :proplists.get_value("X-Sentry-Error", headers, "")
         error = "Received #{status} from Sentry server: #{error_header}"
         {:error, error}
@@ -288,10 +302,6 @@ defmodule Sentry.Client do
     end
   end
 
-  def hackney_pool_name do
-    @hackney_pool_name
-  end
-
   @doc """
   Transform the Event struct into JSON map.
 

lib/sentry/config.ex

@@ -13,6 +13,8 @@ defmodule Sentry.Config do
   @default_path_pattern "**/*.ex"
   @default_context_lines 3
   @default_sample_rate 1.0
+  @default_send_result :none
+  @default_send_max_attempts 4
 
   @permitted_log_level_values ~w(debug info warn error)a
 
@@ -68,7 +70,7 @@ defmodule Sentry.Config do
   end
 
   def client do
-    get_config(:client, default: Sentry.Client, check_dsn: false)
+    get_config(:client, default: Sentry.HackneyClient, check_dsn: false)
   end
 
   def enable_source_code_context do
@@ -105,6 +107,14 @@ defmodule Sentry.Config do
     get_config(:in_app_module_whitelist, default: [], check_dsn: false)
   end
 
+  def send_result do
+    get_config(:send_result, default: @default_send_result, check_dsn: false)
+  end
+
+  def send_max_attempts do
+    get_config(:send_max_attempts, default: @default_send_max_attempts, check_dsn: false)
+  end
+
   def sample_rate do
     get_config(:sample_rate, default: @default_sample_rate, check_dsn: false)
   end