-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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: TestServerReadAfterWriteHeader100Continue/h1 failures #67382
Comments
Found new dashboard test flakes for:
2024-05-15 13:27 gotip-linux-arm go@0222a028 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-15 13:52 gotip-darwin-arm64_11 go@6ccd8e4c net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-15 15:02 gotip-linux-loong64 go@c3c97ad1 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-15 20:03 gotip-linux-arm64-boringcrypto go@3d807615 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-15 21:33 gotip-linux-arm go@3c8f9256 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-16 01:32 gotip-linux-amd64-clang15 go@1ffc2967 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
CC @neild. |
Found new dashboard test flakes for:
2024-05-15 14:39 gotip-linux-ppc64le_power10 go@80565407 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-15 15:02 gotip-linux-ppc64_power10 go@c3c97ad1 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-15 15:44 gotip-linux-ppc64_power10 go@18d0e6a1 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-15 21:44 gotip-linux-ppc64_power10 go@7730e5b7 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-16 15:19 gotip-linux-arm go@a22cb5ca net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-16 15:53 gotip-linux-arm64-boringcrypto go@2b3d98f2 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-16 15:53 gotip-linux-ppc64le_power9 go@2b3d98f2 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-16 15:56 gotip-linux-ppc64_power10 go@33d725e5 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-16 16:34 gotip-linux-ppc64_power8 go@6a050557 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-16 16:44 gotip-linux-ppc64le_power10 go@4c1cc1c9 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-16 17:32 gotip-linux-ppc64le_power8 go@18104621 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-16 18:37 gotip-linux-arm go@06b45781 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-16 18:42 gotip-linux-ppc64_power10 go@a523152e net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-16 18:56 gotip-linux-loong64 go@26ed0528 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-16 15:26 gotip-linux-arm go@643ad42d net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-17 01:23 gotip-linux-ppc64le_power10 go@2f642683 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-17 15:28 gotip-linux-arm go@d11e4172 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-17 15:58 gotip-linux-loong64 go@a61729b8 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-17 18:28 gotip-linux-ppc64le_power10 go@e95f6af0 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Adding a few investigative notes. As visible by comparing here and here, only the h1 subtest is flaky. h2 is consistently passing. It seems to reproduce locally but requires a quite a few attempts:
|
@neild Fixing the panic uncovered a lurking client side issue with 100 Continues.
The spec is not very explicit but it does seem that the body is still supposed to be sent.
A question is whether there is a case when the request body should not be sent. One could look for a |
Found new dashboard test flakes for:
2024-05-20 18:05 gotip-linux-ppc64_power8 go@f726c8d0 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-20 19:31 gotip-windows-386 go@d51fc260 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
@dmitshur, @fraenkel, thanks for the analysis, it helped greatly in pointing me at the source of the problem. TestServerReadAfterWriteHeader100Continue is flaky because it doesn't set The test's handler sends a 200 response, reads the request body, and then writes the response body. (The 200 response gets an automatically-added I'm not certain whether this handler is behaving reasonably or not. Sending response headers and Maybe the client should just always send the body on receiving response headers. Or maybe we should close the server handler's request body after it sends headers, so it gets an error if it tries to read from it. |
@neild I too was trying to figure out which side is incorrect. Both sides seem to have issues. The behavior needs to work across clients and servers. |
In the HTTP/1 case, receiving response headers disables the client's 100-continue timer, leading to this deadlock. In the HTTP/2 case, receiving response headers does not disable the 100-continue timer. Setting a longer I'm not sure if that's all behaving as we want or not. We probably want HTTP/1 and HTTP/2 to behave similarly, or at least for any divergence to be intentional. |
This might clarify some things:
curl had this discussion https://curl.se/mail/lib-2004-08/0002.html Mapping similar behavior to http/2 is most likely going to cause a GOAWAY. |
This test is not properly testing the behavior that it wants to exercise. It configures a client to send a request with a The flaky test failures occur in when the server's response arrives before the client's (0-duration) timeout expires. Setting even a 1 millisecond So: The test is wrong. But the code under test is also wrong. Fixing the test converts the flaky failure into a non-flaky failure. The correct fix for the implementation of 100-continue is not immediately obvious. I'm going to say that this issue (#67382) is about the flakiness. I'll make a CL that I will file a new issue for the underlying bug in 100-continue handling. |
Change https://go.dev/cl/587155 mentions this issue: |
Found new dashboard test flakes for:
2024-05-20 20:23 gotip-linux-ppc64_power10 go@1028d973 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-20 20:23 gotip-darwin-arm64_13 go@1028d973 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-20 20:23 gotip-linux-arm go@1028d973 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-21 14:38 gotip-linux-386-softfloat go@1b9dc3e1 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-21 14:38 gotip-linux-loong64 go@66cc2b7c net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-21 15:09 gotip-linux-ppc64le_power10 go@dbe2e757 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-21 16:24 gotip-linux-arm go@d68d4854 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-21 17:17 gotip-linux-amd64-goamd64v3 go@d881ed63 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-21 17:17 gotip-linux-ppc64_power10 go@ba1c5b2c net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
2024-05-21 17:17 gotip-linux-ppc64le_power10 go@ba1c5b2c net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-21 20:52 gotip-linux-arm go@64386585 net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Found new dashboard test flakes for:
2024-05-21 17:17 gotip-linux-arm go@ba1c5b2c net/http.TestServerReadAfterWriteHeader100Continue/h1 [ABORT] (log)
|
Issue created automatically to collect these failures.
Example (log):
— watchflakes
The text was updated successfully, but these errors were encountered: