fix(http/retry): PeekTrailersBody<B> retains first frame
#3947
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
this commit introduces additional test coverage to `linker_http_retry::peek_trailers::PeekTrailersBody<B>`. this body middleware is used to facilitate transparent http retries, and allows callers to possibly inspect the trailers for a response, by polling an `http_body::Body`. this commit introduces additional unit test coverage that confirms that the data and trailers of the inner body are passed through faithfully. Signed-off-by: katelyn martin <[email protected]>
this commit introduces some additional coverage for bodies that return `Pending` when polled a second time. Signed-off-by: katelyn martin <[email protected]>
this commit fixes a logical bug with `linkerd_http_retry::peek_trailers::PeekTrailersBody::<B>::read_body(..)`. `read_body(..)` constructs a `PeekTrailersBody<B>`, by polling the inner body to see whether or not it can reach the end of the stream by only yielding to the asynchronous runtime once. in #3559, we restructured this middleware's internal modeling to reflect the `Frame<T>`-oriented signatures of the `http_body::Body` trait's 1.0 interface. unfortunately, this included a bug which could cause the first frame in a stream to be discarded if the second `Body::poll_frame()` call (_invoked via `now_or_never()`_) returns `Pending`. this could cause non-deterministic errors for users when sending traffic to HTTPRoutes and GRPCRoutes with retry annotations applied. this commit rectifies this problem, ensuring that the first frame is not discarded when attempting to peek a body's trailers. Signed-off-by: katelyn martin <[email protected]>
olix0r
approved these changes
Jun 2, 2025
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.
see linkerd/linkerd2#14050.
this change fixes a logical bug with
linkerd_http_retry::peek_trailers::PeekTrailersBody::<B>::read_body(..).read_body(..)constructs aPeekTrailersBody<B>, by polling the innerbody to see whether or not it can reach the end of the stream by only
yielding to the asynchronous runtime once.
in #3559, we restructured this middleware's
internal modeling to reflect the
Frame<T>-oriented signatures of thehttp_body::Bodytrait's 1.0 interface.unfortunately, this included a bug which could cause the first frame in
a stream to be discarded if the second
Body::poll_frame()call(invoked via
now_or_never()) returnsPending. this could causenon-deterministic errors for users when sending traffic to HTTPRoutes
and GRPCRoutes with retry annotations applied.
this change rectifies this problem, ensuring that the first frame is not
discarded when attempting to peek a body's trailers.
to confirm that this works as expected, additional test coverage is
introduced that confirms that the data and trailers of the inner body
are passed through faithfully.