[release/7.0] Cleanup and tests around transcoding error handlingjamesnk/grpctranscoding-errorhandling

4 files changed, 128 insertions, 8 deletions

diff --git a/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/GrpcServerLog.cs b/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/GrpcServerLog.cs
index 3678496f0f..1805574187 100644
--- a/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/GrpcServerLog.cs
+++ b/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/GrpcServerLog.cs
@@ -15,7 +15,7 @@ internal static partial class GrpcServerLog
     public static partial void ErrorExecutingServiceMethod(ILogger logger, string serviceMethod, Exception ex);
 
     [LoggerMessage(3, LogLevel.Information, "Error status code '{StatusCode}' with detail '{Detail}' raised.", EventName = "RpcConnectionError")]
-    public static partial void RpcConnectionError(ILogger logger, StatusCode statusCode, string detail);
+    public static partial void RpcConnectionError(ILogger logger, StatusCode statusCode, string detail, Exception? debugException);
 
     [LoggerMessage(4, LogLevel.Debug, "Reading message.", EventName = "ReadingMessage")]
     public static partial void ReadingMessage(ILogger logger);
diff --git a/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/JsonTranscodingServerCallContext.cs b/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/JsonTranscodingServerCallContext.cs
index c99f446354..a90e192510 100644
--- a/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/JsonTranscodingServerCallContext.cs
+++ b/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/JsonTranscodingServerCallContext.cs
@@ -104,8 +104,9 @@ internal sealed class JsonTranscodingServerCallContext : ServerCallContext, ISer
         if (ex is RpcException rpcException)
         {
             // RpcException is thrown by client code to modify the status returned from the server.
-            // Log the status and detail. Don't log the exception to reduce log verbosity.
-            GrpcServerLog.RpcConnectionError(Logger, rpcException.StatusCode, rpcException.Status.Detail);
+            // Log the status, detail and debug exception (if present).
+            // Don't log the RpcException itself to reduce log verbosity. All of its information is already captured.
+            GrpcServerLog.RpcConnectionError(Logger, rpcException.StatusCode, rpcException.Status.Detail, rpcException.Status.DebugException);
 
             status = rpcException.Status;
         }
diff --git a/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ServerStreamingServerCallHandlerTests.cs b/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ServerStreamingServerCallHandlerTests.cs
index a6dc6c43c6..a64ce87d2b 100644
--- a/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ServerStreamingServerCallHandlerTests.cs
+++ b/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ServerStreamingServerCallHandlerTests.cs
@@ -109,6 +109,53 @@ public class ServerStreamingServerCallHandlerTests : LoggedTest
         Assert.Equal("Exception was thrown by handler.", responseJson2.RootElement.GetProperty("error").GetString());
         Assert.Equal(2, responseJson2.RootElement.GetProperty("code").GetInt32());
 
+        var exceptionWrite = TestSink.Writes.Single(w => w.EventId.Name == "ErrorExecutingServiceMethod");
+        Assert.Equal("Error when executing service method 'TestMethodName'.", exceptionWrite.Message);
+        Assert.Equal("Exception!", exceptionWrite.Exception.Message);
+
+        await callTask.DefaultTimeout();
+    }
+
+    [Fact]
+    public async Task HandleCallAsync_MessageThenRpcException_MessageThenErrorReturned()
+    {
+        // Arrange
+        var debugException = new Exception("Error!");
+        ServerStreamingServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = async (s, r, w, c) =>
+        {
+            await w.WriteAsync(new HelloReply { Message = $"Hello {r.Name} 1" });
+            throw new RpcException(new Status(StatusCode.Aborted, "Detail!", debugException));
+        };
+
+        var pipe = new Pipe();
+
+        var routeParameterDescriptors = new Dictionary<string, List<FieldDescriptor>>
+        {
+            ["name"] = new List<FieldDescriptor>(new[] { HelloRequest.Descriptor.FindFieldByNumber(HelloRequest.NameFieldNumber) })
+        };
+        var descriptorInfo = TestHelpers.CreateDescriptorInfo(routeParameterDescriptors: routeParameterDescriptors);
+        var callHandler = CreateCallHandler(invoker, descriptorInfo: descriptorInfo);
+        var httpContext = TestHelpers.CreateHttpContext(bodyStream: pipe.Writer.AsStream());
+        httpContext.Request.RouteValues["name"] = "TestName!";
+
+        // Act
+        var callTask = callHandler.HandleCallAsync(httpContext);
+
+        // Assert
+        var line1 = await ReadLineAsync(pipe.Reader).DefaultTimeout();
+        using var responseJson1 = JsonDocument.Parse(line1!);
+        Assert.Equal("Hello TestName! 1", responseJson1.RootElement.GetProperty("message").GetString());
+
+        var line2 = await ReadLineAsync(pipe.Reader).DefaultTimeout();
+        using var responseJson2 = JsonDocument.Parse(line2!);
+        Assert.Equal("Detail!", responseJson2.RootElement.GetProperty("message").GetString());
+        Assert.Equal("Detail!", responseJson2.RootElement.GetProperty("error").GetString());
+        Assert.Equal((int)StatusCode.Aborted, responseJson2.RootElement.GetProperty("code").GetInt32());
+
+        var exceptionWrite = TestSink.Writes.Single(w => w.EventId.Name == "RpcConnectionError");
+        Assert.Equal("Error status code 'Aborted' with detail 'Detail!' raised.", exceptionWrite.Message);
+        Assert.Equal(debugException, exceptionWrite.Exception);
+
         await callTask.DefaultTimeout();
     }
 
@@ -143,6 +190,10 @@ public class ServerStreamingServerCallHandlerTests : LoggedTest
         Assert.Equal("Exception was thrown by handler. Exception: Exception!", responseJson.RootElement.GetProperty("error").GetString());
         Assert.Equal(2, responseJson.RootElement.GetProperty("code").GetInt32());
 
+        var exceptionWrite = TestSink.Writes.Single(w => w.EventId.Name == "ErrorExecutingServiceMethod");
+        Assert.Equal("Error when executing service method 'TestMethodName'.", exceptionWrite.Message);
+        Assert.Equal("Exception!", exceptionWrite.Exception.Message);
+
         await callTask.DefaultTimeout();
     }
 
diff --git a/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/UnaryServerCallHandlerTests.cs b/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/UnaryServerCallHandlerTests.cs
index 722a9410eb..10d3e35cc3 100644
--- a/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/UnaryServerCallHandlerTests.cs
+++ b/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/UnaryServerCallHandlerTests.cs
@@ -548,9 +548,10 @@ public class UnaryServerCallHandlerTests : LoggedTest
     public async Task HandleCallAsync_RpcExceptionThrown_StatusReturned()
     {
         // Arrange
+        var debugException = new Exception("Error!");
         UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = (s, r, c) =>
         {
-            throw new RpcException(new Status(StatusCode.Unauthenticated, "Detail!"), "Message!");
+            throw new RpcException(new Status(StatusCode.Unauthenticated, "Detail!", debugException), "Message!");
         };
 
         var unaryServerCallHandler = CreateCallHandler(invoker);
@@ -567,6 +568,70 @@ public class UnaryServerCallHandlerTests : LoggedTest
         Assert.Equal("Detail!", responseJson.RootElement.GetProperty("message").GetString());
         Assert.Equal("Detail!", responseJson.RootElement.GetProperty("error").GetString());
         Assert.Equal((int)StatusCode.Unauthenticated, responseJson.RootElement.GetProperty("code").GetInt32());
+
+        var exceptionWrite = TestSink.Writes.Single(w => w.EventId.Name == "RpcConnectionError");
+        Assert.Equal("Error status code 'Unauthenticated' with detail 'Detail!' raised.", exceptionWrite.Message);
+        Assert.Equal(debugException, exceptionWrite.Exception);
+    }
+
+    [Fact]
+    public async Task HandleCallAsync_OtherExceptionThrown_StatusReturned()
+    {
+        // Arrange
+        UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = (s, r, c) =>
+        {
+            throw new InvalidOperationException("Error!");
+        };
+
+        var unaryServerCallHandler = CreateCallHandler(invoker);
+        var httpContext = TestHelpers.CreateHttpContext();
+
+        // Act
+        await unaryServerCallHandler.HandleCallAsync(httpContext);
+
+        // Assert
+        Assert.Equal(500, httpContext.Response.StatusCode);
+
+        httpContext.Response.Body.Seek(0, SeekOrigin.Begin);
+        using var responseJson = JsonDocument.Parse(httpContext.Response.Body);
+        Assert.Equal("Exception was thrown by handler.", responseJson.RootElement.GetProperty("message").GetString());
+        Assert.Equal("Exception was thrown by handler.", responseJson.RootElement.GetProperty("error").GetString());
+        Assert.Equal((int)StatusCode.Unknown, responseJson.RootElement.GetProperty("code").GetInt32());
+
+        var exceptionWrite = TestSink.Writes.Single(w => w.EventId.Name == "ErrorExecutingServiceMethod");
+        Assert.Equal("Error when executing service method 'TestMethodName'.", exceptionWrite.Message);
+        Assert.Equal("Error!", exceptionWrite.Exception.Message);
+    }
+
+    [Fact]
+    public async Task HandleCallAsync_EnableDetailedErrors_OtherExceptionThrown_StatusReturned()
+    {
+        // Arrange
+        UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = (s, r, c) =>
+        {
+            throw new InvalidOperationException("Error!");
+        };
+
+        var unaryServerCallHandler = CreateCallHandler(
+            invoker,
+            serviceOptions: new GrpcServiceOptions { EnableDetailedErrors = true });
+        var httpContext = TestHelpers.CreateHttpContext();
+
+        // Act
+        await unaryServerCallHandler.HandleCallAsync(httpContext);
+
+        // Assert
+        Assert.Equal(500, httpContext.Response.StatusCode);
+
+        httpContext.Response.Body.Seek(0, SeekOrigin.Begin);
+        using var responseJson = JsonDocument.Parse(httpContext.Response.Body);
+        Assert.Equal("Exception was thrown by handler. InvalidOperationException: Error!", responseJson.RootElement.GetProperty("message").GetString());
+        Assert.Equal("Exception was thrown by handler. InvalidOperationException: Error!", responseJson.RootElement.GetProperty("error").GetString());
+        Assert.Equal((int)StatusCode.Unknown, responseJson.RootElement.GetProperty("code").GetInt32());
+
+        var exceptionWrite = TestSink.Writes.Single(w => w.EventId.Name == "ErrorExecutingServiceMethod");
+        Assert.Equal("Error when executing service method 'TestMethodName'.", exceptionWrite.Message);
+        Assert.Equal("Error!", exceptionWrite.Exception.Message);
     }
 
     [Fact]
@@ -1271,14 +1336,16 @@ public class UnaryServerCallHandlerTests : LoggedTest
         UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker,
         CallHandlerDescriptorInfo? descriptorInfo = null,
         List<(Type Type, object[] Args)>? interceptors = null,
-        GrpcJsonTranscodingOptions? jsonTranscodingOptions = null)
+        GrpcJsonTranscodingOptions? jsonTranscodingOptions = null,
+        GrpcServiceOptions? serviceOptions = null)
     {
         return CreateCallHandler(
             invoker,
             CreateServiceMethod("TestMethodName", HelloRequest.Parser, HelloReply.Parser),
             descriptorInfo,
             interceptors,
-            jsonTranscodingOptions);
+            jsonTranscodingOptions,
+            serviceOptions);
     }
 
     private UnaryServerCallHandler<JsonTranscodingGreeterService, TRequest, TResponse> CreateCallHandler<TRequest, TResponse>(
@@ -1286,11 +1353,12 @@ public class UnaryServerCallHandlerTests : LoggedTest
         Method<TRequest, TResponse> method,
         CallHandlerDescriptorInfo? descriptorInfo = null,
         List<(Type Type, object[] Args)>? interceptors = null,
-        GrpcJsonTranscodingOptions? jsonTranscodingOptions = null)
+        GrpcJsonTranscodingOptions? jsonTranscodingOptions = null,
+        GrpcServiceOptions? serviceOptions = null)
         where TRequest : class, IMessage<TRequest>
         where TResponse : class, IMessage<TResponse>
     {
-        var serviceOptions = new GrpcServiceOptions();
+        serviceOptions ??= new GrpcServiceOptions();
         if (interceptors != null)
         {
             foreach (var interceptor in interceptors)