Skip to content

Fix RecordingClient request body recording for non‑streaming bodies#4496

Open
signadou wants to merge 4 commits intosmithy-lang:mainfrom
Piebald-AI:fix-recordingclient-request-body-recording-for-non-streaming-bodies-012426
Open

Fix RecordingClient request body recording for non‑streaming bodies#4496
signadou wants to merge 4 commits intosmithy-lang:mainfrom
Piebald-AI:fix-recordingclient-request-body-recording-for-non-streaming-bodies-012426

Conversation

@signadou
Copy link

Motivation and Context

aws_smithy_http_client::test_util::dvr::RecordingClient can cause real AWS SDK requests with non-streaming request bodies to fail (observed as 400 Bad Request from awselb/2.0). The same request succeeds when using the standard client without RecordingClient.

Closes #4495

Description

For non‑streaming request bodies (where SdkBody::bytes() is available), we now buffer the bytes, record them, and rebuild the request body from the buffered data instead of swapping in a channel‑based streaming body. This avoids timing issues that can break HTTP/2 framing and cause 400 responses from ELB.

Also preserves size hints on channel bodies by carrying over the original content length, keeping body metadata consistent with Content-Length.

Note: This change is limited to the DVR test utility (aws-smithy-http-client test_util::dvr) and does not affect default/prod HTTP clients.

Testing

  • Ran cargo test -p aws-smithy-http-client --features "test-util,legacy-rustls-ring" dvr inside rust-runtime and confirmed the tests still pass.
  • Updated real project to use patched crate and confirmed the problem was fixed.

Checklist

  • For changes to the smithy-rs codegen or runtime crates, I have created a changelog entry Markdown file in the .changelog directory, specifying "client," "server," or both in the applies_to key.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@signadou signadou requested review from a team as code owners January 24, 2026 19:11
Copy link
Contributor

@landonxjames landonxjames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generally looks good to me. There is one CI failure related to rustfmt issues, tried to fix it myself, but I don't seem to have permissions to push to your branch. Can approve once that is fixed. Note you'll also need to bump the patch number in aws-smithy-http-client/Cargo.toml since that is going to fail a later check we run.

@signadou signadou force-pushed the fix-recordingclient-request-body-recording-for-non-streaming-bodies-012426 branch from 41b0456 to 1654879 Compare January 27, 2026 18:37
This fixes a bug in `RecordingClient` where recording request bodies could
corrupt real HTTP requests. For non‑streaming request bodies (where `SdkBody::bytes()`
is available), we now buffer the bytes, record them, and rebuild the request body
from the buffered data instead of swapping in a channel‑based streaming body. This
avoids timing issues that can break HTTP/2 framing and cause 400 responses from ELB.

Also preserves size hints on channel bodies by carrying over the original content length,
keeping body metadata consistent with `Content-Length`.
@signadou signadou force-pushed the fix-recordingclient-request-body-recording-for-non-streaming-bodies-012426 branch from 1654879 to 819f9d9 Compare January 27, 2026 18:39
@signadou
Copy link
Author

@landonxjames Thank you for reviewing!  I think you're not able to push because I forked this repo into the Piebald-AI organization instead of into my personal account.  I'll make sure to use a personal fork for PRs in the future. I've fixed the formatting and bumped the patch version.

@signadou signadou requested a review from landonxjames January 30, 2026 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RecordingClient breaks requests with non-streaming request bodies (HTTP 400 from ELB)

2 participants