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

WARN is too severe for the "Previous digest authentication with same nonce failed, returning null" #88

Closed
ylexus opened this issue Oct 19, 2024 · 4 comments

Comments

@ylexus
Copy link

ylexus commented Oct 19, 2024

These statements are logged at the WARN level. My app operates a zero noise in logs approach and sends alert when WARN or ERROR are logged. Both typically mean something needs an attention of a human. However, these two do not always happen when password is wrong, despite the comment. I can sometimes observe them during normal operation. Also, these are very low level technical messages, and are logged on a critical path, i.e. with a high volume of traffic has a potential to flood the logs. They are not really problems. It feels like these should be logged at the DEBUG level instead. Can this be done please if you agree?

BasicAuthenticator:

Platform.get().log( "Previous basic authentication failed, returning null", Platform.WARN,null);

DigestAuthenticator

// prevent infinite loops when the password is wrong
Platform.get().log("Previous digest authentication with same nonce failed, returning null", Platform.WARN,
                    null);
@ylexus
Copy link
Author

ylexus commented Oct 19, 2024

For that matter, the 2nd argument also applies to this:

Platform.get().log("Cached authentication expired. Sending a new request.", Platform.INFO, null);

These are per-request messages, in the presence of large number of requests there's a log spam risk.

ylexus pushed a commit to ylexus/okhttp-digest that referenced this issue Oct 19, 2024
@ylexus
Copy link
Author

ylexus commented Oct 19, 2024

Seems like OkHttp logging mechanism used only allows INFO or WARN. For your convenience, here's a PR: #89

rburgst added a commit that referenced this issue Oct 20, 2024
#88 reduce logging level of auth failures from WARN to INFO
@rburgst
Copy link
Owner

rburgst commented Oct 20, 2024

released 3.1.1

@rburgst rburgst closed this as completed Oct 20, 2024
@rburgst
Copy link
Owner

rburgst commented Oct 20, 2024

thanks a lot for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants