diff --git a/src/Grpc.Net.Client/Internal/GrpcCall.cs b/src/Grpc.Net.Client/Internal/GrpcCall.cs index 890b02379..fae7f9907 100644 --- a/src/Grpc.Net.Client/Internal/GrpcCall.cs +++ b/src/Grpc.Net.Client/Internal/GrpcCall.cs @@ -307,15 +307,6 @@ private async Task GetResponseHeadersCoreAsync() { var httpResponse = await HttpResponseTask.ConfigureAwait(false); - // Check if the headers have a status. If they do then wait for the overall call task - // to complete before returning headers. This means that if the call failed with a - // a status then it is possible to await response headers and then call GetStatus(). - var grpcStatus = HttpRequestHelpers.GetHeaderValue(httpResponse.Headers, GrpcProtocolConstants.StatusTrailer); - if (grpcStatus != null) - { - await CallTask.ConfigureAwait(false); - } - var metadata = GrpcProtocolHelpers.BuildMetadata(httpResponse.Headers); // https://github.com/grpc/proposal/blob/master/A6-client-retries.md#exposed-retry-metadata @@ -550,6 +541,16 @@ private async Task RunCall(HttpRequestMessage request, TimeSpan? timeout) } else { + if (ClientStreamReader != null && status.Value.StatusCode == StatusCode.OK) + { + // This is a Trailers-Only response with OK status for a streaming call. + // Hand the response to the stream reader so it can process the (empty) + // stream and resolve the final call status. TCS will also be set in Dispose. + ClientStreamReader.HttpResponseTcs.TrySetResult((HttpResponse, status)); + + status = await CallTask.ConfigureAwait(false); + } + finished = FinishCall(request, diagnosticSourceEnabled, activity, status.Value); FinishResponseAndCleanUp(status.Value); } diff --git a/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs b/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs index 0ce01c4b5..0760c0490 100644 --- a/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs +++ b/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs @@ -326,7 +326,11 @@ public static Status GetResponseStatus(HttpResponseMessage httpResponse, bool is Status? status; try { - if (!TryGetStatusCore(httpResponse.TrailingHeaders(), out status)) + // Scenarios: + // 1. The status will be in the trailers for a response with a message. + // 2. Trailers are in the headers when there is no message. That means we also check the headers for a trailers-only only response. + // 3. No status. This is an error. Return cancelled status. + if (!TryGetStatusCore(httpResponse.TrailingHeaders(), out status) && !TryGetStatusCore(httpResponse.Headers, out status)) { var detail = "No grpc-status found on response."; if (isBrowser) diff --git a/src/Grpc.Net.Client/Internal/HttpContentClientStreamReader.cs b/src/Grpc.Net.Client/Internal/HttpContentClientStreamReader.cs index 907604752..af1ff2248 100644 --- a/src/Grpc.Net.Client/Internal/HttpContentClientStreamReader.cs +++ b/src/Grpc.Net.Client/Internal/HttpContentClientStreamReader.cs @@ -142,7 +142,11 @@ private async Task MoveNextCore(CancellationToken cancellationToken) #if NET5_0_OR_GREATER _responseStream = await _httpResponse.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); #else - _responseStream = await _httpResponse.Content.ReadAsStreamAsync().ConfigureAwait(false); + // On .NET Framework, HttpResponseMessage.Content can be null when no content + // body was set (e.g. a Trailers-Only response). Use Stream.Null in that case. + _responseStream = _httpResponse.Content != null + ? await _httpResponse.Content.ReadAsStreamAsync().ConfigureAwait(false) + : Stream.Null; #endif } catch (ObjectDisposedException) diff --git a/test/Grpc.Net.Client.Tests/AsyncClientStreamingCallTests.cs b/test/Grpc.Net.Client.Tests/AsyncClientStreamingCallTests.cs index 8f126b8e8..3cb63f4a1 100644 --- a/test/Grpc.Net.Client.Tests/AsyncClientStreamingCallTests.cs +++ b/test/Grpc.Net.Client.Tests/AsyncClientStreamingCallTests.cs @@ -23,6 +23,7 @@ using Grpc.Net.Client.Internal; using Grpc.Net.Client.Internal.Http; using Grpc.Net.Client.Tests.Infrastructure; +using Grpc.Shared; using Grpc.Tests.Shared; using Microsoft.Extensions.Logging.Testing; using NUnit.Framework; @@ -411,6 +412,58 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc #endif } + [TestCase(StatusCode.OK, "Detail!")] + [TestCase(StatusCode.Unauthenticated, "")] + public async Task AsyncClientStreamingCall_TrailersOnly_TrailersReturnedWithHeaders(StatusCode statusCode, string statusMessage) + { + // Arrange + HttpResponseMessage? responseMessage = null; + var httpClient = ClientTestHelpers.CreateTestClient(request => + { + responseMessage = ResponseUtils.CreateResponse(HttpStatusCode.OK, new ByteArrayContent(Array.Empty()), grpcStatusCode: null); + responseMessage.Headers.Add(GrpcProtocolConstants.StatusTrailer, statusCode.ToString("D")); + responseMessage.Headers.Add(GrpcProtocolConstants.MessageTrailer, statusMessage); + return Task.FromResult(responseMessage); + }); + var invoker = HttpClientCallInvokerFactory.Create(httpClient); + + // Act + var call = invoker.AsyncClientStreamingCall(); + var headers = await call.ResponseHeadersAsync.DefaultTimeout(); + if (statusCode == StatusCode.OK) + { + // Trailers-only with OK status has no response message to deserialize + var ex = await ExceptionAssert.ThrowsAsync(() => call.ResponseAsync).DefaultTimeout(); + Assert.AreEqual(StatusCode.Internal, ex.Status.StatusCode); + StringAssert.StartsWith("Failed to deserialize response message.", ex.Status.Detail); + } + else + { + var ex = await ExceptionAssert.ThrowsAsync(() => call.ResponseAsync).DefaultTimeout(); + Assert.AreEqual(statusCode, ex.StatusCode); + Assert.AreEqual(statusMessage, ex.Status.Detail); + } + + // Assert + Assert.NotNull(responseMessage); + + Assert.IsFalse(responseMessage!.TrailingHeaders().Any()); // sanity check that there are no trailers + + if (statusCode == StatusCode.OK) + { + Assert.AreEqual(StatusCode.Internal, call.GetStatus().StatusCode); + StringAssert.StartsWith("Failed to deserialize response message.", call.GetStatus().Detail); + } + else + { + Assert.AreEqual(statusCode, call.GetStatus().StatusCode); + Assert.AreEqual(statusMessage, call.GetStatus().Detail); + } + + Assert.AreEqual(0, headers.Count); + Assert.AreEqual(0, call.GetTrailers().Count); + } + [Test] public async Task ClientStreamWriter_CancelledBeforeCallStarts_ThrowsError() { diff --git a/test/Grpc.Net.Client.Tests/AsyncDuplexStreamingCallTests.cs b/test/Grpc.Net.Client.Tests/AsyncDuplexStreamingCallTests.cs index 0dfe9c663..21f88a8a1 100644 --- a/test/Grpc.Net.Client.Tests/AsyncDuplexStreamingCallTests.cs +++ b/test/Grpc.Net.Client.Tests/AsyncDuplexStreamingCallTests.cs @@ -181,6 +181,48 @@ await streamContent.AddDataAndWait(await ClientTestHelpers.GetResponseDataAsync( Assert.IsFalse(await moveNextTask3.DefaultTimeout()); } + [TestCase(StatusCode.OK, "Detail!")] + [TestCase(StatusCode.Unauthenticated, "")] + public async Task AsyncDuplexStreamingCall_TrailersOnly_TrailersReturnedWithHeaders(StatusCode statusCode, string statusMessage) + { + // Arrange + HttpResponseMessage? responseMessage = null; + var httpClient = ClientTestHelpers.CreateTestClient(request => + { + responseMessage = ResponseUtils.CreateResponse(HttpStatusCode.OK, new ByteArrayContent(Array.Empty()), grpcStatusCode: null); + responseMessage.Headers.Add(GrpcProtocolConstants.StatusTrailer, statusCode.ToString("D")); + responseMessage.Headers.Add(GrpcProtocolConstants.MessageTrailer, statusMessage); + return Task.FromResult(responseMessage); + }); + var invoker = HttpClientCallInvokerFactory.Create(httpClient); + + // Act + var call = invoker.AsyncDuplexStreamingCall(); + var headers = await call.ResponseHeadersAsync.DefaultTimeout(); + var moveNextTask = call.ResponseStream.MoveNext(CancellationToken.None); + if (statusCode == StatusCode.OK) + { + Assert.IsFalse(await moveNextTask.DefaultTimeout()); + } + else + { + var ex = await ExceptionAssert.ThrowsAsync(() => moveNextTask).DefaultTimeout(); + Assert.AreEqual(statusCode, ex.StatusCode); + Assert.AreEqual(statusMessage, ex.Status.Detail); + } + + // Assert + Assert.NotNull(responseMessage); + + Assert.IsFalse(responseMessage!.TrailingHeaders().Any()); // sanity check that there are no trailers + + Assert.AreEqual(statusCode, call.GetStatus().StatusCode); + Assert.AreEqual(statusMessage, call.GetStatus().Detail); + + Assert.AreEqual(0, headers.Count); + Assert.AreEqual(0, call.GetTrailers().Count); + } + [Test] public async Task AsyncDuplexStreamingCall_CancellationDisposeRace_Success() { diff --git a/test/Grpc.Net.Client.Tests/AsyncServerStreamingCallTests.cs b/test/Grpc.Net.Client.Tests/AsyncServerStreamingCallTests.cs index 151238300..94dc32f41 100644 --- a/test/Grpc.Net.Client.Tests/AsyncServerStreamingCallTests.cs +++ b/test/Grpc.Net.Client.Tests/AsyncServerStreamingCallTests.cs @@ -335,18 +335,22 @@ public async Task ClientStreamReader_WriteWithInvalidHttpStatus_ErrorThrown() Assert.AreEqual(StatusCode.Unimplemented, ex.StatusCode); Assert.AreEqual("Bad gRPC response. HTTP status code: 404", ex.Status.Detail); + + Assert.AreEqual(StatusCode.Unimplemented, call.GetStatus().StatusCode); + Assert.AreEqual("Bad gRPC response. HTTP status code: 404", call.GetStatus().Detail); } - [Test] - public async Task AsyncServerStreamingCall_TrailersOnly_TrailersReturnedWithHeaders() + [TestCase(StatusCode.OK, "Detail!")] + [TestCase(StatusCode.Unauthenticated, "")] + public async Task AsyncServerStreamingCall_TrailersOnly_TrailersReturnedWithHeaders(StatusCode statusCode, string statusMessage) { // Arrange HttpResponseMessage? responseMessage = null; var httpClient = ClientTestHelpers.CreateTestClient(request => { responseMessage = ResponseUtils.CreateResponse(HttpStatusCode.OK, new ByteArrayContent(Array.Empty()), grpcStatusCode: null); - responseMessage.Headers.Add(GrpcProtocolConstants.StatusTrailer, StatusCode.OK.ToString("D")); - responseMessage.Headers.Add(GrpcProtocolConstants.MessageTrailer, "Detail!"); + responseMessage.Headers.Add(GrpcProtocolConstants.StatusTrailer, statusCode.ToString("D")); + responseMessage.Headers.Add(GrpcProtocolConstants.MessageTrailer, statusMessage); return Task.FromResult(responseMessage); }); var invoker = HttpClientCallInvokerFactory.Create(httpClient); @@ -354,15 +358,25 @@ public async Task AsyncServerStreamingCall_TrailersOnly_TrailersReturnedWithHead // Act var call = invoker.AsyncServerStreamingCall(new HelloRequest()); var headers = await call.ResponseHeadersAsync.DefaultTimeout(); - Assert.IsFalse(await call.ResponseStream.MoveNext(CancellationToken.None).DefaultTimeout()); + var moveNextTask = call.ResponseStream.MoveNext(CancellationToken.None); + if (statusCode == StatusCode.OK) + { + Assert.IsFalse(await moveNextTask.DefaultTimeout()); + } + else + { + var ex = await ExceptionAssert.ThrowsAsync(() => moveNextTask).DefaultTimeout(); + Assert.AreEqual(statusCode, ex.StatusCode); + Assert.AreEqual(statusMessage, ex.Status.Detail); + } // Assert Assert.NotNull(responseMessage); Assert.IsFalse(responseMessage!.TrailingHeaders().Any()); // sanity check that there are no trailers - Assert.AreEqual(StatusCode.OK, call.GetStatus().StatusCode); - Assert.AreEqual("Detail!", call.GetStatus().Detail); + Assert.AreEqual(statusCode, call.GetStatus().StatusCode); + Assert.AreEqual(statusMessage, call.GetStatus().Detail); Assert.AreEqual(0, headers.Count); Assert.AreEqual(0, call.GetTrailers().Count);