]> Cypherpunks repositories - gostls13.git/commitdiff
net/http: deflake TestClientTimeout_Headers_h2 on Windows
authorBrad Fitzpatrick <bradfitz@golang.org>
Wed, 14 Dec 2016 18:57:13 +0000 (18:57 +0000)
committerBrad Fitzpatrick <bradfitz@golang.org>
Wed, 14 Dec 2016 19:15:56 +0000 (19:15 +0000)
The client code was using time.Now() (wall time) to determine whether
the cause of a non-nil error meant that a timeout had occured. But on
Windows, the clock used for timers (time.After, time.Sleep, etc) is
much more accurate than the time.Now clock, which doesn't update
often.

But it turns out that as of the recent https://golang.org/cl/32478 we
already have the answer available easily. It just wasn't in scope.

Instead of passing this information along by decorating the errors
(risky this late in Go 1.8, especially with #15935 unresolved), just
passing along the "didTimeout" func internally for now. We can remove
that later in Go 1.9 if we overhaul Transport errors.

Fixes #18287 (I hope)

Change-Id: Icbbfceaf02de6c7ed04fe37afa4ca16374b58f3c
Reviewed-on: https://go-review.googlesource.com/34381
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/net/http/client.go

index 7deccff5876e979936f8636412a9a0a4a2970f8d..9308c5c9686da7c91eab49fbc238682154f40695 100644 (file)
@@ -163,22 +163,23 @@ func refererForURL(lastReq, newReq *url.URL) string {
        return referer
 }
 
-func (c *Client) send(req *Request, deadline time.Time) (*Response, error) {
+// didTimeout is non-nil only if err != nil.
+func (c *Client) send(req *Request, deadline time.Time) (resp *Response, didTimeout func() bool, err error) {
        if c.Jar != nil {
                for _, cookie := range c.Jar.Cookies(req.URL) {
                        req.AddCookie(cookie)
                }
        }
-       resp, err := send(req, c.transport(), deadline)
+       resp, didTimeout, err = send(req, c.transport(), deadline)
        if err != nil {
-               return nil, err
+               return nil, didTimeout, err
        }
        if c.Jar != nil {
                if rc := resp.Cookies(); len(rc) > 0 {
                        c.Jar.SetCookies(req.URL, rc)
                }
        }
-       return resp, nil
+       return resp, nil, nil
 }
 
 func (c *Client) deadline() time.Time {
@@ -197,22 +198,22 @@ func (c *Client) transport() RoundTripper {
 
 // send issues an HTTP request.
 // Caller should close resp.Body when done reading from it.
-func send(ireq *Request, rt RoundTripper, deadline time.Time) (*Response, error) {
+func send(ireq *Request, rt RoundTripper, deadline time.Time) (resp *Response, didTimeout func() bool, err error) {
        req := ireq // req is either the original request, or a modified fork
 
        if rt == nil {
                req.closeBody()
-               return nil, errors.New("http: no Client.Transport or DefaultTransport")
+               return nil, alwaysFalse, errors.New("http: no Client.Transport or DefaultTransport")
        }
 
        if req.URL == nil {
                req.closeBody()
-               return nil, errors.New("http: nil Request.URL")
+               return nil, alwaysFalse, errors.New("http: nil Request.URL")
        }
 
        if req.RequestURI != "" {
                req.closeBody()
-               return nil, errors.New("http: Request.RequestURI can't be set in client requests.")
+               return nil, alwaysFalse, errors.New("http: Request.RequestURI can't be set in client requests.")
        }
 
        // forkReq forks req into a shallow clone of ireq the first
@@ -245,7 +246,7 @@ func send(ireq *Request, rt RoundTripper, deadline time.Time) (*Response, error)
        }
        stopTimer, didTimeout := setRequestCancel(req, rt, deadline)
 
-       resp, err := rt.RoundTrip(req)
+       resp, err = rt.RoundTrip(req)
        if err != nil {
                stopTimer()
                if resp != nil {
@@ -259,7 +260,7 @@ func send(ireq *Request, rt RoundTripper, deadline time.Time) (*Response, error)
                                err = errors.New("http: server gave HTTP response to HTTPS client")
                        }
                }
-               return nil, err
+               return nil, didTimeout, err
        }
        if !deadline.IsZero() {
                resp.Body = &cancelTimerBody{
@@ -268,7 +269,7 @@ func send(ireq *Request, rt RoundTripper, deadline time.Time) (*Response, error)
                        reqDidTimeout: didTimeout,
                }
        }
-       return resp, nil
+       return resp, nil, nil
 }
 
 // setRequestCancel sets the Cancel field of req, if deadline is
@@ -570,8 +571,9 @@ func (c *Client) Do(req *Request) (*Response, error) {
 
                reqs = append(reqs, req)
                var err error
-               if resp, err = c.send(req, deadline); err != nil {
-                       if !deadline.IsZero() && !time.Now().Before(deadline) {
+               var didTimeout func() bool
+               if resp, didTimeout, err = c.send(req, deadline); err != nil {
+                       if !deadline.IsZero() && didTimeout() {
                                err = &httpError{
                                        err:     err.Error() + " (Client.Timeout exceeded while awaiting headers)",
                                        timeout: true,