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: deal with half-closed remote in Transport, flaky TestTransportResPattern_c0h1d0t0, TestTransportResPattern_c1h2d0t0 #16029

Open
mikioh opened this Issue Jun 10, 2016 · 4 comments

Comments

Projects
None yet
4 participants
@mikioh
Copy link
Contributor

mikioh commented Jun 10, 2016

See https://build.golang.org/log/6c16a5f66e28811897fa9ff2818873f56a9a128c and https://build.golang.org/log/5ea1f443a67a371b9acdebeee89cc802345ce5fb.

--- FAIL: TestTransportResPattern_c0h1d1t0 (0.00s)
    transport_test.go:664: client: RoundTrip: http2: stream closed
FAIL
FAIL    golang.org/x/net/http2  7.585s

@mikioh mikioh changed the title x/net/http2: flaky TestTransportResPattern_c0h1d0t0 x/net/http2: flaky TestTransportResPattern_c0h1d0t0, TestTransportResPattern_c1h2d0t0 Jun 10, 2016

@mikioh

This comment has been minimized.

Copy link
Contributor

mikioh commented Jun 10, 2016

Also https://build.golang.org/log/11add19306c4c449d8d05e6b3fb05129b0407ed6

--- FAIL: TestTransportResPattern_c1h2d0t0 (0.00s)
    transport_test.go:664: client: RoundTrip: http2: stream closed
FAIL
FAIL    golang.org/x/net/http2  33.617s

@mikioh mikioh added this to the Go1.7Maybe milestone Jun 10, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Jul 13, 2016

This is just a bug in the x/net/http2 test. func testTransportResPattern needs to wait to response until the DATA frame arrives when withData is true.

Removing the Go1.7Maybe label.

@bradfitz bradfitz self-assigned this Jul 13, 2016

@bradfitz bradfitz modified the milestones: Unreleased, Go1.7Maybe Jul 13, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Jul 15, 2016

Actually, it looks like the http2.Transport doesn't handle half-closed situations well.

The server should be able to keep reading the client's request body even after having replied. This is much sketchier in http1, but http2 is clear that it should work.

Somewhat low priority to fix properly, but we can at least fix the failing tests in the meantime.

@bradfitz bradfitz changed the title x/net/http2: flaky TestTransportResPattern_c0h1d0t0, TestTransportResPattern_c1h2d0t0 x/net/http2: deal with half-closed remote in Transport, flaky TestTransportResPattern_c0h1d0t0, TestTransportResPattern_c1h2d0t0 Jul 15, 2016

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Jul 15, 2016

CL https://golang.org/cl/24970 mentions this issue.

gopherbot pushed a commit to golang/net that referenced this issue Jul 15, 2016

http2: fix flaky TestTransportResPattern_* tests
The test's server was replying with the stream closure before reading
the test client's request body. This should in theory be handled
better (and golang/go#16029 will be kept open to track it), but that's not
what this test was trying to test anyway.

Instead, make the test do things in the expected order so it can
continue to test its original subject reliably. (It was trying to test
all the orders of optional & optionally fragmented headers in responses.)

Updates golang/go#16029 (flaky http2 TestTransportResPattern_*)
Updates golang/go#11811 (subrepos need to be green)

Change-Id: I631730fce5dad598120bb2d1ea8895e39255d711
Reviewed-on: https://go-review.googlesource.com/24970
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018

http2: fix flaky TestTransportResPattern_* tests
The test's server was replying with the stream closure before reading
the test client's request body. This should in theory be handled
better (and golang/go#16029 will be kept open to track it), but that's not
what this test was trying to test anyway.

Instead, make the test do things in the expected order so it can
continue to test its original subject reliably. (It was trying to test
all the orders of optional & optionally fragmented headers in responses.)

Updates golang/go#16029 (flaky http2 TestTransportResPattern_*)
Updates golang/go#11811 (subrepos need to be green)

Change-Id: I631730fce5dad598120bb2d1ea8895e39255d711
Reviewed-on: https://go-review.googlesource.com/24970
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment