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

caddyhttp: Always log handled errors at debug level #4584

Merged
merged 1 commit into from Feb 19, 2022

Conversation

francislavoie
Copy link
Member

Context: https://caddy.community/t/optional-reverse-proxy-with-file-system-fallback/15105

There are some situations where error handling is used to intercept 5xx errors emitted by Caddy, like from reverse_proxy, and those errors would still be logged at the ERROR level.

This could get noisy when this is being done "on-purpose". For example, a proxy being turned off half the time and only used as an optimization during development (for hot-module reload or something).

So instead, it probably makes more sense to always log handled errors at DEBUG level, since it was actually handled. If the error falls through unhandled, or a new error is triggered, then it'll still log at ERROR level.

@francislavoie francislavoie added the bug 🐞 Something isn't working label Feb 19, 2022
@francislavoie francislavoie added this to the v2.5.0 milestone Feb 19, 2022
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

LGTM, and makes sense. User is already handling the error, so no need to log it as an error (unless their error handling also has an error).

@mholt mholt merged commit ddbb234 into master Feb 19, 2022
@mholt mholt deleted the quieter-error-handling branch February 19, 2022 22:10
@delfick
Copy link

delfick commented Feb 20, 2022

super cool. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants