Skip to content
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

Manual notify exception #76

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,31 @@ defmodule YourApp.Router do
...
```

## Manually notify of exception

If your controller action manually handles an error, the notifier will never be run.

To manually notify of an error you can use `BoomNotifier` in your controller and call the `manual_notify_error/2` callback:

```elixir
defmodule MyController do
use BoomNotifier
Copy link
Member

Choose a reason for hiding this comment

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

I feel this should be an import instead of a use. So, you can avoid all the extra complexity of checking if we are in the context of a router or a controller in BoomNotifier.__using__.

In fact, it is probably fine to say that the user should invoke BoomNotifier.notify_error directly (the user can or not add an import)

Copy link
Member

Choose a reason for hiding this comment

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

Well, the only thing to solve here is to make sure that config validations runs at least once. If the module is used in the Router is fine, but we should make it mandatory (even if it is not going to be used there).

Another option would be to check configs directly in the new Config module, as part of any of the functions defined there.. or maybe as an operation that is done only once and cached.


def some_action(conn, _params) do
try do
# ...
rescue
error ->
# ...
manual_notify_error(conn, error)
# ...
end
end

,
...
```

## Custom notifiers

To create a custom notifier, you need to implement the `BoomNotifier.Notifier` behaviour:
Expand Down
52 changes: 30 additions & 22 deletions lib/boom_notifier.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ defmodule BoomNotifier do
# Responsible for sending a notification to each notifier every time an
# exception is raised.

alias BoomNotifier.Config
alias BoomNotifier.ErrorStorage
alias BoomNotifier.NotifierSenderServer
require Logger
Expand All @@ -25,10 +26,10 @@ defmodule BoomNotifier do
end
end

def walkthrough_notifiers(settings, callback) do
case Keyword.get(settings, :notifiers) do
nil ->
run_callback(settings, callback)
def walkthrough_notifiers(callback) do
case Config.notifiers() do
[] ->
run_callback(Config.single_notifier_config(), callback)

notifiers_settings when is_list(notifiers_settings) ->
Enum.each(notifiers_settings, &run_callback(&1, callback))
Expand All @@ -49,13 +50,14 @@ defmodule BoomNotifier do
end
end

defmacro __using__(config) do
defmacro __using__(_config) do
quote location: :keep do
import BoomNotifier

error_handler_in_use = {:handle_errors, 2} in Module.definitions_in(__MODULE__)
is_router = __MODULE__ |> to_string() |> String.split(".") |> List.last() == "Router"

unless error_handler_in_use do
if is_router and not error_handler_in_use do
use Plug.ErrorHandler

@impl Plug.ErrorHandler
Expand All @@ -65,40 +67,46 @@ defmodule BoomNotifier do
end

# Notifiers validation
walkthrough_notifiers(
unquote(config),
fn notifier, options -> validate_notifiers(notifier, options) end
)
walkthrough_notifiers(fn notifier, options -> validate_notifiers(notifier, options) end)

def notify_error(conn, %{kind: :error, reason: %mod{}} = error) do
settings = unquote(config)
{ignored_exceptions, _settings} = Keyword.pop(settings, :ignore_exceptions, [])

unless Enum.member?(ignored_exceptions, mod) do
do_notify_error(conn, settings, error)
unless Enum.member?(Config.ignore_exceptions(), mod) do
do_notify_error(conn, error)
end
end

def notify_error(conn, error) do
do_notify_error(conn, unquote(config), error)
do_notify_error(conn, error)
end

def manual_notify_error(conn, error) do
Copy link
Member

Choose a reason for hiding this comment

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

What about using the already existing notify_error function? I think all the logic there applies here (limits & filters for exceptions). So, we would have similar ways in router and controller to notify errors when exceptions are being intercepted by user code (error_handler or simple try-catch-rescue blocks).

{_error_kind, error_info} =
ErrorInfo.build(%{kind: :error, reason: error, stack: []}, conn, Config.custom_data())

# Triggers the notification in each notifier
walkthrough_notifiers(fn notifier, options ->
NotifierSenderServer.send(
notifier,
[error_info],
options
)
end)
end

defp do_notify_error(conn, settings, error) do
{custom_data, _settings} = Keyword.pop(settings, :custom_data, :nothing)
{error_kind, error_info} = ErrorInfo.build(error, conn, custom_data)
defp do_notify_error(conn, error) do
{error_kind, error_info} = ErrorInfo.build(error, conn, Config.custom_data())

ErrorStorage.add_errors(error_kind, error_info)

if ErrorStorage.send_notification?(error_kind) do
occurrences = ErrorStorage.get_errors(error_kind)

# Triggers the notification in each notifier
walkthrough_notifiers(settings, fn notifier, options ->
walkthrough_notifiers(fn notifier, options ->
NotifierSenderServer.send(notifier, occurrences, options)
end)

{notification_trigger, _settings} =
Keyword.pop(settings, :notification_trigger, :always)
notification_trigger = Config.notification_trigger()

ErrorStorage.clear_errors(notification_trigger, error_kind)
end
Expand Down
42 changes: 42 additions & 0 deletions lib/boom_notifier/config.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
defmodule BoomNotifier.Config do
@moduledoc """
This module provides the functionality for fetching configuration settings and their defaults.
"""

@custom_data_default :nothing
@ignore_exceptions_default []
@notifiers_default []
@notification_trigger_default :always
@notifier_default nil
@options_default []

def custom_data do
get_config(:custom_data, @custom_data_default)
end

def ignore_exceptions do
get_config(:ignore_exceptions, @ignore_exceptions_default)
end

def notifiers do
get_config(:notifiers, @notifiers_default)
end

def notification_trigger do
get_config(:notification_trigger, @notification_trigger_default)
end

def single_notifier_config do
[
notifier: get_config(:notifier, @notifier_default),
options: get_config(:options, @options_default)
]
end

defp get_config(key, default) do
case Application.fetch_env(:boom_notifier, key) do
{:ok, value} -> value
_ -> default
end
end
end