Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(app/core): a unit test suite for rescue body #3615

Merged
merged 10 commits into from
Feb 13, 2025

Conversation

cratelyn
Copy link
Collaborator

@cratelyn cratelyn commented Feb 13, 2025

linkerd-app-core includes an error recovery body middleware. this middleware will gracefully catch and report errors encountered when polling an inner body, and via an R-typed recovery strategy provided by the caller, will attempt to map the error to a gRPC status code denoting an error.

before we upgrade to hyper 1.0 in service of linkerd/linkerd2#8733, we add some test coverage to ensure that we preserve the behavior of this middleware.

see:

for historical context on this tower layer, see:

in #3559 (4b53081), we introduced a backported `Frame<T>` type, and a
`ForwardCompatibleBody<B>` type that allows us to interact with a
`http_body::Body` circa 0.4.6 in terms of frame-based interfaces that
match those of the 1.0 interface.

see linkerd/linkerd2#8733 for more information on upgrading hyper.

in #3559, we narrowly added this as an internal submodule of the
`linkerd-http-retry` library. these facilities however, would have
utility in other places such as `linkerd-app-core`.

this commit pulls these compatibility shims out into a
`linkerd-http-body-compat` library so that they can be imported and
reused elsewhere.

Signed-off-by: katelyn martin <[email protected]>
these are header values. `http::HeaderName` has a const fn constructor,
so let's use that.

Signed-off-by: katelyn martin <[email protected]>
we'll add a few tests for this middleware shortly.

this commit moves this middleware out into its own submodule.

Signed-off-by: katelyn martin <[email protected]>
for other body middleware, we hide inner enum variants and their
constituent members by using the "inner" pattern.

this commit tweaks `ResponseBody` to follow suit, such that it now holds
an `Inner`, but does not expose its passthrough and rescue variants to
callers.

Signed-off-by: katelyn martin <[email protected]>
this adds a small documentation comment describing what this type does.

Signed-off-by: katelyn martin <[email protected]>
this commit introduces a test suite for our error recovery middleware.

this body middleware provides a mechanism to "rescue" errors, gracefully
mapping an error encountered when polling a gRPC body into e.g. trailers
with a gRPC status code.

before we upgrade this middleware in service of linkerd/linkerd2#8733,
we add some test coverage to ensure that we preserve this middleware.

Signed-off-by: katelyn martin <[email protected]>
@cratelyn cratelyn marked this pull request as ready for review February 13, 2025 03:50
@cratelyn cratelyn requested a review from a team as a code owner February 13, 2025 03:50
Base automatically changed from kate/http-retry.outline-http-body-compat to main February 13, 2025 13:47
@cratelyn cratelyn merged commit b883089 into main Feb 13, 2025
15 checks passed
@cratelyn cratelyn deleted the kate/app-core.rescue-body-continues-to-yield-ready-none branch February 13, 2025 15:41
cratelyn added a commit that referenced this pull request Feb 14, 2025
this commit makes some minor alterations to our error recovery body
middleware. see linkerd/linkerd2#8733 for more
information.

this commit removes an `assert!` statement from the implementation of
`<Response<R, B> as Body>::poll_data()`. see the documentation of
`Body::poll_frame()`:

> Once the end of the stream is reached, implementations should
> continue to return [`Poll::Ready(None)`].

hyperium/http-body@1090bff#diff-33aabe8c2aaa7614022addf244245e09bbff576a67a9ae3c6938c8a868201d36R60-R61

to do this, this commit introduces a distinct terminal state
`Inner::Rescued` to represent when the underlying `B`-typed body has
yielded an error and been rescued. once in this state the body will
yield no more data frames, instead yielding a collection of trailers
describing the mid-stream error that was encountered by the underlying
body.

the call to `R::rescue` is also moved down into the helper function fka
`grpc_trailers()`. this helps the function follow the grain of our
"state machine" a little more directly.

see #3615, #3614, and #3611 for pretext to this change.
cratelyn added a commit that referenced this pull request Feb 14, 2025
this commit makes some minor alterations to our error recovery body
middleware. see linkerd/linkerd2#8733 for more
information.

this commit removes an `assert!` statement from the implementation of
`<Response<R, B> as Body>::poll_data()`. see the documentation of
`Body::poll_frame()`:

> Once the end of the stream is reached, implementations should
> continue to return [`Poll::Ready(None)`].

hyperium/http-body@1090bff#diff-33aabe8c2aaa7614022addf244245e09bbff576a67a9ae3c6938c8a868201d36R60-R61

to do this, this commit introduces a distinct terminal state
`Inner::Rescued` to represent when the underlying `B`-typed body has
yielded an error and been rescued. once in this state the body will
yield no more data frames, instead yielding a collection of trailers
describing the mid-stream error that was encountered by the underlying
body.

the call to `R::rescue` is also moved down into the helper function fka
`grpc_trailers()`. this helps the function follow the grain of our
"state machine" a little more directly.

see #3615, #3614, and #3611 for pretext to this change.

Signed-off-by: katelyn martin <[email protected]>
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.

2 participants