golang / go

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

net/http: Unexpected "define Request.GetBody to avoid this error" error after setting Request.GetBody #69412

Open Smerom opened 1 month ago

Smerom commented 1 month ago

Go version

go version go1.23.0 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/xxx/Library/Caches/go-build'
GOENV='/Users/xxx/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'

What did you do?

I have not been able to make a stand alone example, but will walk through the exact code path that is triggered.

The offending code is as follows:

go func(client *http.Client){
    dataToPost := []byte("somedata")

    // if we don't set the body here, the first attempt seems send with an empty body even though GetBody is set
    req, err := http.NewRequest(http.MethodPost, "http://127.0.0.1/someapi", bytes.NewBuffer(dataToPost))
    if err != nil {
        log.Fatalln("failed to create request!")
    }
    req.GetBody = func() (io.ReadCloser, error) {
        return io.NopCloser(bytes.NewBuffer(dataToPost)), nil
    }

    res, err := client.Do(req)
    if err != nil {
        log.Fatal(err)
    }
}(someSharedClient)

Which under very particular circumstances, the call to client.Do returns the error http2: Transport: cannot retry err [http2: Transport received Server's graceful shutdown GOAWAY] after Request.Body was written; define Request.GetBody to avoid this error, even though GetBody has been set.

To get here, the request must be redirected followed by a retry-able HTTP/2 error such as GOAWAY. (I believe it also may require a cached HTTP/2 connection from a previous call).

How I managed this was the following setup: Load generating go program (the one getting the error) with at least two go routines sending on the same client <-> nginx minikube ingress <-> some kubernetes service

The code path is as follows:

  1. The first attempt to send the request gets redirected to https (with support for HTTP/2), entering the second iteration of this loop. I did not determine if a redirect from an HTTP/2 endpoint to another HTTP/2 endpoint would cause the same errors.
  2. Since the first loop added the request to the reqs slice here we enter this if statement.
  3. Notice the GetBody function does not get set when the original request is copied here.
  4. The new request enters the http2 round tripper here.
  5. The second attempt to send the request is met by a retry-able error here (in my case it appears to be a GOAWAY sent by nginx after some number of requests as described), and http2shouldRetryRequest is called.
  6. Since GetBody was not copied on the redirect, this section fails to copy the body, and we reach the resulting error here.

What did you see happen?

I received an error telling me to set GetBody to avoid said error, even though GetBody had in fact been set.

What did you expect to see?

I would expect either the GetBody function pointer to be copied on redirect, or the error returned to better reflect the actual error condition.

gabyhelp commented 1 month ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

timothy-king commented 1 month ago

CC @neild, @rsc.

Possibly related flakes #66521, #66477. Some of the issues (#62453, #25009) pointed to by gaby mention the same error message.