Skip to content

Commit

Permalink
Close Hackney connections after use
Browse files Browse the repository at this point in the history
Make sure that Hackney connections are closed after use. This fixes
an issue where the Hackney pool does not correctly claim back
connections, which fixes #970.

Implement a `transmit_and_close/3` convenience method in the
transmitter for use cases where the body is not of interest, meaning
that the connection can be immediately closed.
  • Loading branch information
unflxw committed Nov 6, 2024
1 parent 37ae062 commit 80827bf
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 7 deletions.
6 changes: 3 additions & 3 deletions lib/appsignal/check_in/scheduler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ defmodule Appsignal.CheckIn.Scheduler do
config = Appsignal.Config.config()
endpoint = "#{config[:logging_endpoint]}/check_ins/json"

case @transmitter.transmit(endpoint, {Enum.reverse(events), :ndjson}, config) do
{:ok, status_code, _, _} when status_code in 200..299 ->
case @transmitter.transmit_and_close(endpoint, {Enum.reverse(events), :ndjson}, config) do
{:ok, status_code, _} when status_code in 200..299 ->
@integration_logger.trace("Transmitted #{description}")

{:ok, status_code, _, _} ->
{:ok, status_code, _} ->
@integration_logger.error(
"Failed to transmit #{description}: status code was #{status_code}"
)
Expand Down
2 changes: 2 additions & 0 deletions lib/appsignal/diagnose/report.ex
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ defmodule Appsignal.Diagnose.Report do
case Transmitter.transmit(config[:diagnose_endpoint], {%{diagnose: report}, :json}, config) do
{:ok, 200, _, reference} ->
{:ok, body} = :hackney.body(reference)
:hackney.close(reference)

case Jason.decode(body) do
{:ok, response} -> {:ok, response["token"]}
Expand All @@ -21,6 +22,7 @@ defmodule Appsignal.Diagnose.Report do

{:ok, status_code, _, reference} ->
{:ok, body} = :hackney.body(reference)
:hackney.close(reference)
{:error, %{status_code: status_code, body: body}}

{:error, reason} ->
Expand Down
23 changes: 23 additions & 0 deletions lib/appsignal/transmitter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ defmodule Appsignal.Transmitter do
def transmit(url, payload_and_format \\ {nil, nil}, config \\ nil)
def transmit(url, nil, config), do: transmit(url, {nil, nil}, config)

# This function calls `:hackney.request/5` -- it is the
# caller's responsibility to ensure that `:hackney.close/1` is
# called afterwards.
#
# If you're not interested in the body, only in the status code
# and headers, use `transmit_and_close/3` instead.
def transmit(url, {payload, format}, config) do
config = config || Appsignal.Config.config()

Expand All @@ -32,6 +38,17 @@ defmodule Appsignal.Transmitter do
request(:post, url, headers, body)
end

def transmit_and_close(url, payload_and_format \\ {nil, nil}, config \\ nil) do
case transmit(url, payload_and_format, config) do
{:ok, status, headers, reference} ->
:hackney.close(reference)
{:ok, status, headers}

{:error, reason} ->
{:error, reason}
end
end

defp encode_body(nil, _), do: ""
defp encode_body(payload, :json), do: Jason.encode!(payload)

Expand All @@ -43,6 +60,12 @@ defmodule Appsignal.Transmitter do
defp options do
ssl_options() ++
[
<<<<<<< HEAD
=======
checkout_timeout: 5000,
connect_timeout: 5000,
recv_timeout: 5000,
>>>>>>> 92f64eea (Close Hackney connections after use)
pool: :appsignal_transmitter
]
end
Expand Down
8 changes: 4 additions & 4 deletions lib/appsignal/utils/push_api_key_validator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ defmodule Appsignal.Utils.PushApiKeyValidator do
def validate(config) do
url = "#{config[:endpoint]}/1/auth"

case Transmitter.transmit(url, nil, config) do
{:ok, 200, _, _} -> :ok
{:ok, 401, _, _} -> {:error, :invalid}
{:ok, status_code, _, _} -> {:error, status_code}
case Transmitter.transmit_and_close(url, nil, config) do
{:ok, 200, _} -> :ok
{:ok, 401, _} -> {:error, :invalid}
{:ok, status_code, _} -> {:error, status_code}
{:error, reason} -> {:error, reason}
end
end
Expand Down
10 changes: 10 additions & 0 deletions test/support/appsignal/fake_transmitter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ defmodule Appsignal.FakeTransmitter do
Agent.get(__MODULE__, & &1[:response]).()
end

def transmit_and_close(url, payload, config) do
case transmit(url, payload, config) do
{:ok, status, headers, _reference} ->
{:ok, status, headers}

{:error, reason} ->
{:error, reason}
end
end

def transmitted do
Agent.get(__MODULE__, &Enum.reverse(&1[:transmitted]))
end
Expand Down

0 comments on commit 80827bf

Please sign in to comment.