okhttp icon indicating copy to clipboard operation
okhttp copied to clipboard

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

Open goweii opened this issue 6 months ago • 3 comments

goweii avatar Jun 14 '25 13:06 goweii

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.

yschimke avatar Jun 14 '25 14:06 yschimke

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

swankjesse avatar Jun 14 '25 18:06 swankjesse

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!

goweii avatar Jun 15 '25 03:06 goweii

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.

yschimke avatar Jun 16 '25 13:06 yschimke

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

https://github.com/square/okhttp/pull/8968

yschimke avatar Aug 03 '25 19:08 yschimke