feat(http/retry): model `PeekTrailersBody` with `Frame<T>` #3559

cratelyn · 2025-01-22T18:21:56Z

this branch contains a sequence of commits that refactor PeekTrailersBody.

this branch is specifically focused on making this body middleware
forward-compatible with the 1.0 interface(s) of http_body::Body and
http_body_util::BodyExt.

it does this in two main steps: (1) temporarily vendoring http_body::Frame<T>
and providing a compatibility shim that provides a frame() method for a body,
and (2) modeling PeekTrailersBody and its peeking logic in terms of this
Frame<'a, T> future.

feat(http/retry): add `Frame<T>` compatibility facilities

this commit introduces a compat submodule to linkerd-http-retry.

this helps us frontrun the task of replacing all of the finicky control flow in
PeekTrailersBody using the antiquated data() and trailers() future
combinators. instead, we can perform our peeking in terms of an approximation
of http_body_util::BodyExt::frame().

to accomplish this, this commit vendors a copy of the Frame<T> type. we can
use this to preemptively model our peek body in terms of this type, and move to
the "real" version of it when we're upgrading in pr #3504.

additionally, this commit includes a type called ForwardCompatibleBody,
and a variant of the Frame<'a, T> combinator. these are a bit boilerplate-y,
admittedly, but the pleasant part of this is that we have, in effect, migrated
the trickiest body middleware in advance of #3504. once we upgrade to http-body
1.0, all of these types can be removed.

Signed-off-by: katelyn martin [email protected]

refactor(http/retry): `PeekTrailersBody` uses `BodyExt::frame()`

this commit reworks PeekTrailersBody.

the most important goal here is replacing the control flow of read_body(),
which polls a body using BodyExt future combinators data() and frame()
for up to two frames, with varying levels of persistence depending on outcomes.

to quote #3556:

the intent of this type is to only yield the asynchronous task
responsible for reading the body once. depending on what the inner
body yields, and when, this can result in combinations of: no data
frames and no trailers, no data frames with trailers, one data frame
and no trailers, one data frame with trailers, or two data frames.
moreover, depending on which of these are yielded, the body will call
.await some scenarios, and only poll functions once in others.

migrating this to the Frame and poll_frame() style of the 1.0 Body
interface, away from the 0.4 interface that provides distinct
poll_data() and poll_trailers() methods, is fundamentally tricky.

this means that PeekTrailersBody is notably difficult to port across the
http-body 0.4/1.0 upgrade boundary.

this body middleware must navigate a number of edge conditions, and once it
has obtained a Frame<T>, make use of conversion methods to ascertain
whether it is a data or trailers frame, due to the fact that its internal enum
representation is not exposed publicly. one it has done all of that, it must do
the same thing once more to examine the second frame.

this commit uses the compatibility facilities and backported Frame<T>
introduced in the previous commit, and rewrites this control flow using a form
of the BodyExt::frame() combinator.

this means that this middleware is forward-compatible with http-body 1.0, which
will dramatically simplify the remaining migration work to be done in #3504.

see linkerd/linkerd2#8733 for more information and
other links related to this ongoing migration work.

refactor(http/retry): mock body enforces `poll_trailers()` contract

this commit addresses a TODO note, and tightens the enforcement of a rule
defined by the v0.4 signature of the Body trait.

this commit changes the mock body type, used in tests, so that it will panic if
the caller improperly polls for a trailers frame before the final data frame
has been yielded.

previously, a comment indicated that we were "fairly sure" this was okay. while
that may have been acceptable in practice, the changes in the previous commit
mean that we now properly respect these rules.

thus, a panic can be placed here, to enforce that "[is] only be called once
poll_data() returns None", per the documentation.

this commit introduces a `compat` submodule to `linkerd-http-retry`. this helps us frontrun the task of replacing all of the finicky control flow in `PeekTrailersBody` using the antiquated `data()` and `trailers()` future combinators. instead, we can perform our peeking in terms of an approximation of `http_body_util::BodyExt::frame()`. to accomplish this, this commit vendors a copy of the `Frame<T>` type. we can use this to preemptively model our peek body in terms of this type, and move to the "real" version of it when we're upgrading in pr #3504. additionally, this commit includes a type called `ForwardCompatibleBody`, and a variant of the `Frame<'a, T>` combinator. these are a bit boilerplate-y, admittedly, but the pleasant part of this is that we have, in effect, migrated the trickiest body middleware in advance of #3504. once we upgrade to http-body 1.0, all of these types can be removed. https://docs.rs/http-body-util/latest/http_body_util/trait.BodyExt.html#method.frame https://docs.rs/http-body-util/0.1.2/src/http_body_util/combinators/frame.rs.html#10 Signed-off-by: katelyn martin <[email protected]>

this commit reworks `PeekTrailersBody`. the most important goal here is replacing the control flow of `read_body()`, which polls a body using `BodyExt` future combinators `data()` and `frame()` for up to two frames, with varying levels of persistence depending on outcomes. to quote #3556: > the intent of this type is to only yield the asynchronous task > responsible for reading the body once. depending on what the inner > body yields, and when, this can result in combinations of: no data > frames and no trailers, no data frames with trailers, one data frame > and no trailers, one data frame with trailers, or two data frames. > moreover, depending on which of these are yielded, the body will call > .await some scenarios, and only poll functions once in others. > > migrating this to the Frame<T> and poll_frame() style of the 1.0 Body > interface, away from the 0.4 interface that provides distinct > poll_data() and poll_trailers() methods, is fundamentally tricky. this means that `PeekTrailersBody` is notably difficult to port across the http-body 0.4/1.0 upgrade boundary. this body middleware must navigate a number of edge conditions, and once it _has_ obtained a `Frame<T>`, make use of conversion methods to ascertain whether it is a data or trailers frame, due to the fact that its internal enum representation is not exposed publicly. one it has done all of that, it must do the same thing once more to examine the second frame. this commit uses the compatibility facilities and backported `Frame<T>` introduced in the previous commit, and rewrites this control flow using a form of the `BodyExt::frame()` combinator. this means that this middleware is forward-compatible with http-body 1.0, which will dramatically simplify the remaining migration work to be done in #3504. see linkerd/linkerd2#8733 for more information and other links related to this ongoing migration work. Signed-off-by: katelyn martin <[email protected]>

this commit addresses a `TODO` note, and tightens the enforcement of a rule defined by the v0.4 signature of the `Body` trait. this commit changes the mock body type, used in tests, so that it will panic if the caller improperly polls for a trailers frame before the final data frame has been yielded. previously, a comment indicated that we were "fairly sure" this was okay. while that may have been acceptable in practice, the changes in the previous commit mean that we now properly respect these rules. thus, a panic can be placed here, to enforce that "[is] only be called once `poll_data()` returns `None`", per the documentation. Signed-off-by: katelyn martin <[email protected]>

cratelyn

💬 breadcrumbs for review...