Fix request body handling in DoExponentialBackoff #89
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, DoExponentialBackoff relied on Seek to reset request bodies between retries. This approach failed when bodies didn't implement Seek or when req.GetBody wasn't set (e.g., manually wrapped io.NopCloser), causing "ContentLength=N with Body length 0" errors on retry attempts. The issue occurred because req.Clone() reuses the exhausted body reader without resetting it, leading to empty bodies on subsequent attempts while ContentLength remained set from the original request. Now read the entire request body upfront and create fresh io.Readers for each retry attempt. This ensures: - Body is always available for all retry attempts - ContentLength is correctly set to match the actual body - Works reliably with all io.Reader types, not just seekable ones Added test case with io.NopCloser-wrapped bytes.Buffer that validates both body content and ContentLength matching across retry attempts.
Pull Request Test Coverage Report for Build 20778351413Details
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in DoExponentialBackoff where request bodies would fail to be sent correctly on retry attempts. The previous implementation relied on Seek() to reset bodies between retries, which failed for non-seekable readers or when req.GetBody wasn't set, causing "ContentLength=N with Body length 0" errors.
Key changes:
- Modified body handling to read the entire request body upfront and create fresh readers for each retry
- Updated
ContentLengthto match the actual body size on each retry attempt - Added comprehensive test case validating body content and ContentLength across retries with
io.NopCloser-wrapped bodies
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| httputilx/httputilx.go | Replaced Seek-based body reset with upfront body reading and fresh reader creation for each retry |
| httputilx/httputilx_test.go | Added test case to verify request bodies are correctly preserved across retry attempts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, DoExponentialBackoff relied on Seek to reset request bodies between retries. This approach failed when bodies didn't implement Seek or when req.GetBody wasn't set (e.g., manually wrapped io.NopCloser), causing "ContentLength=N with Body length 0" errors on retry attempts.
The issue occurred because req.Clone() reuses the exhausted body reader without resetting it, leading to empty bodies on subsequent attempts while ContentLength remained set from the original request.
Now read the entire request body upfront and create fresh io.Readers for each retry attempt. This ensures:
Added test case with io.NopCloser-wrapped bytes.Buffer that validates both body content and ContentLength matching across retry attempts.