Document that Logger reflects Feign's Request, not the wire request - #3544
Document that Logger reflects Feign's Request, not the wire request#3544ChrisJr404 wants to merge 1 commit into
Conversation
velo
left a comment
There was a problem hiding this comment.
Thanks for adding this — the clarification about Logger reflecting Feign's Request rather than the wire request is a genuinely useful thing to document, and I confirmed the core claim is accurate (Logger.logRequest is called in SynchronousMethodHandler before client.execute, so the request logged really can differ from what goes over the wire).
Two things before merging:
-
The doc points at
{@link Client.Default}, but that class is@Deprecated(see its javadoc: "useDefaultClientinstead").Feign.Builderactually defaults tonew DefaultClient(...), and the restricted-header-dropping behavior you're describing lives inDefaultClient(e.g. theContent-Length: 0handling around line 228), not in the deprecated wrapper. Please repoint the{@link}toDefaultClient. -
Consider moving the detailed JDK-specific rationale (the
HttpURLConnectionrestricted-headers behavior,sun.net.http.allowRestrictedHeaders, specific header names) ontoDefaultClient's own class javadoc, with a short pointer fromLogger's doc ("see{@link DefaultClient}for a concrete example"). That keeps the detail next to the code that actually causes it, where someone touchingDefaultClient's header handling would be looking.
Happy to merge once the class reference is fixed.
Adds a note to the Logger class javadoc explaining that request logging reflects the Request that Feign builds, which can differ from what the underlying Client actually sends. The default HttpURLConnection based Client drops headers the JDK treats as restricted, so a header can show up in the log without being on the wire. Closes #2682.