Don't try to parse error responses with no body#4307
Don't try to parse error responses with no body#4307milosgajdos merged 1 commit intodistribution:mainfrom
Conversation
HEAD requests for instance return no body while still having all the relevant Content-Type headers set, causing unnecessary parsing errors. This skips further parsing for all requests that don't have any body to begin with. Signed-off-by: Markus Thömmes <[email protected]>
|
If accepted, a backport to 2.8 would be neat 😅 |
milosgajdos
left a comment
There was a problem hiding this comment.
Reluctantly approving as
- we explicitly warn in the README the client is deprecated which is the reason why it's been moved to the
internalpackage - we will be removing the client code in some future release TBD
If this code is somewhat important to you I'd strongly encourage you to consider replacing this client code with something that's maintained.
|
@milosgajdos I'm not well versed in this codebase rn, I was purely moving off of error messages I've seen in the deployed product when being run in mirror mode. Would that not use this code-path and am I on the wrong track there? 🤔 Edit: I've poked around and yeah, it seems like the client is in-use in the proxy code, so I guess from that PoV, it's still valid? Or are we looking at replacing it there as well? |
We've not discussed that at large -- but your observation is spot on; the only reason why the client code has not been removed completely is because it's used on the proxy. Frankly, the proxy is in the state it is and would need a significant investment of effort to make it work better than it is now. But for the time being it'll remain in this codebase |
HEAD requests for instance return no body while still having all the relevant Content-Type headers set, causing unnecessary parsing errors. This skips further parsing for all requests that don't have any body to begin with.
Augments #4013.