net/http: don't wrap request cancellation errors in timeouts

Based on Filippo Valsorda's https://golang.org/cl/24230

Fixes #16094

Change-Id: Ie39b0834e220f0a0f4fbfb3bbb271e70837718c3
Reviewed-on: https://go-review.googlesource.com/32478
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit is contained in:
Emmanuel Odeke 2016-11-01 00:44:48 -07:00 committed by Brad Fitzpatrick
parent 47bdae9422
commit 7448eb4172
2 changed files with 63 additions and 32 deletions

View file

@ -1185,10 +1185,12 @@ func TestClientTimeout_h1(t *testing.T) { testClientTimeout(t, h1Mode) }
func TestClientTimeout_h2(t *testing.T) { testClientTimeout(t, h2Mode) }
func testClientTimeout(t *testing.T, h2 bool) {
if testing.Short() {
t.Skip("skipping in short mode")
}
setParallel(t)
defer afterTest(t)
testDone := make(chan struct{})
const timeout = 100 * time.Millisecond
sawRoot := make(chan bool, 1)
sawSlow := make(chan bool, 1)
cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
@ -1201,16 +1203,22 @@ func testClientTimeout(t *testing.T, h2 bool) {
w.Write([]byte("Hello"))
w.(Flusher).Flush()
sawSlow <- true
time.Sleep(2 * time.Second)
select {
case <-testDone:
case <-time.After(timeout * 10):
}
return
}
}))
defer cst.close()
const timeout = 500 * time.Millisecond
defer close(testDone)
cst.c.Timeout = timeout
res, err := cst.c.Get(cst.ts.URL)
if err != nil {
if strings.Contains(err.Error(), "Client.Timeout") {
t.Skip("host too slow to get fast resource in 100ms")
}
t.Fatal(err)
}
@ -1260,11 +1268,9 @@ func TestClientTimeout_Headers_h2(t *testing.T) { testClientTimeout_Headers(t, h
// Client.Timeout firing before getting to the body
func testClientTimeout_Headers(t *testing.T, h2 bool) {
if testing.Short() {
t.Skip("skipping in short mode")
}
setParallel(t)
defer afterTest(t)
donec := make(chan bool)
donec := make(chan bool, 1)
cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
<-donec
}))
@ -1278,9 +1284,10 @@ func testClientTimeout_Headers(t *testing.T, h2 bool) {
// doesn't know this, so synchronize explicitly.
defer func() { donec <- true }()
cst.c.Timeout = 500 * time.Millisecond
_, err := cst.c.Get(cst.ts.URL)
cst.c.Timeout = 5 * time.Millisecond
res, err := cst.c.Get(cst.ts.URL)
if err == nil {
res.Body.Close()
t.Fatal("got response from Get; expected error")
}
if _, ok := err.(*url.Error); !ok {
@ -1298,6 +1305,36 @@ func testClientTimeout_Headers(t *testing.T, h2 bool) {
}
}
// Issue 16094: if Client.Timeout is set but not hit, a Timeout error shouldn't be
// returned.
func TestClientTimeoutCancel(t *testing.T) {
setParallel(t)
defer afterTest(t)
testDone := make(chan struct{})
ctx, cancel := context.WithCancel(context.Background())
cst := newClientServerTest(t, h1Mode, HandlerFunc(func(w ResponseWriter, r *Request) {
w.(Flusher).Flush()
<-testDone
}))
defer cst.close()
defer close(testDone)
cst.c.Timeout = 1 * time.Hour
req, _ := NewRequest("GET", cst.ts.URL, nil)
req.Cancel = ctx.Done()
res, err := cst.c.Do(req)
if err != nil {
t.Fatal(err)
}
cancel()
_, err = io.Copy(ioutil.Discard, res.Body)
if err != ExportErrRequestCanceled {
t.Fatal("error = %v; want errRequestCanceled")
}
}
func TestClientRedirectEatsBody_h1(t *testing.T) { testClientRedirectEatsBody(t, h1Mode) }
func TestClientRedirectEatsBody_h2(t *testing.T) { testClientRedirectEatsBody(t, h2Mode) }
func testClientRedirectEatsBody(t *testing.T, h2 bool) {
@ -1317,10 +1354,10 @@ func testClientRedirectEatsBody(t *testing.T, h2 bool) {
t.Fatal(err)
}
_, err = ioutil.ReadAll(res.Body)
res.Body.Close()
if err != nil {
t.Fatal(err)
}
res.Body.Close()
var first string
select {