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

x/net/http2: http.Client.Do() sometimes returns "http2: stream closed" if server doesn't consume request body. #20521

Closed
akmistry opened this issue May 29, 2017 · 14 comments
Milestone

Comments

@akmistry
Copy link

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

go version go1.8.3 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/amistry/go-src"
GORACE=""
GOROOT="/home/amistry/go"
GOTOOLDIR="/home/amistry/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build344478198=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

Using http2, create a client request with a large-ish request body. On the server, return a response without consuming the body. Sometimes, the client returns a "http2: stream closed" error. According to http://httpwg.org/specs/rfc7540.html#HttpSequence (last paragraph), the server should be able to return a response without consuming all of the body, and the client should not treat it as an error.

I've made a demo program to demonstrate this error: https://gist.github.com/akmistry/4cbf566a2d99c75e02a6d1771e2b3b97

What did you expect to see?

No error on the client when the server doesn't consume the request body.

What did you see instead?

"http2: stream closed"

@odeke-em
Copy link
Member

/cc @tombergan

@tombergan
Copy link
Contributor

Thanks for the report. Your summary looks accurate. Too late for 1.9, since this is not a regression.

@tombergan tombergan self-assigned this May 30, 2017
@tombergan tombergan modified the milestones: Go1.10, Go1.10Early May 30, 2017
@bradfitz bradfitz added early-in-cycle A change that should be done early in the 3 month dev cycle. and removed early-in-cycle A change that should be done early in the 3 month dev cycle. labels Jun 14, 2017
@bradfitz bradfitz modified the milestones: Go1.10Early, Go1.10 Jun 14, 2017
@ernsheong
Copy link

Is this something worth worrying about?

@akmistry
Copy link
Author

I use this behaviour as a latency optimisation. I don't rely on it, but it would be really nice if it worked consistently.

@tombergan
Copy link
Contributor

Is this something worth worrying about?

It looks to me like a violation of the H2 spec, so yes, it's a bug we should fix.

@rjammala
Copy link

Any workarounds for this bug?

@tombergan tombergan removed the early-in-cycle A change that should be done early in the 3 month dev cycle. label Oct 12, 2017
@tombergan
Copy link
Contributor

Unfortunately no, other than a fix. Is this causing you big problems? If so, I can try to prioritize a fix within a few days.

@rjammala
Copy link

I wrote a web app in Go that will be user facing and hit this issue if the user requests a large body and closes the browser. Very much appreciate it if you can prioritize it. Thanks!

@tombergan
Copy link
Contributor

To be clear, this is a bug in the client, not the server. If your clients are all web browsers, they should not hit this bug. If you're seeing a similar error from the server, please file a separate bug.

@rjammala
Copy link

sorry, will file a separate one.

@akmistry
Copy link
Author

I did a bit of digging and think I've found the root cause of it. I'm not that familiar with Go's http stack so I can't tell if there are other interactions, but I'll clean it up and send it for review.

@tombergan
Copy link
Contributor

@akmistry, awesome, thank you!

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/70510 mentions this issue: net/http2: reset client stream after processing response headers

gopherbot pushed a commit to golang/net that referenced this issue Oct 16, 2017
When a client receives a HEADER frame with a END_STREAM flag,
clientConn.streamByID closes the stream before processing the headers
which may contain a full non-error response. This causes the request's
bodyWriter cancelation to race with the response.

Closing the stream after processing headers allows the response to be
available before the bodyWriter is canceled.

Updates golang/go#20521

Change-Id: I70740e88f75240836e922163a54a6cd89535f7b3
Reviewed-on: https://go-review.googlesource.com/70510
Run-TryBot: Emmanuel Odeke <[email protected]>
Reviewed-by: Tom Bergan <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/71611 mentions this issue: net/http: update bundled http2

c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
When a client receives a HEADER frame with a END_STREAM flag,
clientConn.streamByID closes the stream before processing the headers
which may contain a full non-error response. This causes the request's
bodyWriter cancelation to race with the response.

Closing the stream after processing headers allows the response to be
available before the bodyWriter is canceled.

Updates golang/go#20521

Change-Id: I70740e88f75240836e922163a54a6cd89535f7b3
Reviewed-on: https://go-review.googlesource.com/70510
Run-TryBot: Emmanuel Odeke <[email protected]>
Reviewed-by: Tom Bergan <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
@golang golang locked and limited conversation to collaborators Oct 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants