grpc / grpc-dotnet

gRPC for .NET
Apache License 2.0
4.17k stars 769 forks source link

Why is catch block called twice in Grpc Interceptor C#? #1895

Closed jangarach closed 2 years ago

jangarach commented 2 years ago

I ran into a problem when I started to implement a global error handler for the grpc server service. The problem is that when I get a validation error, I don't want to log it, but I want to return an RpcException with information to the client, and in other Exceptions I log it and return an unhandled error. The question is why do I repeatedly get into the catch block (Exception e) after I caught the ValidationException and threw the RpcException? To the fact that it is called twice, the logic I described above breaks down for me. The implementation is shown below:

public class ExceptionInterceptor : Interceptor
{
    private readonly ILogger<ExceptionInterceptor> _logger;

    public ExceptionInterceptor(ILogger<ExceptionInterceptor> logger)
    {
        _logger = logger;
    }

    public override async Task<TResponse> UnaryServerHandler<TRequest, TResponse>(
        TRequest request,
        ServerCallContext context,
        UnaryServerMethod<TRequest, TResponse> continuation)
    {
        try
        {
            return await continuation(request, context);
        }
        catch (ValidationException e)
        {
            var error = string.Join(Environment.NewLine, e.Errors.Select(p => p.ErrorMessage));
            throw new RpcException(new Status(StatusCode.InvalidArgument, error));
        }
        catch (Exception e)
        {
            _logger.LogError(e, "gRPC Exception");
            throw;
        }
    }
}

Interceptor registration:

services.AddGrpc(o =>
{
    o.Interceptors.Add<ExceptionInterceptor>();
});
jangarach commented 2 years ago

The reason was that when we register the interceptor globally, it fires twice. If we register it for a specific service, then it works once.The answer is:

services.AddGrpc()
    .AddServiceOptions<MyService>(o =>
    {
        o.Interceptors.Add<ExceptionInterceptor>();
    });
JamesNK commented 2 years ago

Thanks for investigating and providing an update. I'll look into why registering globally is behaving oddly.

JamesNK commented 2 years ago

I couldn't replicate this. When I register the interceptor and throw an error, the error is caught then rethrown as RpcException, and the other catch block does nothing.

Log output:

info: Microsoft.Hosting.Lifetime[14]
      Now listening on: https://localhost:5001
info: Microsoft.Hosting.Lifetime[0]
      Application started. Press Ctrl+C to shut down.
info: Microsoft.Hosting.Lifetime[0]
      Hosting environment: Development
info: Microsoft.Hosting.Lifetime[0]
      Content root path: C:\Development\Source\grpc-dotnet\examples\Reflector\Server
info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/2 POST https://localhost:5001/greet.Greeter/SayHello application/grpc -
info: Microsoft.AspNetCore.Routing.EndpointMiddleware[0]
      Executing endpoint 'gRPC - /greet.Greeter/SayHello'
info: Grpc.AspNetCore.Server.ServerCallHandler[7]
      Error status code 'InvalidArgument' with detail 'Validation failed' raised.
info: Microsoft.AspNetCore.Routing.EndpointMiddleware[1]
      Executed endpoint 'gRPC - /greet.Greeter/SayHello'
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished HTTP/2 POST https://localhost:5001/greet.Greeter/SayHello application/grpc - - 200 0 application/grpc 1748.5373ms

Changes to register interceptor: https://github.com/JamesNK/grpc-dotnet/commit/7fff9da6a06dcec170786a9bb98006235085c020

jangarach commented 2 years ago

Hey James. Yes, indeed, in your commit, I could not achieve such behavior as mine. When registering services, I use StashBox, if you try to register a service like mine, then you should have this behavior:

var builder = WebApplication.CreateBuilder(args);
builder.Host
    .UseStashbox()
    .ConfigureServices(ConfigureServices);

var app = builder.Build();
app.MapGrpcService<GreeterService>();

app.Run();

static void ConfigureServices(HostBuilderContext context, IServiceCollection services)
{
    services.AddGrpc(o => o.Interceptors.Add<ExceptionInterceptor>());
}

I am using package: Stashbox.Extensions.Hosting 4.2.3 I kept it minimal for the sake of the example, but Stashbox is used for a more complex situation. If this is not within the scope of your work, then I will independently study this issue. In any case, thank you for helping to find the cause.

JamesNK commented 2 years ago

It sounds like a bug in Stashbox. I don't know anything about that.