Commit 901005e8 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

net/http: deflake TestClientTimeout_Headers_h2 on Windows

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: 's avatarIan Lance Taylor <iant@golang.org>
Reviewed-by: 's avatarEmmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 1da1e432
...@@ -163,22 +163,23 @@ func refererForURL(lastReq, newReq *url.URL) string { ...@@ -163,22 +163,23 @@ func refererForURL(lastReq, newReq *url.URL) string {
return referer 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 { if c.Jar != nil {
for _, cookie := range c.Jar.Cookies(req.URL) { for _, cookie := range c.Jar.Cookies(req.URL) {
req.AddCookie(cookie) req.AddCookie(cookie)
} }
} }
resp, err := send(req, c.transport(), deadline) resp, didTimeout, err = send(req, c.transport(), deadline)
if err != nil { if err != nil {
return nil, err return nil, didTimeout, err
} }
if c.Jar != nil { if c.Jar != nil {
if rc := resp.Cookies(); len(rc) > 0 { if rc := resp.Cookies(); len(rc) > 0 {
c.Jar.SetCookies(req.URL, rc) c.Jar.SetCookies(req.URL, rc)
} }
} }
return resp, nil return resp, nil, nil
} }
func (c *Client) deadline() time.Time { func (c *Client) deadline() time.Time {
...@@ -197,22 +198,22 @@ func (c *Client) transport() RoundTripper { ...@@ -197,22 +198,22 @@ func (c *Client) transport() RoundTripper {
// send issues an HTTP request. // send issues an HTTP request.
// Caller should close resp.Body when done reading from it. // 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 req := ireq // req is either the original request, or a modified fork
if rt == nil { if rt == nil {
req.closeBody() 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 { if req.URL == nil {
req.closeBody() req.closeBody()
return nil, errors.New("http: nil Request.URL") return nil, alwaysFalse, errors.New("http: nil Request.URL")
} }
if req.RequestURI != "" { if req.RequestURI != "" {
req.closeBody() 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 // 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) ...@@ -245,7 +246,7 @@ func send(ireq *Request, rt RoundTripper, deadline time.Time) (*Response, error)
} }
stopTimer, didTimeout := setRequestCancel(req, rt, deadline) stopTimer, didTimeout := setRequestCancel(req, rt, deadline)
resp, err := rt.RoundTrip(req) resp, err = rt.RoundTrip(req)
if err != nil { if err != nil {
stopTimer() stopTimer()
if resp != nil { if resp != nil {
...@@ -259,7 +260,7 @@ func send(ireq *Request, rt RoundTripper, deadline time.Time) (*Response, error) ...@@ -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") err = errors.New("http: server gave HTTP response to HTTPS client")
} }
} }
return nil, err return nil, didTimeout, err
} }
if !deadline.IsZero() { if !deadline.IsZero() {
resp.Body = &cancelTimerBody{ resp.Body = &cancelTimerBody{
...@@ -268,7 +269,7 @@ func send(ireq *Request, rt RoundTripper, deadline time.Time) (*Response, error) ...@@ -268,7 +269,7 @@ func send(ireq *Request, rt RoundTripper, deadline time.Time) (*Response, error)
reqDidTimeout: didTimeout, reqDidTimeout: didTimeout,
} }
} }
return resp, nil return resp, nil, nil
} }
// setRequestCancel sets the Cancel field of req, if deadline is // setRequestCancel sets the Cancel field of req, if deadline is
...@@ -570,8 +571,9 @@ func (c *Client) Do(req *Request) (*Response, error) { ...@@ -570,8 +571,9 @@ func (c *Client) Do(req *Request) (*Response, error) {
reqs = append(reqs, req) reqs = append(reqs, req)
var err error var err error
if resp, err = c.send(req, deadline); err != nil { var didTimeout func() bool
if !deadline.IsZero() && !time.Now().Before(deadline) { if resp, didTimeout, err = c.send(req, deadline); err != nil {
if !deadline.IsZero() && didTimeout() {
err = &httpError{ err = &httpError{
err: err.Error() + " (Client.Timeout exceeded while awaiting headers)", err: err.Error() + " (Client.Timeout exceeded while awaiting headers)",
timeout: true, timeout: true,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment