-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Only warn when writing to client before LastContent when a body is expected #2014
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
Conversation
| && !shouldAllowPreemptiveResponse(channel) | ||
| && zuulResponse.getInboundRequest().hasBody()) { | ||
| responseBeforeReceivedLastContentCounter.increment(); | ||
| logger.warn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is this warning useful overall?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be a signal of a misbehaving client or origin; will be interesting to see how it behaves in prod given the metric should reduce/provide insight into the cases when it does legitimately happen
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(booleans = {false, true}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
| if (hasBodyChunk) { | ||
| request.bufferBodyContents(new io.netty.handler.codec.http.DefaultHttpContent( | ||
| Unpooled.copiedBuffer("body", java.nio.charset.StandardCharsets.UTF_8))); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't remember if zuul-core tests have leak detection asserts or not, but can you release this if it isn't being released somewhere else already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call - added a dispose call
If a client request is responded by an origin immediately whilst the client is still sending the request (i.e. the origin has enough details in the headers to respond) then warnings are logged.
This PR silences those warnings for cases when the client does not have a body but the origin still responded before the client channel
LastContentwas sent through the pipeline.Example log: