golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.31k stars 17.58k forks source link

net/http: performs unexpected retry on GET requests under specific conditions #48438

Open JonathanArns opened 3 years ago

JonathanArns commented 3 years ago

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

$ go version go1.16.6 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jonathan/.cache/go-build"
GOENV="/home/jonathan/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/jonathan/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/jonathan/go"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/usr/lib/golang"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/golang/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.6"
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-build1539237600=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/w-oW1oYN0ZN

The above playground provides a small example of http.Get() performing a retry under specific circumstances. This does not happen with POST requests, which can be demonstrated by setting post to true in the playground.

What did you expect to see?

No automatic retries. Aka seeing "HELLO WORLD!" printed exactly 3 times in all cases.

What did you see instead?

"HELLO WORLD!" printed 4 times, due to the retry of a failed request. The retry happens only if there has been a successful request directly before it.

JonathanArns commented 3 years ago

I've realized now, that the playground that I provided often does not print the full output of the program for some reason. You have the full output only if it prints program exited. in grey font at the bottom. Either just run the playground again if you don't see it, or copy the code and run it locally on your machine.

Also, let me explain a bit further in words what my problem is here. I am sending requests with the default HTTP client to a net/http server. The receiving handler func panics to cause a failure of the requests. Now, as far as I can tell, the client is not supposed to send a retry by itself, but it does, if and only if the previous request (possibly only to the same host, I did not test otherwise) was sucessfull. If there was no previous request, or the previous request failed, this does not happen. Maybe this is intended behavior, but I could not find it documented anywhere and it does seem awfully inconsistent to me.

seankhliao commented 3 years ago

I believe this is documented: https://pkg.go.dev/net/http#Transport

Transport only retries a request upon encountering a network error if the request is idempotent and either has no body or has its Request.GetBody defined. HTTP requests are considered idempotent if they have HTTP methods GET, HEAD, OPTIONS, or TRACE; or if their Header map contains an "Idempotency-Key" or "X-Idempotency-Key" entry. If the idempotency key value is a zero-length slice, the request is treated as idempotent but the header is not sent on the wire.

JonathanArns commented 3 years ago

Thanks for pointing that out, I did not see that. Still, I think this documentation does not exactly match what happens. According to the documentation, in my example, I would expect more requests to retry than just one. The example has two identical requests which encounter the same panicing handler (aka. network error?). However only one of those requests is retried. The one which had a successful request come right beforehand.

seankhliao commented 3 years ago

See #4677 and https://go-review.googlesource.com/c/go/+/3210/9//COMMIT_MSG

JonathanArns commented 3 years ago

Okay cool, so this is intended behavior :smile:

Should we improve the documentation to reflect the real behavior more closely then?

seankhliao commented 3 years ago

Feel free to send a PR with any suggested clarifications