make server exception interceptors more reusable via ServerExceptionsInterceptorBase

Author: mgravell

src/protobuf-net.Grpc/Configuration/SimpleRpcExceptionsInterceptor.cs

@@ -2,26 +2,25 @@
 using Grpc.Core.Interceptors;
 using System;
 using System.IO;
-using System.Runtime.CompilerServices;
 using System.Security;
 using System.Threading.Tasks;
 
 namespace ProtoBuf.Grpc.Configuration
 {
     /// <summary>
-    /// Indicates that a service or method should use simplified exception handling - which means that all server exceptions are treated as <see cref="RpcException"/>; this
-    /// will expose the <see cref="Exception.Message"/> to the caller (and the type may be interpreted as a <see cref="StatusCode"/> when possible), which should only be
-    /// done with caution as this may present security implications. Additional exception metadata (<see cref="Exception.Data"/>, <see cref="Exception.InnerException"/>,
-    /// <see cref="Exception.StackTrace"/>, etc) is not propagated. The exception is still exposed at the client as an <see cref="RpcException"/>.
+    /// A base interceptor that handles all server-side exceptions.
     /// </summary>
-    public sealed class SimpleRpcExceptionsInterceptor : Interceptor
+    public abstract class ServerExceptionsInterceptorBase : Interceptor
     {
-        private SimpleRpcExceptionsInterceptor() { }
-        private static SimpleRpcExceptionsInterceptor? s_Instance;
         /// <summary>
-        /// Provides a shared instance of this interceptor
+        /// Allows implementors to intercept exceptions, optionally re-exposing them as <see cref="RpcException"/>.
         /// </summary>
-        public static SimpleRpcExceptionsInterceptor Instance => s_Instance ??= new SimpleRpcExceptionsInterceptor();
+        /// <returns><c>true</c> if the exception should be re-exposed as an <see cref="RpcException"/>, <c>false</c> otherwise</returns>
+        protected virtual bool OnException(Exception exception, out Status status)
+        {
+            status = default;
+            return false;
+        }
 
         /// <inheritdoc/>
         public override async Task<TResponse> ClientStreamingServerHandler<TRequest, TResponse>(IAsyncStreamReader<TRequest> requestStream, ServerCallContext context, ClientStreamingServerMethod<TRequest, TResponse> continuation)
@@ -30,10 +29,9 @@ public override async Task<TResponse> ClientStreamingServerHandler<TRequest, TRe
             {
                 return await base.ClientStreamingServerHandler(requestStream, context, continuation).ConfigureAwait(false);
             }
-            catch (Exception ex) when (IsNotRpcException(ex))
+            catch (Exception ex) when (OnException(ex, out var status))
             {
-                RethrowAsRpcException(ex);
-                return default!; // make compiler happy
+                throw new RpcException(status, ex.Message);
             }
         }
 
@@ -44,9 +42,9 @@ public override async Task ServerStreamingServerHandler<TRequest, TResponse>(TRe
             {
                 await base.ServerStreamingServerHandler(request, responseStream, context, continuation).ConfigureAwait(false);
             }
-            catch (Exception ex) when (IsNotRpcException(ex))
+            catch (Exception ex) when (OnException(ex, out var status))
             {
-                RethrowAsRpcException(ex);
+                throw new RpcException(status, ex.Message);
             }
         }
 
@@ -57,9 +55,9 @@ public override async Task DuplexStreamingServerHandler<TRequest, TResponse>(IAs
             {
                 await base.DuplexStreamingServerHandler(requestStream, responseStream, context, continuation).ConfigureAwait(false);
             }
-            catch (Exception ex) when (IsNotRpcException(ex))
+            catch (Exception ex) when (OnException(ex, out var status))
             {
-                RethrowAsRpcException(ex);
+                throw new RpcException(status, ex.Message);
             }
         }
 
@@ -70,33 +68,56 @@ public override async Task<TResponse> UnaryServerHandler<TRequest, TResponse>(TR
             {
                 return await base.UnaryServerHandler(request, context, continuation).ConfigureAwait(false);
             }
-            catch (Exception ex) when (IsNotRpcException(ex))
+            catch (Exception ex) when (OnException(ex, out var status))
             {
-                RethrowAsRpcException(ex);
-                return default!; // make compiler happy
+                throw new RpcException(status, ex.Message);
             }
         }
+    }
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal static bool IsNotRpcException(Exception ex) => !(ex is RpcException);
+    /// <summary>
+    /// Indicates that a service or method should use simplified exception handling - which means that all server exceptions are treated as <see cref="RpcException"/>; this
+    /// will expose the <see cref="Exception.Message"/> to the caller (and the type may be interpreted as a <see cref="StatusCode"/> when possible), which should only be
+    /// done with caution as this may present security implications. Additional exception metadata (<see cref="Exception.Data"/>, <see cref="Exception.InnerException"/>,
+    /// <see cref="Exception.StackTrace"/>, etc) is not propagated. The exception is still exposed at the client as an <see cref="RpcException"/>.
+    /// </summary>
+    public class SimpleRpcExceptionsInterceptor : ServerExceptionsInterceptorBase
+    {
+        private SimpleRpcExceptionsInterceptor() { }
+        private static SimpleRpcExceptionsInterceptor? s_Instance;
+
+        /// <summary>
+        /// Provides a shared instance of this interceptor
+        /// </summary>
+        public static SimpleRpcExceptionsInterceptor Instance => s_Instance ??= new SimpleRpcExceptionsInterceptor();
 
-        internal static void RethrowAsRpcException(Exception ex)
+        internal static bool ShouldWrap(Exception exception, out Status status)
         {
-#pragma warning disable IDE0059 // needs more recent compiler than the CI server has
-            var code = ex switch
+            if (exception is RpcException)
+            {
+                status = default;
+                return false;
+            }
+            status = new Status(exception switch
             {
+#pragma warning disable IDE0059 // needs more recent compiler than the CI server has
                 OperationCanceledException a => StatusCode.Cancelled,
                 ArgumentException b => StatusCode.InvalidArgument,
                 NotImplementedException c => StatusCode.Unimplemented,
-                SecurityException d => StatusCode.PermissionDenied,
-                EndOfStreamException e => StatusCode.OutOfRange,
-                FileNotFoundException f => StatusCode.NotFound,
-                DirectoryNotFoundException g => StatusCode.NotFound,
-                TimeoutException h => StatusCode.DeadlineExceeded,
-                _ => StatusCode.Unknown,
-            };
+                NotSupportedException d => StatusCode.Unimplemented,
+                SecurityException e => StatusCode.PermissionDenied,
+                EndOfStreamException f => StatusCode.OutOfRange,
+                FileNotFoundException g => StatusCode.NotFound,
+                DirectoryNotFoundException h => StatusCode.NotFound,
+                TimeoutException i => StatusCode.DeadlineExceeded,
 #pragma warning restore IDE0059 // needs more recent compiler than the CI server has
-            throw new RpcException(new Status(code, ex.Message), ex.Message);
+                _ => StatusCode.Unknown,
+            }, exception.Message);
+            return true;
         }
+
+        /// <inheritdoc/>
+        protected override bool OnException(Exception exception, out Status status)
+            => ShouldWrap(exception, out status);
     }
 }

src/protobuf-net.Grpc/Internal/Reshape.cs

@@ -152,9 +152,9 @@ static async Task Awaited(Task task)
                 {
                     await task.ConfigureAwait(false);
                 }
-                catch (Exception ex) when (SimpleRpcExceptionsInterceptor.IsNotRpcException(ex))
+                catch (Exception ex) when (SimpleRpcExceptionsInterceptor.ShouldWrap(ex, out var status))
                 {
-                    SimpleRpcExceptionsInterceptor.RethrowAsRpcException(ex);
+                    throw new RpcException(status, ex.Message);
                 }
             }
         }
@@ -176,10 +176,9 @@ static async Task<T> Awaited(Task<T> task)
                 {
                     return await task.ConfigureAwait(false);
                 }
-                catch (Exception ex) when (SimpleRpcExceptionsInterceptor.IsNotRpcException(ex))
+                catch (Exception ex) when (SimpleRpcExceptionsInterceptor.ShouldWrap(ex, out var status))
                 {
-                    SimpleRpcExceptionsInterceptor.RethrowAsRpcException(ex);
-                    return default!; // make compiler happy
+                    throw new RpcException(status, ex.Message);
                 }
             }
         }