net/http: Client.Do documentation is incorrect about *url.Error.Timeout

[perillo]

What version of Go are you using (go version)?

$ go version
go version go1.16.4 linux/amd64

Does this issue reproduce with the latest release?

Yes.
It also reproduces with go1.17 devel.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE="*.local"
GOMODCACHE="/home/manlio/.local/lib/go/pkg/mod"
GONOPROXY=""
GONOSUMDB="*.local"
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.4"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1653706074=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.16.4 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.16.4
uname -sr: Linux 5.12.5-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) release release version 2.33.
gdb --version: GNU gdb (GDB) 10.2

What did you do?

https://play.golang.org/p/YQH5oWtQWjC

What did you expect to see?

timed out: true

As documented by https://golang.org/pkg/net/http/#Client.Do:

Any returned error will be of type *url.Error. The url.Error value's Timeout method will report true if request timed out or was canceled.

What did you see instead?

timed out: false

[mknyszek]

If this is just a documentation fix, this can land any time. I haven't read deeply into this, but maybe you're up for contributing the doc fix? :) See https://golang.org/doc/contribute.

CC @neild

[perillo]

Looking at the source code, the implementation does not check if Client.send returns context.Canceled. So it is either the documentation incorrect or the implementation incorrect.

Also, I think that the TODO in https://go.googlesource.com/go/+/refs/heads/master/src/net/http/client.go#732 is wrong. The send function seems to returns err == context.Canceled and didTimeout() == false when the context is canceled.

[neild]

I was going to say that we should drop the mention of the Timeout method in favor of errors.Is(err, context.DeadlineExceeded) or errors.Is(err, os.DeadlineExceeded), but those are apparently different errors and I'm not 100% certain that net/http consistently returns one or the other.

We should make sure that it's possible to test for timeouts with a single sentinel error and document that.

[perillo]

According to #33545, the problem is with the documentation. https://golang.org/cl/200798 also locked-in the current behavior with a test.

The problem is that #33545 only fixed the documentation of the Client.Get method, forgetting(?) the Get function and Client.Do method.

[gopherbot]

Change https://golang.org/cl/323089 mentions this issue: net/http: fix Get and Client.Get docs on request cancelation