Skip to content

Output as much human-readable text as possible to the log.#8855

Closed
goweii wants to merge 1 commit into
lysine-dev:masterfrom
goweii:master
Closed

Output as much human-readable text as possible to the log.#8855
goweii wants to merge 1 commit into
lysine-dev:masterfrom
goweii:master

Conversation

@goweii

@goweii goweii commented Jun 14, 2025

Copy link
Copy Markdown

No description provided.

@yschimke

Copy link
Copy Markdown
Collaborator

HttpLoggingInterceptor isn't designed to be everything to everyone. And shouldn't be doing anything magic.

So the first response to PRs changing the logging interceptor is to suggest you copy and paste it and change as you need in your own project.

@swankjesse

Copy link
Copy Markdown
Collaborator

Yeah, I think you’ll be better served by Chucker or similar.
https://github.com/ChuckerTeam/chucker

@goweii

goweii commented Jun 15, 2025

Copy link
Copy Markdown
Author

Hi, thanks again for your feedback and for maintaining OkHttp!

I understand and agree that HttpLoggingInterceptor should not be doing anything "magic". However, I would argue that its current behavior can already be a bit unpredictable, which may surprise developers in real-world usage.

For example:

  • When the body is a File, it logs only the byte count — which is reasonable.
  • But when the body is multipart/form-data, especially when it starts with a few readable bytes, the interceptor attempts to decode and log the entire body as UTF-8 text, including binary file contents. This often leads to:
    • Log pollution with unreadable characters;
    • Performance issues due to excessive log output;
    • Potential crashes or freezes in log viewers (especially on Android).

So while trying to be helpful, the current implementation might mislead developers into thinking all logged content is safe and readable, when it’s not.

If the intended role of HttpLoggingInterceptor is to serve primarily as a simple example or starting point, it might help to make that explicit in the documentation, so developers are aware of its limitations. Perhaps even a name like SimpleHttpLoggingInterceptor would clarify its scope.

That said, I totally understand your preference to avoid adding complexity. My PR was an attempt to make it safer for more common debugging scenarios, without introducing heuristics or smart guessing.

Would it be acceptable to:

  • Add a warning to the documentation about logging binary/multipart bodies?
  • Or consider exposing a small customization hook (e.g. BodyLogger) so developers can override logging behavior without duplicating the whole class?

Thanks again for the thoughtful discussion!

@yschimke

yschimke commented Jun 16, 2025

Copy link
Copy Markdown
Collaborator

I meant not "magic" as in, nothing in the impl requires being part of OkHttp. So everyone is free to write the one they think is ideal.

val count = s.codePointCount(0, s.length)
for (i in 0 until count) {
val codePoint = s.codePointAt(i)
if (codePoint.isProbablyUtf8CodePoint()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This feels like a lot of extra work for these cases. I think for clients sending/receiving possibly binary data, they should just avoid logging it, or write a custom impl.

@yschimke

yschimke commented Aug 3, 2025

Copy link
Copy Markdown
Collaborator

Going to close as not planned.

Yeah, I think you’ll be better served by Chucker or similar. https://github.com/ChuckerTeam/chucker

There is a high bar, so even simple fixes like this one to LoggingEventListener often don't make the cut

#8968

@yschimke yschimke closed this Aug 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants