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
Use a separate client process #93
Merged
joshuap
merged 12 commits into
honeybadger-io:master
from
sorentwo:chore/use-hackney-directly
Oct 10, 2017
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
993f279
Add get_env to support system variables
sorentwo 89b24b5
Consistently use get_env for all config access
sorentwo eb91845
Include active? status in the client struct
sorentwo d963369
Refactor client module into a GenServer
sorentwo a85df92
Test notifications using a fake api server
sorentwo 2b0ff71
Remove mock testing from logger tests
sorentwo 23764de
Update recursive error reporting test
sorentwo df85ca6
Use a function for lazy exception logging
sorentwo 201cd41
Update and enhance logging in client server
sorentwo 92ee7f6
Add get_all_env/0 to fully resolve system tuples
sorentwo 0559ec2
Fetch the response body before logging it
sorentwo 4160bec
Define all static env as app defaults
sorentwo File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,57 +1,111 @@ | ||
defmodule Honeybadger.Client do | ||
alias Poison, as: JSON | ||
@moduledoc false | ||
|
||
defstruct [:environment_name, :headers, :hostname, :origin, :proxy, :proxy_auth] | ||
use GenServer | ||
|
||
require Logger | ||
|
||
@notices_endpoint "/v1/notices" | ||
@headers [ | ||
{"Accept", "application/json"}, | ||
{"Content-Type", "application/json"} | ||
{"Content-Type", "application/json"}, | ||
{"User-Agent", "Honeybadger Elixir"} | ||
] | ||
@max_connections 20 | ||
@notices_endpoint "/v1/notices" | ||
|
||
# State | ||
|
||
defstruct [:enabled, :headers, :proxy, :proxy_auth, :url] | ||
|
||
# API | ||
|
||
def start_link(config_opts) do | ||
state = new(config_opts) | ||
|
||
GenServer.start_link(__MODULE__, state, [name: __MODULE__]) | ||
end | ||
|
||
def new do | ||
origin = Application.get_env(:honeybadger, :origin) | ||
api_key = Application.get_env(:honeybadger, :api_key) | ||
env_name = Application.get_env(:honeybadger, :environment_name) | ||
hostname = Application.get_env(:honeybadger, :hostname) | ||
proxy = Application.get_env(:honeybadger, :proxy) | ||
proxy_auth = Application.get_env(:honeybadger, :proxy_auth) | ||
%__MODULE__{origin: origin, | ||
headers: headers(api_key), | ||
environment_name: env_name, | ||
hostname: hostname, | ||
proxy: proxy, | ||
proxy_auth: proxy_auth} | ||
end | ||
|
||
def send_notice(%__MODULE__{} = client, notice, http_mod \\ HTTPoison) do | ||
encoded_notice = JSON.encode!(notice) | ||
do_send_notice(client, encoded_notice, http_mod, active_environment?()) | ||
end | ||
|
||
defp do_send_notice(_client, _encoded_notice, _http_mod, false), do: {:ok, :unsent} | ||
defp do_send_notice(client, encoded_notice, http_mod, true) do | ||
case client.proxy do | ||
nil -> | ||
http_mod.post( | ||
client.origin <> @notices_endpoint, encoded_notice, client.headers | ||
) | ||
_ -> | ||
http_mod.post( | ||
client.origin <> @notices_endpoint, encoded_notice, client.headers, | ||
[proxy: client.proxy, proxy_auth: client.proxy_auth] | ||
) | ||
@doc false | ||
def new(opts) do | ||
%__MODULE__{enabled: enabled?(opts), | ||
headers: build_headers(opts), | ||
proxy: get_env(opts, :proxy), | ||
proxy_auth: get_env(opts, :proxy_auth), | ||
url: get_env(opts, :origin) <> @notices_endpoint} | ||
end | ||
|
||
@doc false | ||
def send_notice(notice) when is_map(notice) do | ||
if pid = Process.whereis(__MODULE__) do | ||
GenServer.cast(pid, {:notice, notice}) | ||
else | ||
Logger.warn("[Honeybadger] Unable to notify, the :honeybadger client isn't running") | ||
end | ||
end | ||
|
||
defp headers(api_key) do | ||
[{"X-API-Key", api_key}] ++ @headers | ||
# Callbacks | ||
|
||
def init(state) do | ||
:ok = :hackney_pool.start_pool(__MODULE__, [max_connections: @max_connections]) | ||
|
||
{:ok, state} | ||
end | ||
|
||
def terminate(_reason, _state) do | ||
:ok = :hackney_pool.stop_pool(__MODULE__) | ||
end | ||
|
||
def active_environment? do | ||
env = Application.get_env(:honeybadger, :environment_name) | ||
exclude_envs = Application.get_env(:honeybadger, :exclude_envs, [:dev, :test]) | ||
not env in exclude_envs | ||
def handle_cast({:notice, _notice}, %{enabled: false} = state) do | ||
{:noreply, state} | ||
end | ||
|
||
def handle_cast({:notice, notice}, %{enabled: true} = state) do | ||
payload = Poison.encode!(notice) | ||
|
||
opts = | ||
state | ||
|> Map.take([:proxy, :proxy_auth]) | ||
|> Enum.into(Keyword.new) | ||
|> Keyword.put(:pool, __MODULE__) | ||
|
||
case :hackney.post(state.url, state.headers, payload, opts) do | ||
{:ok, code, _headers, ref} when code >= 200 and code <= 399 -> | ||
Logger.debug("[Honeybadger] API success: #{inspect(body_from_ref(ref))}") | ||
{:ok, code, _headers, ref} when code >= 400 and code <= 504 -> | ||
Logger.error("[Honeybadger] API failure: #{inspect(body_from_ref(ref))}") | ||
{:error, reason} -> | ||
Logger.error("[Honeybadger] connection error: #{inspect(reason)}") | ||
end | ||
|
||
{:noreply, state} | ||
end | ||
|
||
def handle_info(message, state) do | ||
Logger.info("[Honeybadger] unexpected message: #{inspect(message)}") | ||
|
||
{:noreply, state} | ||
end | ||
|
||
# Helpers | ||
|
||
def enabled?(opts) do | ||
env_name = get_env(opts, :environment_name) | ||
excluded = get_env(opts, :exclude_envs) | ||
|
||
not env_name in excluded | ||
end | ||
|
||
defp body_from_ref(ref) do | ||
ref | ||
|> :hackney.body() | ||
|> elem(1) | ||
end | ||
|
||
defp build_headers(opts) do | ||
[{"X-API-Key", get_env(opts, :api_key)}] ++ @headers | ||
end | ||
|
||
defp get_env(opts, key) do | ||
Keyword.get(opts, key, Honeybadger.get_env(key)) | ||
end | ||
end |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a test app I'm trying to configure honeybadger via the HONEYBADGER_API_KEY environment variable. In a phoneix app, the app boots up fine (it determines the environment variable is present), but when it reports an error the Client GenServer is crashing with a bunch of errors like this (eventually the application shuts down instead of looping forever, which is nice):
(note that I omitted the contents of
%Honeybadger.Notice{}
in the "Last message:" line for brevity.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this may be an issue with how we're merging the environment variables. This is what the config looks like:
Edit: Since the tests didn't cover this, it would be awesome if you can add a test case!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was due to the use of
Application.get_all_env
, which didn't make any use ofHoneybadger.get_env/1
. I've added a new function,Honeybadger.get_all_env/0
that takes care of this automatically—and is tested.