net/http: deflake TestClientRedirectContext

The test was checking for 1 of 2 possible error values. But based on
goroutine scheduling and the randomness of select statement receive
cases, it was possible for a 3rd type of error to be returned.

This modifies the code (not the test) to make that third type of error
actually the second type of error, which is a nicer error message.

The test is no longer flaky. The flake was very reproducible with a
5ms sleep before the select at the end of Transport.getConn.

Thanks to Github user @jaredborner for debugging.

Fixes #16049

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

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 43b20f2da2f86ad7c0633a40e1059116828a0b52..f7904b4a892b6f5e4b7561b662528c8f960adf19 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -845,10 +845,26 @@ func (t *Transport) getConn(treq *transportRequest, cm connectMethod) (*persistC
        select {
        case v := <-dialc:
                // Our dial finished.
-               if trace != nil && trace.GotConn != nil && v.pc != nil && v.pc.alt == nil {
-                       trace.GotConn(httptrace.GotConnInfo{Conn: v.pc.conn})
+               if v.pc != nil {
+                       if trace != nil && trace.GotConn != nil && v.pc.alt == nil {
+                               trace.GotConn(httptrace.GotConnInfo{Conn: v.pc.conn})
+                       }
+                       return v.pc, nil
                }
-               return v.pc, v.err
+               // Our dial failed. See why to return a nicer error
+               // value.
+               select {
+               case <-req.Cancel:
+               case <-req.Context().Done():
+               case <-cancelc:
+               default:
+                       // It wasn't an error due to cancelation, so
+                       // return the original error message:
+                       return nil, v.err
+               }
+               // It was an error due to cancelation, so prioritize that
+               // error value. (Issue 16049)
+               return nil, errRequestCanceledConn
        case pc := <-idleConnCh:
                // Another request finished first and its net.Conn
                // became available before our dial. Or somebody