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
net/http/httputil: ReverseProxy flush immediately additional case #41642
Comments
FYI, Caddy's approach. |
Doing that TODO from line 409 seems reasonable. /cc @neild |
Sounds good, I can take care of this. I'll make a PR in the next days. |
Change https://golang.org/cl/260637 mentions this issue: |
cc @odeke-em as possible reviewer. |
Thank you for the tag @networkimprov :) I've provided my review. |
@dmitshur should we perhaps backport this change? |
This was a TODO comment added 2 years ago that was only reported 2 weeks ago. That suggests to me it's not a very serious issue, and fine to leave for 1.16. |
While using
ReverseProxy
in a stream-based HTTP API I got hit by the default value ofFlushInterval
. This API used aTransfer-Encoding: chunked
andContent-Length: -1
for the intention of a stream-based response.The default value of
FlushInterval
isn't latency based nor flush immediately which seems fine in general, but in this particular case, the end client (talking with the reverse-proxy) wasn't seeing any stream messages since they were being buffered until the HTTP response finished, which in this case was never since only gets canceled by the client.Looking at
ReverseProxy
implementation the solution was simple, just useFlushInterval = -1
to signal to flush immediately. Interestingly, the exact same situation was in a TODO comment which from my point of view sounds like it might be a good idea to implement.I'm opening the discussion to see if makes sense. If that's the case, I can propose a PR if we decide is a good idea to also flush immediately on
Content-Length: -1
or other decided case in addition to the current exceptionContent-Type: "text/event-stream"
.The text was updated successfully, but these errors were encountered: