diff --git a/lib/sentry.ex b/lib/sentry.ex index 2fc40975..1a7b3e2b 100644 --- a/lib/sentry.ex +++ b/lib/sentry.ex @@ -114,6 +114,10 @@ defmodule Sentry do that is, a module implementing the `Sentry.EventFilter` behaviour. This is still supported, but is now deprecated. See `Sentry.EventFilter` for more information. + If the configured filter cannot be called, or its `c:Sentry.EventFilter.exclude_exception?/2` + callback crashes, the exception is excluded and the failure is logged. See the + [*Crashing Callbacks* section](#module-crashing-callbacks) below. + ## Event Callbacks You can configure the `:before_send` and `:after_send_event` options to @@ -149,6 +153,28 @@ defmodule Sentry do If the `before_send` callback returns `nil` or `false`, the event is not reported. + ## Crashing Callbacks + + If a `:before_send`, `:after_send_event`, or `:filter` callback raises, throws, or exits, + Sentry catches the failure and logs it at the `:error` level instead of letting it reach + the code that was reporting the event. The log carries the `:sentry` logger domain, so the + SDK never reports its own callback failure as an event. Other configurable callbacks, such + as `:before_send_log` and `:before_send_metric`, handle their own failures and are not + covered by this section. + + The item being handled is then dropped: + + * A `:before_send` callback that crashes is treated like one that returned `false`. + The event or transaction is not sent, and the capture function returns `:excluded`. + + * A `:filter` module that cannot be called, or whose + `c:Sentry.EventFilter.exclude_exception?/2` crashes, is treated like one that + excluded the exception. `capture_exception/2` returns `:excluded`. + + An `:after_send_event` callback runs once the event has already been sent and its + return value is ignored, so a crash there changes nothing the caller sees: the send + result is still the one the transport produced. + ## Reporting Source Code Sentry supports reporting the source code of (and around) the line that @@ -184,6 +210,7 @@ defmodule Sentry do """ alias Sentry.{ + Callback, CheckIn, Client, ClientError, @@ -273,7 +300,14 @@ defmodule Sentry do event_source = Keyword.get(options, :event_source) {send_opts, create_event_opts} = Options.split_send_event_options(options) - if filter_module.exclude_exception?(exception, event_source) do + exclude? = + Callback.run( + :filter, + fn -> filter_module.exclude_exception?(exception, event_source) end, + true + ) + + if exclude? do :excluded else exception diff --git a/lib/sentry/callback.ex b/lib/sentry/callback.ex new file mode 100644 index 00000000..9ce2b47f --- /dev/null +++ b/lib/sentry/callback.ex @@ -0,0 +1,36 @@ +defmodule Sentry.Callback do + @moduledoc false + + alias Sentry.LoggerUtils + + @type spec() :: (... -> term()) | {module(), atom()} + + @spec run(atom(), (-> result), result) :: result when result: var + def run(name, fun, fallback) when is_atom(name) and is_function(fun, 0) do + fun.() + catch + kind, reason -> + LoggerUtils.error( + "#{inspect(name)} callback failed: " <> + Exception.format(kind, reason, __STACKTRACE__) + ) + + fallback + end + + @spec to_fun(atom(), spec(), [term()]) :: (-> term()) + def to_fun(name, spec, args) when is_atom(name) and is_list(args) do + case spec do + fun when is_function(fun, length(args)) -> + fn -> apply(fun, args) end + + {mod, fun} when is_atom(mod) and is_atom(fun) -> + fn -> apply(mod, fun, args) end + + other -> + raise ArgumentError, + "#{inspect(name)} must be an anonymous function or a {module, function} tuple, " <> + "got: #{inspect(other)}" + end + end +end diff --git a/lib/sentry/client.ex b/lib/sentry/client.ex index c2c249a0..ef07bdab 100644 --- a/lib/sentry/client.ex +++ b/lib/sentry/client.ex @@ -6,6 +6,7 @@ defmodule Sentry.Client do # See https://develop.sentry.dev/sdk/unified-api/#client. alias Sentry.{ + Callback, CheckIn, ClientError, ClientReport, @@ -197,30 +198,21 @@ defmodule Sentry.Client do end end - defp call_before_send(event, function) when is_function(function, 1) do - function.(event) || false + defp call_before_send(event, callback) do + invocation = Callback.to_fun(:before_send, callback, [event]) + Callback.run(:before_send, fn -> invocation.() || false end, false) end - defp call_before_send(event, {mod, fun}) do - apply(mod, fun, [event]) || false - end - - defp call_before_send(_event, other) do - raise ArgumentError, """ - :before_send must be an anonymous function or a {module, function} tuple, got: \ - #{inspect(other)}\ - """ + defp maybe_call_after_send(_event_or_transaction, _result, nil) do + nil end defp maybe_call_after_send(event_or_transaction, result, callback) do - message = ":after_send_event must be an anonymous function or a {module, function} tuple" - - case callback do - function when is_function(function, 2) -> function.(event_or_transaction, result) - {module, function} -> apply(module, function, [event_or_transaction, result]) - nil -> nil - _ -> raise ArgumentError, message - end + Callback.run( + :after_send_event, + Callback.to_fun(:after_send_event, callback, [event_or_transaction, result]), + nil + ) end defp encode_and_send(_event, _result_type = :async, _client, _request_retries) do diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index fa8ccc5e..6c1df271 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -485,7 +485,9 @@ defmodule Sentry.Config do default: Sentry.DefaultEventFilter, doc: """ A module that implements the `Sentry.EventFilter` - behaviour. Defaults to `Sentry.DefaultEventFilter`. See the + behaviour. Defaults to `Sentry.DefaultEventFilter`. If the module cannot be called, + or its callback crashes, the failure is logged at the `:error` level and the + exception is excluded. See the [*Filtering Exceptions* section](#module-filtering-exceptions) below. """ ], @@ -934,8 +936,10 @@ defmodule Sentry.Config do Allows performing operations on the event *before* it is sent as well as filtering out the event altogether. If the callback returns `nil` or `false`, the event is not reported. If it returns an - updated `Sentry.Event`, then the updated event is used instead. See the [*Event Callbacks* - section](#module-event-callbacks) below for more information. + updated `Sentry.Event`, then the updated event is used instead. If the callback crashes, + the failure is logged at the `:error` level and the event is not reported. See the + [*Event Callbacks*](#module-event-callbacks) and [*Crashing + Callbacks*](#module-crashing-callbacks) sections below for more information. `:before_send` is available *since v10.0.0*. Before, it was called `:before_send_event`. """ @@ -954,8 +958,11 @@ defmodule Sentry.Config do doc: """ Callback that is called *after* attempting to send an event. The result of the HTTP call as well as the event will - be passed as arguments. The return value of the callback is not returned. See the - [*Event Callbacks* section](#module-event-callbacks) below for more information. + be passed as arguments. The return value of the callback is not returned. If the + callback crashes, the failure is logged at the `:error` level and the caller still + receives the result of the send. See the [*Event Callbacks*](#module-event-callbacks) + and [*Crashing Callbacks*](#module-crashing-callbacks) sections below for more + information. """ ], before_send_log: [ diff --git a/lib/sentry/event_filter.ex b/lib/sentry/event_filter.ex index bde9eb01..96a67673 100644 --- a/lib/sentry/event_filter.ex +++ b/lib/sentry/event_filter.ex @@ -109,6 +109,11 @@ defmodule Sentry.EventFilter do will have `:plug` as a source and events from `Sentry.LoggerBackend` will have `:logger` as the source. A custom source can also be specified by passing the `:event_source` option to `Sentry.capture_exception/2`. + + If this callback raises, throws, or exits, or if the configured module does not + export it, the failure is logged at the `:error` level and never reaches the code + that was reporting the exception. The exception is excluded, so + `Sentry.capture_exception/2` returns `:excluded`. """ @callback exclude_exception?(exception :: Exception.t(), source :: atom) :: boolean end diff --git a/test/sentry_test.exs b/test/sentry_test.exs index 7092cbbc..83279c7a 100644 --- a/test/sentry_test.exs +++ b/test/sentry_test.exs @@ -16,6 +16,12 @@ defmodule SentryTest do def exclude_exception?(_, _), do: false end + defmodule RaisingFilter do + @behaviour Sentry.EventFilter + + def exclude_exception?(_exception, _source), do: raise("filter is broken") + end + setup do SentryTest.setup_sentry(dedup_events: false) end @@ -163,6 +169,154 @@ defmodule SentryTest do assert :counters.get(request_count, 1) == 2 end + describe "a :before_send callback that crashes" do + test "drops the event and returns :excluded when the callback raises" do + put_test_config(before_send: fn _event -> raise "before_send is broken" end) + + log = + capture_log(fn -> + assert :excluded = Sentry.capture_message("raising before_send", result: :sync) + end) + + assert log =~ ":before_send callback failed" + assert log =~ "before_send is broken" + assert SentryTest.pop_sentry_reports() == [] + end + + test "drops the event and returns :excluded when the callback throws" do + put_test_config(before_send: fn _event -> throw(:before_send_is_broken) end) + + log = + capture_log(fn -> + assert :excluded = Sentry.capture_message("throwing before_send", result: :sync) + end) + + assert log =~ ":before_send callback failed" + assert log =~ "before_send_is_broken" + assert SentryTest.pop_sentry_reports() == [] + end + + test "drops the exception and returns :excluded when the callback exits" do + put_test_config(before_send: fn _event -> exit(:before_send_is_broken) end) + + log = + capture_log(fn -> + assert :excluded = + Sentry.capture_exception(%RuntimeError{message: "oops"}, result: :sync) + end) + + assert log =~ ":before_send callback failed" + assert log =~ "before_send_is_broken" + assert SentryTest.pop_sentry_reports() == [] + end + + test "drops the transaction and returns :excluded" do + transaction = create_transaction(%{transaction: "crashing-before-send-transaction"}) + + log = + capture_log(fn -> + assert :excluded = + Sentry.send_transaction(transaction, + result: :sync, + before_send: fn _transaction -> exit(:before_send_is_broken) end + ) + end) + + assert log =~ ":before_send callback failed" + assert SentryTest.pop_sentry_reports() == [] + end + + test "does not report its own failure back to Sentry" do + test_pid = self() + ref = make_ref() + handler_name = :"sentry_handler_#{System.unique_integer([:positive])}" + + :ok = + :logger.add_handler(handler_name, Sentry.LoggerHandler, %{ + config: %{capture_log_messages: true, level: :debug} + }) + + on_exit(fn -> _ = :logger.remove_handler(handler_name) end) + + put_test_config( + before_send: fn _event -> + send(test_pid, {ref, :called}) + raise "before_send is broken" + end + ) + + capture_log(fn -> + assert :excluded = Sentry.capture_message("self-reporting before_send", result: :sync) + end) + + assert_received {^ref, :called} + refute_received {^ref, :called} + end + end + + describe "an :after_send_event callback that crashes" do + test "still returns the successful send result for an event" do + put_test_config(after_send_event: fn _event, _result -> raise "after_send is broken" end) + + log = + capture_log(fn -> + assert {:ok, _id} = Sentry.capture_message("raising after_send", result: :sync) + end) + + assert log =~ ":after_send_event callback failed" + assert log =~ "after_send is broken" + + assert_sentry_report(:event, message: %{formatted: "raising after_send"}) + end + + test "still returns the successful send result for a transaction" do + transaction = create_transaction(%{transaction: "crashing-after-send-transaction"}) + + log = + capture_log(fn -> + assert {:ok, _id} = + Sentry.send_transaction(transaction, + result: :sync, + after_send_event: fn _transaction, _result -> exit(:after_send_is_broken) end + ) + end) + + assert log =~ ":after_send_event callback failed" + assert log =~ "after_send_is_broken" + + assert_sentry_report(:transaction, transaction: "crashing-after-send-transaction") + end + end + + describe "a :filter callback that crashes" do + test "drops the exception and returns :excluded when the filter raises" do + put_test_config(filter: RaisingFilter) + + log = + capture_log(fn -> + assert :excluded = + Sentry.capture_exception(%RuntimeError{message: "oops"}, result: :sync) + end) + + assert log =~ ":filter callback failed" + assert log =~ "filter is broken" + assert SentryTest.pop_sentry_reports() == [] + end + + test "drops the exception and returns :excluded when the filter cannot be called" do + put_test_config(filter: __MODULE__.MissingFilter) + + log = + capture_log(fn -> + assert :excluded = + Sentry.capture_exception(%RuntimeError{message: "oops"}, result: :sync) + end) + + assert log =~ ":filter callback failed" + assert SentryTest.pop_sentry_reports() == [] + end + end + describe "send_check_in/1" do test "posts a check-in with all the explicit arguments", %{bypass: bypass} do put_test_config(environment_name: "test", release: "1.3.2")