Handle Trailers-Only OK responses in streaming calls without exception handling#2697
Open
michaelmccord wants to merge 1 commit intogrpc:masterfrom
Open
Handle Trailers-Only OK responses in streaming calls without exception handling#2697michaelmccord wants to merge 1 commit intogrpc:masterfrom
michaelmccord wants to merge 1 commit intogrpc:masterfrom
Conversation
|
|
Avoid expensive TaskCanceledException in streaming calls when the server returns grpc-status in response headers (Trailers-Only). Set HttpResponseTcs before cleanup so MoveNextCore can read the empty stream gracefully. Also handle null HttpResponseMessage.Content on .NET Framework and read grpc-status from response headers in GetResponseStatus as a fallback.
Member
|
Why have lots of people immediately thumbs up this? Do you know if there are existing unit tests for this scenario? |
Author
One piece that I'm not entirely sure of is the removal of the await from |
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
2 participants
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.
Summary
TaskCanceledExceptionin server streaming calls when the server returns a Trailers-Only response with OK status by settingHttpResponseTcswithTrySetResultbeforeCleanuprunsGetResponseStatusto readgrpc-statusfrom response headers when not found in trailers (Trailers-Only responses per gRPC spec)HttpResponseMessage.Contenton .NET Framework inHttpContentClientStreamReader.MoveNextCoreawait CallTaskfromGetResponseHeadersCoreAsyncto prevent deadlock whenResponseHeadersAsyncis awaited beforeMoveNexton streaming calls with status in headersFixes #2696
Test plan
AsyncServerStreamingCall_TrailersOnly_TrailersReturnedWithHeaderspasses on all TFMs (net8.0, net9.0, net10.0, net462)AsyncServerStreamingCall_NoMessagesSuccess_SuccussCommitLoggedpasses on all TFMs including net462Grpc.Net.Client.Testssuite passes with 0 failures across all TFMsGrpc.AspNetCore.Server.Tests