net/http: deflake request-not-written path

When we receive an error writing the first byte of a request to a
reused connection, we retry the request on a new connection. Remove
a flaky path which could cause the request to not be retried if
persistConn.roundTrip reads the error caused by closing the connection
before it reads the write error that caused the connection to be
closed.

Fixes #30938.

Change-Id: Iafd99e3239cd9dba4a4c9ddd950a877ca9815e59
Reviewed-on: https://go-review.googlesource.com/c/go/+/379554
Trust: Bryan Mills <bcmills@google.com>
Trust: Damien Neil <dneil@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 5fe3e6ebb49ac2654b72f247ddf2e493d2df5088..e41b20a15ba46656d4656e7e0152996f72bee113 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -606,6 +606,9 @@ func (t *Transport) roundTrip(req *Request) (*Response, error) {
                } else if !pconn.shouldRetryRequest(req, err) {
                        // Issue 16465: return underlying net.Conn.Read error from peek,
                        // as we've historically done.
+                       if e, ok := err.(nothingWrittenError); ok {
+                               err = e.error
+                       }
                        if e, ok := err.(transportReadFromServerError); ok {
                                err = e.err
                        }
@@ -2032,6 +2035,9 @@ func (pc *persistConn) mapRoundTripError(req *transportRequest, startBytesWritte
        }
 
        if _, ok := err.(transportReadFromServerError); ok {
+               if pc.nwrite == startBytesWritten {
+                       return nothingWrittenError{err}
+               }
                // Don't decorate
                return err
        }

diff --git a/src/net/http/transport_internal_test.go b/src/net/http/transport_internal_test.go
index 1cce27235dfdc3ce73c7188c609187d61ebb6bb2..2ed637e9f036cac04101c689803d26130b484827 100644 (file)
--- a/src/net/http/transport_internal_test.go
+++ b/src/net/http/transport_internal_test.go
@@ -52,8 +52,8 @@ func TestTransportPersistConnReadLoopEOF(t *testing.T) {
        conn.Close() // simulate the server hanging up on the client
 
        _, err = pc.roundTrip(treq)
-       if !isTransportReadFromServerError(err) && err != errServerClosedIdle {
-               t.Errorf("roundTrip = %#v, %v; want errServerClosedIdle or transportReadFromServerError", err, err)
+       if !isNothingWrittenError(err) && !isTransportReadFromServerError(err) && err != errServerClosedIdle {
+               t.Errorf("roundTrip = %#v, %v; want errServerClosedIdle, transportReadFromServerError, or nothingWrittenError", err, err)
        }
 
        <-pc.closech
@@ -63,6 +63,11 @@ func TestTransportPersistConnReadLoopEOF(t *testing.T) {
        }
 }
 
+func isNothingWrittenError(err error) bool {
+       _, ok := err.(nothingWrittenError)
+       return ok
+}
+
 func isTransportReadFromServerError(err error) bool {
        _, ok := err.(transportReadFromServerError)
        return ok