Merge pull request #130 from getsentry/sentry-hackney-pool

separate hackney pool

Author: mitchellhenke

README.md

@@ -67,7 +67,9 @@ config :sentry,
 | `release` | False  | None | |
 | `server_name` | False  | None | |
 | `use_error_logger` | False  | False | |
-| hackney_opts | False  | [] | |
+| hackney_opts | False  | `[pool: :sentry_pool]` | |
+| hackney_pool_max_connections | False  | 50 | |
+| hackney_pool_timeout | False  | 5000 | |
 
 An example production config might look like this:
 
@@ -113,7 +115,7 @@ Now, on our servers, we can set the environment variable appropriately. On
 our local development machines, exceptions will never be sent, because the
 default value is not in the list of `included_environments`.
 
-Sentry uses the [hackney HTTP client](https://github.com/benoitc/hackney) for HTTP requests.  If you need to set [hackney configurations](https://github.com/benoitc/hackney/blob/master/doc/hackney.md#request5) for things like a proxy or different pool, the `hackney_opts` configuration is passed directly to hackney.
+Sentry uses the [hackney HTTP client](https://github.com/benoitc/hackney) for HTTP requests.  Sentry starts its own hackney pool named `:sentry_pool` with a default connection pool of 50, and a connection timeout of 5000 milliseconds.  The pool can be configured with the `hackney_pool_max_connections` and `hackney_pool_timeout` configuration keys.  If you need to set other [hackney configurations](https://github.com/benoitc/hackney/blob/master/doc/hackney.md#request5) for things like a proxy, using your own pool or response timeouts, the `hackney_opts` configuration is passed directly to hackney for each request.
 
 ## Testing Your Configuration
 

config/test.exs

@@ -2,4 +2,5 @@ use Mix.Config
 
 config :sentry,
   environment_name: :test,
-  client: Sentry.TestClient
+  client: Sentry.TestClient,
+  hackney_opts: [recv_timeout: 50]

docs/config.rst

@@ -57,9 +57,17 @@ Optional settings
   Set this to a module that implements the ``Sentry.EventFilter`` behaviour if you would like to prevent
   certain exceptions from being sent.  See below for further documentation.
 
+.. describe:: hackney_pool_max_connections
+
+  Number of connections for Sentry's hackney pool.  This defaults to 50.
+
+.. describe:: hackney_pool_timeout
+
+  Timeout for Sentry's hackney pool.  This defaults to 5000 milliseconds.
+
 .. describe:: hackney_opts
 
-  Sentry uses `hackney <https://github.com/benoitc/hackney>`_.  If you would like to set `options <https://github.com/benoitc/hackney/blob/master/doc/hackney.md#request5>`_ for hackney requests, they can be provided via this configuration.
+  Sentry starts its own hackney pool named ``:sentry_pool``, and defaults to using it.  Hackney's ``pool`` configuration as well others like proxy or response timeout can be set through this configuration as it is passed directly to hackney when making a request.
 
 Testing Your Configuration
 --------------------------

lib/sentry.ex

@@ -94,10 +94,13 @@ defmodule Sentry do
   @client Application.get_env(:sentry, :client, Sentry.Client)
   @use_error_logger Application.get_env(:sentry, :use_error_logger, false)
   @default_environment_name Mix.env
+  @max_hackney_connections Application.get_env(:sentry, :hackney_pool_max_connections, 50)
+  @hackney_timeout Application.get_env(:sentry, :hackney_pool_timeout, 5000)
 
   def start(_type, _opts) do
     children = [
       supervisor(Task.Supervisor, [[name: Sentry.TaskSupervisor]]),
+      :hackney_pool.child_spec(Sentry.Client.hackney_pool_name(),  [timeout: @hackney_timeout, max_connections: @max_hackney_connections])
     ]
     opts = [strategy: :one_for_one, name: Sentry.Supervisor]
 

lib/sentry/client.ex

@@ -4,6 +4,7 @@ defmodule Sentry.Client do
   @type get_dsn :: {String.t, String.t, Integer.t}
   @sentry_version 5
   @max_attempts 4
+  @hackney_pool_name :sentry_pool
 
   quote do
     unquote(@sentry_client "sentry-elixir/#{Mix.Project.config[:version]}")
@@ -59,6 +60,7 @@ defmodule Sentry.Client do
   """
   def request(method, url, headers, body) do
     hackney_opts = Application.get_env(:sentry, :hackney_opts, [])
+                   |> Keyword.put_new(:pool, @hackney_pool_name)
     case :hackney.request(method, url, headers, body, hackney_opts) do
       {:ok, 200, _headers, client} ->
         case :hackney.body(client) do
@@ -112,6 +114,10 @@ defmodule Sentry.Client do
     {endpoint, public_key, secret_key}
   end
 
+  def hackney_pool_name do
+    @hackney_pool_name
+  end
+
   defp fetch_dsn do
     case Application.fetch_env!(:sentry, :dsn) do
       {:system, env_var} -> System.get_env(env_var)
@@ -121,7 +127,7 @@ defmodule Sentry.Client do
 
   defp log_api_error(body) do
     Logger.warn(fn ->
-      ["Failed to send sentry event.", ?\n, body]
+      ["Failed to send Sentry event.", ?\n, body]
     end)
   end
 

test/sentry_test.exs

@@ -1,5 +1,6 @@
 defmodule SentryTest do
   use ExUnit.Case
+  import ExUnit.CaptureLog
 
   test "excludes events properly" do
    Application.put_env(:sentry, :filter, Sentry.TestFilter)
@@ -22,4 +23,25 @@ defmodule SentryTest do
    assert :excluded = Sentry.capture_exception(%ArithmeticError{message: "error"}, [event_source: :plug])
    assert {:ok, _} = Sentry.capture_message("RuntimeError: error", [event_source: :plug])
   end
+
+  test "errors when taking too long to receive response" do
+   Application.put_env(:sentry, :filter, Sentry.TestFilter)
+
+   bypass = Bypass.open
+   Bypass.expect bypass, fn conn ->
+     :timer.sleep(100)
+     assert conn.request_path == "/api/1/store/"
+     assert conn.method == "POST"
+     Plug.Conn.send_resp(conn, 200, ~s<{"id": "340"}>)
+   end
+   Bypass.pass(bypass)
+
+   Application.put_env(:sentry, :dsn, "http://public:secret@localhost:#{bypass.port}/1")
+   Application.put_env(:sentry, :included_environments, [:test])
+   Application.put_env(:sentry, :environment_name, :test)
+
+   assert capture_log(fn ->
+     assert :error = Sentry.capture_message("error", [])
+   end) =~ "Failed to send Sentry event"
+  end
 end