service: surface streaming request body read errors to handlers#150
Open
EffortlessSteven wants to merge 1 commit into
Open
service: surface streaming request body read errors to handlers#150EffortlessSteven wants to merge 1 commit into
EffortlessSteven wants to merge 1 commit into
Conversation
`spawn_body_reader` logged `Body::poll_frame` errors and stopped the reader, so the handler-facing request stream saw a clean EOF when the HTTP request body failed mid-decode. A truncated or aborted client-streaming/bidi request was then indistinguishable from a complete client stream. Forward transport-level body read failures to the handler stream as `ConnectError::internal` while the reader is still decoding request messages (`BodyReader::on_body_error`), matching the unary body-read path. Once the reader is only draining trailing bytes (after END_STREAM, a decode error, or the handler dropping the request stream), the error stays diagnostic-only, preserving the END_STREAM-as-terminal drain contract. Adds regression coverage for both the surfaced (decoding) and suppressed (draining) paths.
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What this does
A request body transport failure (
Body::poll_framereturningErr) mid-decode madespawn_body_readerstop, ending the handler's request stream with a clean EOF. A truncated client-streaming/bidi upload was then indistinguishable from a complete stream, so a handler aggregating messages treated partial input as a finished stream.Fix: while still decoding, forward the failure to the handler stream as
ConnectError::internal(BodyReader::on_body_error), the same error the unary body-read path already returns. Once only draining trailing bytes (after END_STREAM, a decode error, or a dropped receiver), it stays diagnostic-only.Verification
Red on
main, green after: unpatched, the decoding-path test seesNonewhere anErrbelongs.None(clean EOF), patchedErr(Internal)after the messageErrcargo +nightly fmt --all --checkcargo clippy -p connectrpc --all-features --all-targets -- -D warningscargo test -p connectrpc --all-featuresReview map
connectrpc/src/service.rs:on_body_errorsurfaces the failure whileDecoding, suppresses it whileDraining; read loop calls it instead of dropping the cause.connectrpc/src/service.rs(tests): pin the surfaced and suppressed paths.