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

[poy]

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]

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]

@apoydence are you proposing any particular change to ReverseProxy?

[mvdan]

/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]

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

[meirf]

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

[poy]

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

[bradfitz]

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

[gopherbot]

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

[poy]

@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]

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

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

[dmitshur]

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!