Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Smerom opened this issue Sep 11, 2024 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@Smerom
Copy link

Smerom commented Sep 11, 2024

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.

@timothy-king timothy-king added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 12, 2024
@timothy-king timothy-king added this to the Backlog milestone Sep 12, 2024
@timothy-king
Copy link
Contributor

CC @neild, @rsc.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants