[Bug]: Wrapped body in http request prevents rewinding the body.

[Noroth]

Component

Instrumentation: otelhttp

Describe the issue you're facing

Hey Otel team,

I'm actually not 100% sure if this is expected or not, but I still wanted to report it. I also couldn't find another issue that seemed related.

Analysis

We are using a retrying roundtripper that uses the otelhttp roundtripper. I'm currently upgrading from quite an old OTEL version otelhttp@v0.46.1 to v0.67.0 and I noticed that our tests were suddenly failing.

I took some time to investigate and I think I found the issue:

The roundtripper creates a wrapper around the body.

opentelemetry-go-contrib/instrumentation/net/http/otelhttp/transport.go

Lines 119 to 139 in 17eafa1

	var lastBW *request.BodyWrapper // Records the last body wrapper. Can be nil.
	maybeWrapBody := func(body io.ReadCloser) io.ReadCloser {
	if body == nil || body == http.NoBody {
	return body
	}
	bw := request.NewBodyWrapper(body, func(int64) {})
	lastBW = bw
	return bw
	}
	r.Body = maybeWrapBody(r.Body)
	if r.GetBody != nil {
	originalGetBody := r.GetBody
	r.GetBody = func() (io.ReadCloser, error) {
	b, err := originalGetBody()
	if err != nil {
	lastBW = nil // The underlying transport will fail to make a retry request, hence, record no data.
	return nil, err
	}
	return maybeWrapBody(b), nil
	}
	}

The net/http library has a fallback to rewind a body in case nothing was written:

https://github.com/golang/go/blob/56ebf80e57db9f61981fc0636fc6419dc6f68eda/src/net/http/transport.go#L2225-L2229
When nothing was written it returns a nothingWrittenError{err} which actually causes the request body to be rewinded later in the logic.

With the wrapped body the check fails at: https://github.com/golang/go/blob/56ebf80e57db9f61981fc0636fc6419dc6f68eda/src/net/http/transfer.go#L106-L108

The wrapped body is not a known type checked in isKnownInMemoryReader(t.Body), which sets t.FlushHeaders = true.

On the next retry request, even though the seek index is at the end of the body, we send another request that only contains the headers with a content length > 0 but an empty content.

The connection then writes the header bytes to the written bytes counter which causes a mismatch in the error mapping logic and instead of returning a nothingWrittenError we get a non rewindable error.

Mitigation

In my case the solution was quite simple. I simply clone the request body and rewind it myself before calling the underlying RoundTripper again

func cloneRequestBody(req *http.Request) error {
	if req.Body == nil || req.Body == http.NoBody {
		return nil
	}

	if req.GetBody == nil {
		return nil
	}

	clonedBody, err := req.GetBody()
	if err != nil {
		return err
	}

	req.Body = clonedBody
	return nil
}

Expected behavior

I expected the net/http logic to rewind the request properly

Steps to Reproduce

We wrapped the otelhttp RoundTripper with this https://github.com/wundergraph/cosmo/blob/main/router/internal/retrytransport/retry_transport.go.

Operating System

MacOS Version 26.2

Device Architecture

ARM64

Go Version

1.25

Component Version

otelhttp@v0.67.0

[pellared]

Can you please quickly double-check if it is not fixed already in main (use a pseudo-version) thanks to #8618?

CC @ash2k

[Noroth]

Ah my bad, I didn't see that. I'll check in a bit and let you know

[Noroth]

I tested it with the following version but I'm still getting issues

go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.67.1-0.20260401134203-17eafa1b9674

The first request provides the content, each consecutive write to the conn looks like this:
Content length of 28 but no content

"POST /graphql HTTP/1.1\r\nHost: 127.0.0.1:54551\r\nUser-Agent: Go-http-client/1.1\r\nContent-Length: 28\r\nAccept: application/json\r\nAccept-Encoding: gzip\r\nAccept-Encoding: deflate\r\nContent-Type: application/json\r\n\r\n"

[pellared]

@Noroth, thanks! If you have some time, then it would also help if you create a failing unit test (that would pass if the issue is fixed).

[ash2k]

Happy to have a look once we have a reproducer.

[Noroth]

Hey @pellared @ash2k,

I'll wrote a very basic test with some comments that fails for me on the current main branch with exactly the error that I mentioned.
Did you want me to create a PR with the failing test or what would be the proper approach here?

Anyways I'll just post the code here, happy to help just let me know.

instrumentation/net/http/otelhttp/transport_test.go

// simple transport wrapper that retries on 500 errors
type simpleRetryTransport struct {
	rt         http.RoundTripper
	maxRetries int
}

func (t *simpleRetryTransport) RoundTrip(r *http.Request) (*http.Response, error) {
	resp, err := t.rt.RoundTrip(r)
	if err != nil {
		return nil, err
	}

	if resp.StatusCode != http.StatusInternalServerError {
		return resp, nil
	}

	for i := 0; i < t.maxRetries; i++ {
		resp, err = t.rt.RoundTrip(r)
		// Very simplistic retry logic. We do not expect an error here from the underlying transport.
		// and return early if we get an error.
		if err != nil {
			return nil, err
		}

		if resp.StatusCode != http.StatusInternalServerError {
			return resp, nil
		}
	}

	return nil, errors.New("max retries reached")
}

// This test is currently failing because the body is not rewound on retry.
// Instead after performing the request, we do not get a response, instead we get an error:
// Post "http://127.0.0.1:60265": net/http: HTTP/1.x transport connection broken: http: ContentLength=19 with Body length 0
func TestRewindBodyOnRetry(t *testing.T) {
	retryTransport := &simpleRetryTransport{
		rt:         NewTransport(http.DefaultTransport),
		maxRetries: 5,
	}

	counter := 0

	// Create a handler that returns 500 errors for the first two requests
	mux := http.NewServeMux()
	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		counter++
		if counter == 3 {
			// We will never reach this code for now because the retry round tripper returns early when we get an error.
			// Currently the logic will run twice with the 500 error before the retry transport cancels the request.
			w.WriteHeader(http.StatusOK)
			w.Write([]byte("OK"))
			return
		}

		w.WriteHeader(http.StatusInternalServerError)
		w.Write([]byte("Internal Server Error"))
	})

	server := httptest.NewServer(mux)
	defer server.Close()

	client := http.Client{Transport: retryTransport}

	postReq, err := http.NewRequestWithContext(t.Context(), http.MethodPost, server.URL, strings.NewReader(`{"message": "test"}`))
	// The problem is that the request body is not rewound on retry. For this to work correctly, there must be exactly 0 bytes written
	// to the underlying network connection. If more bytes are written than the original request sent, we won't encounter any issues.
	// The body wrapper in `NewTransport` is not recognized by the underlying net/http logic during unwrapping,
	// causing the request to be sent again with its headers but without the body.
	// The client automatically sets some headers, such as Content-Length. As a result, on the second retry, we end up sending an empty request with just headers.

	require.NoError(t, err)

	resp, err := client.Do(postReq)
	require.NoError(t, err)
	body, err := io.ReadAll(resp.Body)
	require.NoError(t, resp.Body.Close())
	assert.Equal(t, "OK", string(body))
}

[ash2k]

@Noroth I don't think this is supposed to work in the general case: if a Request was supplied that has an opaque body reader then it cannot be rewound by stdlib. Please see #8532 for the way I see it.

Alternatively, what is the API guarantee that is being broken? Where is it documented?

The test fails for me with this error:

Post "http://127.0.0.1:56247": net/http: HTTP/1.x transport connection broken: http: ContentLength=19 with Body length 0

It also fails if I remove the OTEL wrapper and just use the default transport:

Post "http://127.0.0.1:56252": http: ContentLength=19 with Body length 0

So, either the test is wrong or the simpleRetryTransport is wrong. I think it's both because they both rely on an invalid assumption that the Request can be reused. It can only be reused internally by the stdlib as part of a single HTTP call and it does that using the GetBody() function. Hence, the "mitigation" you've identified is the right way to do it. However, it may be better to reconstruct the whole Request - you can Clone() it before using the first time and then clone the clone and set the body for each attempt. This is future-proof and does not assume nothing will be changed in how other parts of the http.Request/http.Transport behave.

---

You may have a look at this code for inspiration too:

• https://github.com/hashicorp/go-retryablehttp/blob/867c4f597d1322e1642f39fd366f9df7a5c2cdc2/client.go#L319-L337 manages the body by itself, not via stdlib's body.
• https://github.com/hashicorp/go-retryablehttp/blob/867c4f597d1322e1642f39fd366f9df7a5c2cdc2/client.go#L693-L705 then gets a new body for each request/attempt.

Note how this is the job of the transport/client that is implementing retries, not the OTEL instrumentation. FWIW I'm personally using OTEL + that library and have never had any problems. (Although the library is not perfect)

[Noroth]

@ash2k You're right! My bad, I forgot to drain the response body, because otherwise the TCP connection is still in use so it doesn't get flagged as re-usable

As a said before I'm not 100% sure if it is the correct behavior or not I just stumbled across it when I upgraded the library.
It probably makes sense for a retry transport to rewind manually.

If you use the http.DefaultTransport and change the function to

func (t *simpleRetryTransport) RoundTrip(r *http.Request) (*http.Response, error) {
	resp, err := t.rt.RoundTrip(r)
	if err != nil {
		return nil, err
	}

	if resp.StatusCode != http.StatusInternalServerError {
		return resp, nil
	}

	io.Copy(io.Discard, resp.Body)
	resp.Body.Close()

	for i := 0; i < t.maxRetries; i++ {
		resp, err = t.rt.RoundTrip(r)
		// Very simplistic retry logic. We do not expect an error here from the underlying transport.
		// and return early if we get an error.
		if err != nil {
			return nil, err
		}

		if resp.StatusCode != http.StatusInternalServerError {
			return resp, nil
		}

		io.Copy(io.Discard, resp.Body)
		resp.Body.Close()
	}

	return nil, errors.New("max retries reached")
}

Then you can see that the internal reuse flag is set to true