From d8722012afb789f1a2875a0d2ed50bfbae12bb9c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 6 Jul 2016 21:29:40 +0000 Subject: [PATCH] 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 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/transport.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 43b20f2da2..f7904b4a89 100644 --- 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