Skip to content

Switch HTTP Client from Hackney to Finch #897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@ This is the official Sentry SDK for [Sentry].

To use Sentry in your project, add it as a dependency in your `mix.exs` file.

Sentry does not install a JSON library nor an HTTP client by itself. Sentry will default to the [built-in `JSON`](https://hexdocs.pm/elixir/JSON.html) for JSON and [Hackney] for HTTP requests, but can be configured to use other ones. To use the default ones, do:
Sentry does not install a JSON library nor an HTTP client by itself. Sentry will default to the [built-in `JSON`](https://hexdocs.pm/elixir/JSON.html) for JSON and [Finch] for HTTP requests, but can be configured to use other ones. To use the default ones, do:

```elixir
defp deps do
[
# ...

{:sentry, "~> 10.8"},
{:hackney, "~> 1.20"}
{:jason, "~> 1.4"},
{:finch, "~> 0.17.0"}
]
end
```
Expand Down Expand Up @@ -203,7 +204,7 @@ Licensed under the MIT license, see [`LICENSE`](./LICENSE).

[Sentry]: http://sentry.io/
[Jason]: https://github.com/michalmuskala/jason
[Hackney]: https://github.com/benoitc/hackney
[Finch]: https://github.com/sneako/finch
[Bypass]: https://github.com/PSPDFKit-labs/bypass
[docs]: https://hexdocs.pm/sentry/readme.html
[logger-handlers]: https://www.erlang.org/doc/apps/kernel/logger_chapter#handlers
Expand Down
5 changes: 4 additions & 1 deletion config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ if config_env() == :test do
tags: %{},
enable_source_code_context: true,
root_source_code_paths: [File.cwd!()],
hackney_opts: [recv_timeout: 50, pool: :sentry_pool],
finch_request_opts: [receive_timeout: 50],
send_result: :sync,
send_max_attempts: 1,
dedup_events: false,
Expand All @@ -22,3 +22,6 @@ if config_env() == :test do
end

config :phoenix, :json_library, if(Code.ensure_loaded?(JSON), do: JSON, else: Jason)

config :sentry,
client: Sentry.FinchClient
2 changes: 1 addition & 1 deletion lib/mix/tasks/sentry.send_test_event.ex
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ defmodule Mix.Tasks.Sentry.SendTestEvent do
end

Mix.shell().info("current environment_name: #{inspect(to_string(Config.environment_name()))}")
Mix.shell().info("hackney_opts: #{inspect(Config.hackney_opts())}\n")
Mix.shell().info("Finch pool options: #{inspect(Config.finch_pool_opts(), pretty: true)}\n")
end

defp send_event(opts) do
Expand Down
86 changes: 64 additions & 22 deletions lib/sentry/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,12 @@ defmodule Sentry.Config do
client: [
type: :atom,
type_doc: "`t:module/0`",
default: Sentry.HackneyClient,
default: Sentry.FinchClient,
doc: """
A module that implements the `Sentry.HTTPClient`
behaviour. Defaults to `Sentry.HackneyClient`, which uses
[hackney](https://github.com/benoitc/hackney) as the HTTP client.
behaviour. Defaults to `Sentry.FinchClient`, which uses
[Finch](https://github.com/sneako/finch) as the HTTP client.
*The default changed from Hackney to Finch in v10.11.0*.
"""
],
send_max_attempts: [
Expand All @@ -366,32 +367,67 @@ defmodule Sentry.Config do
The maximum number of attempts to send an event to Sentry.
"""
],
hackney_opts: [
finch_pool_opts: [
type: :keyword_list,
default: [pool: :sentry_pool],
default: [size: 50],
doc: """
Options to be passed to `hackney`. Only
applied if `:client` is set to `Sentry.HackneyClient`.
Pool options to be passed to `Finch.start_link/1`. These options control
the connection pool behavior. Only applied if `:client` is set to
`Sentry.FinchClient`. See [Finch documentation](https://hexdocs.pm/finch/0.17.0/Finch.html#start_link/1)
for available options.
"""
],
hackney_pool_timeout: [
type: :timeout,
default: 5000,
finch_request_opts: [
type: :keyword_list,
default: [receive_timeout: 5000],
doc: """
The maximum time to wait for a
connection to become available. Only applied if `:client` is set to
`Sentry.HackneyClient`.
Request options to be passed to `Finch.request/4`. These options control
individual request behavior. Only applied if `:client` is set to
`Sentry.FinchClient`. See [Finch documentation](https://hexdocs.pm/finch/0.17.0/Finch.html#request/4)
for available options.
"""
],
hackney_pool_max_connections: [
type: :pos_integer,
default: 50,
doc: """
The maximum number of
connections to keep in the pool. Only applied if `:client` is set to
`Sentry.HackneyClient`.
"""
]
hackney_opts:
[
type: :keyword_list,
default: [pool: :sentry_pool],
doc: """
Options to be passed to `hackney`. Only
applied if `:client` is set to `Sentry.HackneyClient`.
"""
] ++
if(Mix.env() == :test,
do: [],
else: [deprecated: "Use Finch as the default HTTP client instead."]
),
hackney_pool_timeout:
[
type: :timeout,
default: 5000,
doc: """
The maximum time to wait for a
connection to become available. Only applied if `:client` is set to
`Sentry.HackneyClient`.
"""
] ++
if(Mix.env() == :test,
do: [],
else: [deprecated: "Use Finch as the default HTTP client instead."]
),
hackney_pool_max_connections:
[
type: :pos_integer,
default: 50,
doc: """
The maximum number of
connections to keep in the pool. Only applied if `:client` is set to
`Sentry.HackneyClient`.
"""
] ++
if(Mix.env() == :test,
do: [],
else: [deprecated: "Use Finch as the default HTTP client instead."]
)
]

source_code_context_opts_schema = [
Expand Down Expand Up @@ -663,6 +699,12 @@ defmodule Sentry.Config do
@spec traces_sampler() :: traces_sampler_function() | nil
def traces_sampler, do: get(:traces_sampler)

@spec finch_pool_opts() :: keyword()
def finch_pool_opts, do: fetch!(:finch_pool_opts)

@spec finch_request_opts() :: keyword()
def finch_request_opts, do: fetch!(:finch_request_opts)

@spec hackney_opts() :: keyword()
def hackney_opts, do: fetch!(:hackney_opts)

Expand Down
54 changes: 54 additions & 0 deletions lib/sentry/finch_client.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
defmodule Sentry.FinchClient do
@behaviour Sentry.HTTPClient

@moduledoc """
The built-in HTTP client.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shows up correctly in the module doc summary with ExDoc:

Suggested change
The built-in HTTP client.
The built-in HTTP client.


This client implements the `Sentry.HTTPClient` behaviour.
It's based on the [Finch](https://github.com/sneako/finch) Elixir HTTP client,
which is an *optional dependency* of this library. If you wish to use another
HTTP client, you'll have to implement your own `Sentry.HTTPClient`. See the
documentation for `Sentry.HTTPClient` for more information.
Finch is built on top of [NimblePool](https://github.com/dashbitco/nimble_pool). If you need to set other pool configuration options,
see ["Pool Configuration Options"](https://hexdocs.pm/finch/Finch.html#start_link/1-pool-configuration-options)
in the Finch documentation for details on the possible map values.
[finch configuration options](https://hexdocs.pm/finch/Finch.html#start_link/1-pool-configuration-options)
"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
"""
"""
@moduledoc since: "10.11.0"

@moduledoc since: "10.11.0"

@impl true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
@impl true
@impl true

def child_spec do
if Code.ensure_loaded?(Finch) do
case Application.ensure_all_started(:finch) do
{:ok, _apps} -> :ok
{:error, reason} -> raise "failed to start the :finch application: #{inspect(reason)}"
end

Finch.child_spec(
name: __MODULE__,
pools: %{
:default => Sentry.Config.finch_pool_opts()
}
)
else
raise """
cannot start the :sentry application because the HTTP client is set to \
Sentry.FinchClient (which is the default), but the :finch library is not loaded. \
Add :finch to your dependencies to fix this.
"""
end
end

@impl true
def post(url, headers, body) do
request = Finch.build(:post, url, headers, body)

case Finch.request(request, __MODULE__, Sentry.Config.finch_request_opts()) do
{:ok, %Finch.Response{status: status, headers: headers, body: body}} ->
{:ok, status, headers, body}

{:error, error} ->
{:error, error}
end
end
end
2 changes: 1 addition & 1 deletion lib/sentry/http_client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ defmodule Sentry.HTTPClient do
@moduledoc """
A behaviour for HTTP clients that Sentry can use.

The default HTTP client is `Sentry.HackneyClient`.
The default HTTP client is `Sentry.FinchClient` since v10.11.0 (it was based on Hackney before that).

To configure a different HTTP client, implement the `Sentry.HTTPClient` behaviour and
change the `:client` configuration:
Expand Down
5 changes: 3 additions & 2 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ defmodule Sentry.Mixfile do
"Plug and Phoenix": [Sentry.PlugCapture, Sentry.PlugContext, Sentry.LiveViewHook],
Loggers: [Sentry.LoggerBackend, Sentry.LoggerHandler],
"Data Structures": [Sentry.Attachment, Sentry.CheckIn, Sentry.ClientReport],
HTTP: [Sentry.HTTPClient, Sentry.HackneyClient],
HTTP: [Sentry.HTTPClient, Sentry.FinchClient, Sentry.HackneyClient],
Interfaces: [~r/^Sentry\.Interfaces/],
Testing: [Sentry.Test]
],
Expand All @@ -67,7 +67,7 @@ defmodule Sentry.Mixfile do
],
authors: ["Mitchell Henke", "Jason Stiebs", "Andrea Leopardi"]
],
xref: [exclude: [:hackney, :hackney_pool, Plug.Conn, :telemetry]],
xref: [exclude: [Finch, :hackney, :hackney_pool, Plug.Conn, :telemetry]],
aliases: aliases()
]
end
Expand Down Expand Up @@ -100,6 +100,7 @@ defmodule Sentry.Mixfile do

# Optional dependencies
{:hackney, "~> 1.8", optional: true},
{:finch, "~> 0.19.0", optional: true},
{:jason, "~> 1.1", optional: true},
{:phoenix, "~> 1.6", optional: true},
{:phoenix_live_view, "~> 0.20 or ~> 1.0", optional: true},
Expand Down
8 changes: 6 additions & 2 deletions test/logger_backend_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,9 @@ defmodule Sentry.LoggerBackendTest do

start_supervised!(Sentry.ExamplePlugApplication, restart: :temporary)

:hackney.get("http://127.0.0.1:8003/error_route", [], "", [])
Finch.build(:get, "http://127.0.0.1:8003/error_route", [], "", [])
|> Finch.request(Sentry.FinchClient)

assert_receive {^ref, _event}, 1000
assert_receive {^ref, _event}, 1000
after
Expand All @@ -150,7 +152,9 @@ defmodule Sentry.LoggerBackendTest do

start_supervised!({Sentry.ExamplePlugApplication, server: :bandit}, restart: :temporary)

:hackney.get("http://127.0.0.1:8003/error_route", [], "", [])
Finch.build(:get, "http://127.0.0.1:8003/error_route", [], "", [])
|> Finch.request(Sentry.FinchClient)

assert_receive {^ref, _event}, 1000
assert_receive {^ref, _event}, 1000
after
Expand Down
8 changes: 4 additions & 4 deletions test/mix/sentry.send_test_event_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ defmodule Mix.Tasks.Sentry.SendTestEventTest do
import Sentry.TestHelpers

test "prints if :dsn is not set" do
put_test_config(dsn: nil, hackney_opts: [], environment_name: "some_env")
put_test_config(dsn: nil, finch_pool_opts: [], environment_name: "some_env")

output =
capture_io(fn ->
Expand All @@ -15,7 +15,7 @@ defmodule Mix.Tasks.Sentry.SendTestEventTest do
assert output =~ """
Client configuration:
current environment_name: "some_env"
hackney_opts: []
Finch pool options: []
"""

assert output =~ ~s(Event not sent because the :dsn option is not set)
Expand All @@ -35,7 +35,7 @@ defmodule Mix.Tasks.Sentry.SendTestEventTest do
put_test_config(
dsn: "http://public:secret@localhost:#{bypass.port}/1",
environment_name: "test",
hackney_opts: []
finch_pool_opts: []
)

output =
Expand All @@ -49,7 +49,7 @@ defmodule Mix.Tasks.Sentry.SendTestEventTest do
public_key: public
secret_key: secret
current environment_name: "test"
hackney_opts: []
Finch pool options: []
"""

assert output =~ "Sending test event..."
Expand Down
2 changes: 1 addition & 1 deletion test/sentry/client_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ defmodule Sentry.ClientTest do
{:error, %Sentry.ClientError{} = error} =
Client.send_event(event, result: :sync, request_retries: [])

assert error.reason == {:request_failure, :econnrefused}
assert error.reason == {:request_failure, %Mint.TransportError{reason: :econnrefused}}
end

test "logs an error when unable to encode JSON" do
Expand Down
13 changes: 9 additions & 4 deletions test/sentry/logger_handler_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ defmodule Sentry.LoggerHandlerTest do

test "TODO", %{sender_ref: ref} do
start_supervised!(Sentry.ExamplePlugApplication, restart: :temporary)
:hackney.get("http://127.0.0.1:8003/error_route", [], "", [])

Finch.build(:get, "http://127.0.0.1:8003/error_route", [], "", [])
|> Finch.request(Sentry.FinchClient)

assert_receive {^ref, event}, 1000
assert event.original_exception == %RuntimeError{message: "Error"}
end
Expand All @@ -100,7 +103,8 @@ defmodule Sentry.LoggerHandlerTest do
%{sender_ref: ref} do
start_supervised!(Sentry.ExamplePlugApplication, restart: :temporary)

:hackney.get("http://127.0.0.1:8003/error_route", [], "", [])
Finch.build(:get, "http://127.0.0.1:8003/error_route", [], "", [])
|> Finch.request(Sentry.FinchClient)

assert_receive {^ref, event}, 1000
assert event.original_exception == %RuntimeError{message: "Error"}
Expand All @@ -114,7 +118,8 @@ defmodule Sentry.LoggerHandlerTest do
%{sender_ref: ref} do
start_supervised!({Sentry.ExamplePlugApplication, server: :bandit}, restart: :temporary)

:hackney.get("http://127.0.0.1:8003/error_route", [], "", [])
Finch.build(:get, "http://127.0.0.1:8003/error_route", [], "", [])
|> Finch.request(Sentry.FinchClient)

assert_receive {^ref, _event}, 1000
assert_receive {^ref, _event}, 1000
Expand Down Expand Up @@ -686,7 +691,7 @@ defmodule Sentry.LoggerHandlerTest do
put_test_config(
dsn: "http://public:secret@localhost:#{bypass.port}/1",
dedup_events: false,
hackney_opts: [recv_timeout: 500, pool: :sentry_pool]
finch_request_opts: [receive_timeout: 500]
)

Bypass.expect(bypass, fn conn ->
Expand Down
Loading
Loading