-
Notifications
You must be signed in to change notification settings - Fork 52
Allow to configure whether all errors should be logged #68
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
Conversation
lib/plug/cowboy.ex
Outdated
When using a Unix socket, OTP 21+ is required for `Plug.Static` and | ||
`Plug.Conn.send_file/3` to behave correctly. | ||
## Application configuration |
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.
Is that the right place for these docs? Let me know if I should move it.
test/plug/cowboy/translator_test.exs
Outdated
Application.put_env(:plug_cowboy, :log_all, true) | ||
on_exit(fn -> Application.delete_env(:plug_cowboy, :log_all) 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.
The test suite is not async so this should be fine.
lib/plug/cowboy.ex
Outdated
## Application configuration | ||
* `:log_all` - A boolean that determines whether all exceptions are logged by the adapter. |
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.
What if we call it log_exceptions_with_status_code_from
which defaults to 500? This way people can opt-in to 400 but still leave others out.
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.
Sounds good! 👍
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.
What if we stretched this and allowed a range or a list? E.g. if I wanted to skip logging 415.
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.
log_exceptions_with_status_code
that expects a list of ranges work for me.
💚 💙 💜 💛 ❤️ |
The configuration is done via application env, since there is no way to pass options to Logger translators. I called the option
log_all
, but it might be limiting if you'd like to expand this to allow more fine-grained filtering in the future.Closes #67