Skip to content
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

Jump to bottom

net/http/httputil: ReverseProxy does not do well with streaming #27816

Closed
poy opened this issue Sep 22, 2018 · 11 comments
Closed

net/http/httputil: ReverseProxy does not do well with streaming #27816

poy opened this issue Sep 22, 2018 · 11 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@poy
Copy link
Contributor

poy commented Sep 22, 2018

What version of Go are you using (go version)?

$ go version
go version go1.11 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/pivotal/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/pivotal/workspace/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/44/zr3txl1n45b23kd2kgx1xqq00000gn/T/go-build553003225=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Use the httputil.ReverseProxy to proxy a SSE connection. The internal buffering done inside the proxy seemed to be causing the messages to be very latent and occasionally partial.

What did you expect to see?

I expected the buffering to not cause partial messages or high latency.

What did you see instead?

Messages flushed in full.
@poy
Copy link
Contributor Author

poy commented Sep 22, 2018

I have worked around this by wrapping the given http.ResponseWriter with an object that will invoke Flush() after each successful Write():

apoydence/cf-space-security@bf1df64

@mvdan mvdan changed the title httputil.ReverseProxy does not do well with streaming net/http/httputil: ReverseProxy does not do well with streaming Sep 24, 2018
@mvdan mvdan added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 24, 2018
@mvdan
Copy link
Member

mvdan commented Sep 24, 2018

@apoydence are you proposing any particular change to ReverseProxy?

@mvdan
Copy link
Member

mvdan commented Sep 24, 2018

/cc @bradfitz as per the owners page.

If you want to refactor the API or rewrite the proxy, I presume we could always do that in x/net. Something similar was done for the forward proxy used in net/http: https://go-review.googlesource.com/c/go/+/68091

@poy
Copy link
Contributor Author

poy commented Sep 24, 2018

@mvdan If buffering could be turned off I think it would resolve the issue.

@meirf
Copy link
Contributor

meirf commented Sep 25, 2018

I wonder how common something like this is desired/whether we would want to add a Boolean to the API for disabling buffering.

@poy
Copy link
Contributor Author

poy commented Sep 25, 2018

@meirf I think that would be the best way to preserve existing the API.

@bradfitz
Copy link
Contributor

How about no new API and just do the right thing by default? e.g. https://go-review.googlesource.com/c/go/+/137335

@gopherbot
Copy link

Change https://golang.org/cl/137335 mentions this issue: net/http/httputil: rewrite flushing code, disable on Server-Sent Events

@poy
Copy link
Contributor Author

poy commented Sep 25, 2018

@bradfitz Do other streaming protocols (e.g., websockets #26937) need to be considered? It seems like the linked CL only allows SSE for streaming.

@bradfitz
Copy link
Contributor

@apoydence, there's a TODO in that CL for other cases.

WebSockets is #26937. It doesn't work with ReverseProxy today.

@dmitshur
Copy link
Contributor

For posterity, I have worked around this by setting FlushInterval to 1 second (which obviously adds extra latency, but is okay for non-latency-sensitive uses). CL 137335 is looking incredible and I'm excited to see it get in!

@MrSaints MrSaints mentioned this issue Jul 26, 2019
Open
@golang golang locked and limited conversation to collaborators Oct 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@bradfitz @dmitshur @meirf @mvdan @poy @gopherbot