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 6ff1268 commit 7233adf
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 7 deletions.
8 changes: 8 additions & 0 deletions .changesets/fix-check-ins-not-being-sent-after-some-time.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
bump: patch
type: fix
---

Fix an issue where, after a certain amount of time, check-ins would no longer be sent.

This issue also caused the default Hackney connection pool to be saturated, affecting other code that uses the default Hackney connection pool.
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
17 changes: 17 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 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 7233adf

Please sign in to comment.