Skip to content

Commit

Permalink
net/http: make timeout errors match context.DeadlineExceeded
Browse files Browse the repository at this point in the history
When returning an error which implements net.Error and reports
itself as a timeout, also report it as matching context.DeadlineExceeded.
This matches the behavior of timeout errors in the net package
and elsewhere.

Fixes #50856

Change-Id: I2ca911e3677a699af27ba89b1200401baa8b3b1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/567537
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
neild committed Feb 28, 2024
1 parent b6753ba commit 606b8ff
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 15 deletions.
10 changes: 2 additions & 8 deletions src/net/http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -725,10 +725,7 @@ func (c *Client) do(req *Request) (retres *Response, reterr error) {
// c.send() always closes req.Body
reqBodyClosed = true
if !deadline.IsZero() && didTimeout() {
err = &httpError{
err: err.Error() + " (Client.Timeout exceeded while awaiting headers)",
timeout: true,
}
err = &timeoutError{err.Error() + " (Client.Timeout exceeded while awaiting headers)"}
}
return nil, uerr(err)
}
Expand Down Expand Up @@ -968,10 +965,7 @@ func (b *cancelTimerBody) Read(p []byte) (n int, err error) {
return n, err
}
if b.reqDidTimeout() {
err = &httpError{
err: err.Error() + " (Client.Timeout or context cancellation while reading body)",
timeout: true,
}
err = &timeoutError{err.Error() + " (Client.Timeout or context cancellation while reading body)"}
}
return n, err
}
Expand Down
9 changes: 9 additions & 0 deletions src/net/http/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,9 @@ func testClientTimeout(t *testing.T, mode testMode) {
} else if !ne.Timeout() {
t.Errorf("net.Error.Timeout = false; want true")
}
if !errors.Is(err, context.DeadlineExceeded) {
t.Errorf("ReadAll error = %q; expected some context.DeadlineExceeded", err)
}
if got := ne.Error(); !strings.Contains(got, "(Client.Timeout") {
if runtime.GOOS == "windows" && strings.HasPrefix(runtime.GOARCH, "arm") {
testenv.SkipFlaky(t, 43120)
Expand Down Expand Up @@ -1292,6 +1295,9 @@ func testClientTimeout_Headers(t *testing.T, mode testMode) {
if !ne.Timeout() {
t.Error("net.Error.Timeout = false; want true")
}
if !errors.Is(err, context.DeadlineExceeded) {
t.Errorf("ReadAll error = %q; expected some context.DeadlineExceeded", err)
}
if got := ne.Error(); !strings.Contains(got, "Client.Timeout exceeded") {
if runtime.GOOS == "windows" && strings.HasPrefix(runtime.GOARCH, "arm") {
testenv.SkipFlaky(t, 43120)
Expand Down Expand Up @@ -1992,6 +1998,9 @@ func testClientDoCanceledVsTimeout(t *testing.T, mode testMode) {
if g, w := ue.Err, wantErr; g != w {
t.Errorf("url.Error.Err = %v; want %v", g, w)
}
if got := errors.Is(err, context.DeadlineExceeded); got != wantIsTimeout {
t.Errorf("errors.Is(err, context.DeadlineExceeded) = %v, want %v", got, wantIsTimeout)
}
})
}
}
Expand Down
16 changes: 9 additions & 7 deletions src/net/http/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -2557,16 +2557,18 @@ type writeRequest struct {
continueCh <-chan struct{}
}

type httpError struct {
err string
timeout bool
// httpTimeoutError represents a timeout.
// It implements net.Error and wraps context.DeadlineExceeded.
type timeoutError struct {
err string
}

func (e *httpError) Error() string { return e.err }
func (e *httpError) Timeout() bool { return e.timeout }
func (e *httpError) Temporary() bool { return true }
func (e *timeoutError) Error() string { return e.err }
func (e *timeoutError) Timeout() bool { return true }
func (e *timeoutError) Temporary() bool { return true }
func (e *timeoutError) Is(err error) bool { return err == context.DeadlineExceeded }

var errTimeout error = &httpError{err: "net/http: timeout awaiting response headers", timeout: true}
var errTimeout error = &timeoutError{"net/http: timeout awaiting response headers"}

// errRequestCanceled is set to be identical to the one from h2 to facilitate
// testing.
Expand Down

0 comments on commit 606b8ff

Please sign in to comment.