-
-
Notifications
You must be signed in to change notification settings - Fork 210
Support for filtering logs via before_send_log
#973
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
base: master
Are you sure you want to change the base?
Conversation
bcb816d to
094f947
Compare
ee5ba83 to
741163f
Compare
094f947 to
9e1f449
Compare
741163f to
4ffe87b
Compare
9e1f449 to
3258a25
Compare
4ffe87b to
542b9dc
Compare
7c929ff to
515fa87
Compare
542b9dc to
245b462
Compare
245b462 to
8ff9aa2
Compare
| defp call_before_send_log(log_event, function) when is_function(function, 1) do | ||
| function.(log_event) | ||
| end | ||
|
|
||
| defp call_before_send_log(log_event, {mod, fun}) do | ||
| apply(mod, fun, [log_event]) | ||
| end |
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.
Bug: An unhandled exception in a before_send_log callback will crash the background task, causing the entire batch of logs to be silently lost.
Severity: HIGH
Suggested Fix
Wrap the execution of the user-provided callback within a try/rescue block inside the call_before_send_log function. If an exception is caught, it should be logged appropriately without crashing the task, allowing the remaining logs in the batch to be processed and sent.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: lib/sentry/client.ex#L183-L189
Potential issue: The `before_send_log` callback is executed within a background task
spawned by `Task.Supervisor.start_child()`. The function `call_before_send_log` does not
include any exception handling, such as a `try/rescue` block. If a user-provided
callback raises an exception, it will cause this background task to crash. Because the
task crashes before the logs are sent via an HTTP request, the entire batch of logs
being processed will be silently lost. The application will not be notified of the
failure, and the error will only be visible in Elixir's task error logs, making it a
difficult issue to debug in production.
Did we get this right? 👍 / 👎 to inform future reviews.
I'll update changelog for the entire feature when preparing changelog for
12.0.0.#skip-changelog
Closes #909