From 47e1c79a3b63b144ebeb3c29fe806086c5ee3cb2 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 9 Oct 2019 11:51:46 +1300 Subject: [PATCH] Include trailers in client with RpcException (#567) --- src/Grpc.Net.Client/Internal/GrpcCall.cs | 65 +++++--- .../Internal/HttpContentClientStreamReader.cs | 2 +- .../Client/TrailerMetadataTests.cs | 143 ++++++++++++++++++ testassets/InteropTestsWebsite/Program.cs | 4 +- 4 files changed, 186 insertions(+), 28 deletions(-) create mode 100644 test/FunctionalTests/Client/TrailerMetadataTests.cs diff --git a/src/Grpc.Net.Client/Internal/GrpcCall.cs b/src/Grpc.Net.Client/Internal/GrpcCall.cs index f99020e7d..0eef40281 100644 --- a/src/Grpc.Net.Client/Internal/GrpcCall.cs +++ b/src/Grpc.Net.Client/Internal/GrpcCall.cs @@ -190,7 +190,7 @@ public void EnsureNotDisposed() public Exception CreateCanceledStatusException() { var status = (CallTask.IsCompletedSuccessfully) ? CallTask.Result : new Status(StatusCode.Cancelled, string.Empty); - return new RpcException(status); + return CreateRpcException(status); } /// @@ -241,6 +241,9 @@ public Task GetResponseAsync() // An explicitly specified status header has priority over other failing statuses if (GrpcProtocolHelpers.TryGetStatusCore(httpResponse.Headers, out var status)) { + // Trailers are in the header because there is no message. + // Note that some default headers will end up in the trailers (e.g. Date, Server). + _trailers = GrpcProtocolHelpers.BuildMetadata(httpResponse.Headers); return status; } @@ -298,16 +301,35 @@ public Metadata GetTrailers() { using (StartScope()) { - if (_trailers == null) + if (!TryGetTrailers(out var trailers)) { - ValidateTrailersAvailable(); + // Throw InvalidOperationException here because documentation on GetTrailers says that + // InvalidOperationException is thrown if the call is not complete. + throw new InvalidOperationException("Can't get the call trailers because the call has not completed successfully."); + } - Debug.Assert(HttpResponse != null); - _trailers = GrpcProtocolHelpers.BuildMetadata(HttpResponse.TrailingHeaders); + return trailers; + } + } + + private bool TryGetTrailers([NotNullWhen(true)] out Metadata? trailers) + { + if (_trailers == null) + { + // Trailers are read from the end of the request. + // If the request isn't finished then we can't get the trailers. + if (!ResponseFinished) + { + trailers = null; + return false; } - return _trailers; + Debug.Assert(HttpResponse != null); + _trailers = GrpcProtocolHelpers.BuildMetadata(HttpResponse.TrailingHeaders); } + + trailers = _trailers; + return true; } private void SetMessageContent(TRequest request, HttpRequestMessage message) @@ -348,7 +370,7 @@ private void CancelCall(Status status) if (!Channel.ThrowOperationCanceledOnCancellation) { - _metadataTcs.TrySetException(new RpcException(status)); + _metadataTcs.TrySetException(CreateRpcException(status)); } else { @@ -369,6 +391,12 @@ private void CancelCall(Status status) return null; } + internal RpcException CreateRpcException(Status status) + { + TryGetTrailers(out var trailers); + return new RpcException(status, trailers ?? Metadata.Empty); + } + private async ValueTask RunCall(HttpRequestMessage request) { using (StartScope()) @@ -473,17 +501,17 @@ private async ValueTask RunCall(HttpRequestMessage request) if (ex is OperationCanceledException) { status = (CallTask.IsCompletedSuccessfully) ? CallTask.Result : new Status(StatusCode.Cancelled, string.Empty); - resolvedException = Channel.ThrowOperationCanceledOnCancellation ? ex : new RpcException(status.Value); + resolvedException = Channel.ThrowOperationCanceledOnCancellation ? ex : CreateRpcException(status.Value); } else if (ex is RpcException rpcException) { status = rpcException.Status; - resolvedException = new RpcException(status.Value); + resolvedException = CreateRpcException(status.Value); } else { status = new Status(StatusCode.Internal, "Error starting gRPC call: " + ex.Message); - resolvedException = new RpcException(status.Value); + resolvedException = CreateRpcException(status.Value); } _metadataTcs.TrySetException(resolvedException); @@ -510,7 +538,7 @@ private void SetFailedResult(Status status) } else { - _responseTcs.TrySetException(new RpcException(status)); + _responseTcs.TrySetException(CreateRpcException(status)); } } @@ -526,7 +554,7 @@ public Exception CreateFailureStatusException(Status status) } else { - return new RpcException(status); + return CreateRpcException(status); } } @@ -721,18 +749,5 @@ private void DeadlineExceeded(object state) CancelCall(new Status(StatusCode.DeadlineExceeded, string.Empty)); } } - - private void ValidateTrailersAvailable() - { - // Response is finished - if (ResponseFinished) - { - return; - } - - // Throw InvalidOperationException here because documentation on GetTrailers says that - // InvalidOperationException is thrown if the call is not complete. - throw new InvalidOperationException("Can't get the call trailers because the call has not completed successfully."); - } } } diff --git a/src/Grpc.Net.Client/Internal/HttpContentClientStreamReader.cs b/src/Grpc.Net.Client/Internal/HttpContentClientStreamReader.cs index 796e4f9a5..def9cc7eb 100644 --- a/src/Grpc.Net.Client/Internal/HttpContentClientStreamReader.cs +++ b/src/Grpc.Net.Client/Internal/HttpContentClientStreamReader.cs @@ -85,7 +85,7 @@ public Task MoveNext(CancellationToken cancellationToken) } else { - return Task.FromException(new RpcException(status)); + return Task.FromException(_call.CreateRpcException(status)); } } diff --git a/test/FunctionalTests/Client/TrailerMetadataTests.cs b/test/FunctionalTests/Client/TrailerMetadataTests.cs new file mode 100644 index 000000000..72bddd034 --- /dev/null +++ b/test/FunctionalTests/Client/TrailerMetadataTests.cs @@ -0,0 +1,143 @@ +#region Copyright notice and license + +// Copyright 2019 The gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +using System.Linq; +using System.Threading.Tasks; +using Greet; +using Grpc.AspNetCore.FunctionalTests.Infrastructure; +using Grpc.Core; +using Grpc.Tests.Shared; +using NUnit.Framework; + +namespace Grpc.AspNetCore.FunctionalTests.Server +{ + [TestFixture] + public class TrailerMetadataTests : FunctionalTestBase + { + [Test] + public async Task GetTrailers_UnaryMethodSetStatusWithTrailers_TrailersAvailableInClient() + { + Task UnaryDeadlineExceeded(HelloRequest request, ServerCallContext context) + { + context.ResponseTrailers.Add(new Metadata.Entry("Name", "the value was empty")); + context.Status = new Status(StatusCode.InvalidArgument, "Validation failed"); + return Task.FromResult(new HelloReply()); + } + + // Arrange + SetExpectedErrorsFilter(writeContext => + { + if (writeContext.LoggerName == "Grpc.Net.Client.Internal.GrpcCall" && + writeContext.EventId.Name == "ErrorReadingMessage" && + writeContext.Message == "Error reading message.") + { + return true; + } + + if (writeContext.LoggerName == "Grpc.Net.Client.Internal.GrpcCall" && + writeContext.EventId.Name == "GrpcStatusError" && + writeContext.Message == "Call failed with gRPC error status. Status code: 'InvalidArgument', Message: 'Validation failed'.") + { + return true; + } + + return false; + }); + + var method = Fixture.DynamicGrpc.AddUnaryMethod(UnaryDeadlineExceeded); + + var channel = CreateChannel(); + + var client = TestClientFactory.Create(channel, method); + + // Act + var call = client.UnaryCall(new HelloRequest()); + + var ex = await ExceptionAssert.ThrowsAsync(() => call.ResponseAsync).DefaultTimeout(); + + // Assert + var trailers = call.GetTrailers(); + Assert.AreEqual(1, trailers.Count); + Assert.AreEqual("the value was empty", trailers.Single(m => m.Key == "name").Value); + + Assert.AreEqual(StatusCode.InvalidArgument, ex.StatusCode); + Assert.AreEqual("Validation failed", ex.Status.Detail); + Assert.AreEqual(1, ex.Trailers.Count); + Assert.AreEqual("the value was empty", ex.Trailers.Single(m => m.Key == "name").Value); + } + + [Test] + public async Task GetTrailers_UnaryMethodThrowsExceptionWithTrailers_TrailersAvailableInClient() + { + Task UnaryDeadlineExceeded(HelloRequest request, ServerCallContext context) + { + var trailers = new Metadata(); + trailers.Add(new Metadata.Entry("Name", "the value was empty")); + return Task.FromException(new RpcException(new Status(StatusCode.InvalidArgument, "Validation failed"), trailers)); + } + + // Arrange + SetExpectedErrorsFilter(writeContext => + { + if (writeContext.LoggerName == "Grpc.Net.Client.Internal.GrpcCall" && + writeContext.EventId.Name == "ErrorReadingMessage" && + writeContext.Message == "Error reading message.") + { + return true; + } + + if (writeContext.LoggerName == "Grpc.Net.Client.Internal.GrpcCall" && + writeContext.EventId.Name == "GrpcStatusError" && + writeContext.Message == "Call failed with gRPC error status. Status code: 'InvalidArgument', Message: 'Validation failed'.") + { + return true; + } + + if (writeContext.LoggerName == "SERVER Grpc.AspNetCore.Server.ServerCallHandler" && + writeContext.EventId.Name == "RpcConnectionError" && + writeContext.Message == "Error status code 'InvalidArgument' raised.") + { + return true; + } + + return false; + }); + + var method = Fixture.DynamicGrpc.AddUnaryMethod(UnaryDeadlineExceeded); + + var channel = CreateChannel(); + + var client = TestClientFactory.Create(channel, method); + + // Act + var call = client.UnaryCall(new HelloRequest()); + + var ex = await ExceptionAssert.ThrowsAsync(() => call.ResponseAsync).DefaultTimeout(); + + // Assert + var trailers = call.GetTrailers(); + Assert.GreaterOrEqual(trailers.Count, 1); + Assert.AreEqual("the value was empty", trailers.Single(m => m.Key == "name").Value); + + Assert.AreEqual(StatusCode.InvalidArgument, ex.StatusCode); + Assert.AreEqual("Validation failed", ex.Status.Detail); + Assert.GreaterOrEqual(ex.Trailers.Count, 1); + Assert.AreEqual("the value was empty", ex.Trailers.Single(m => m.Key == "name").Value); + } + } +} diff --git a/testassets/InteropTestsWebsite/Program.cs b/testassets/InteropTestsWebsite/Program.cs index 1448d7700..b841b3528 100644 --- a/testassets/InteropTestsWebsite/Program.cs +++ b/testassets/InteropTestsWebsite/Program.cs @@ -41,8 +41,8 @@ public static IHostBuilder CreateHostBuilder(string[] args) => { // Support --port and --use_tls cmdline arguments normally supported // by gRPC interop servers. - int port = context.Configuration.GetValue("port", 50052); - bool useTls = context.Configuration.GetValue("use_tls", false); + var port = context.Configuration.GetValue("port", 50052); + var useTls = context.Configuration.GetValue("use_tls", false); options.Limits.MinRequestBodyDataRate = null; options.ListenAnyIP(port, listenOptions =>