Merge pull request #886 from Cysharp/feature/CleanupFilterCallStack

Hide internal filter method calls from stack trace
Cysharp · Dec 27, 2024 · 35c5c7b · 35c5c7b
2 parents 5bf0639 + ac0bf01
commit 35c5c7b
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 6 deletions.
diff --git a/src/MagicOnion.Server/Filters/Internal/FilterHelper.cs b/src/MagicOnion.Server/Filters/Internal/FilterHelper.cs
@@ -1,3 +1,4 @@
+using System.Diagnostics;
 using System.Reflection;
 using MagicOnion.Server.Hubs;
 
@@ -69,28 +70,30 @@ IMagicOnionFilterFactory<IStreamingHubFilter> filterFactory
 
     public static Func<ServiceContext, ValueTask> WrapMethodBodyWithFilter(IServiceProvider serviceProvider, IEnumerable<MagicOnionServiceFilterDescriptor> filters, Func<ServiceContext, ValueTask> methodBody)
     {
-        Func<ServiceContext, ValueTask> next = methodBody;
+        Func<ServiceContext, ValueTask> outer = methodBody;
 
         foreach (var filterDescriptor in filters.Reverse())
         {
             var newFilter = CreateOrGetInstance(serviceProvider, filterDescriptor);
-            next = new InvokeHelper<ServiceContext, Func<ServiceContext, ValueTask>>(newFilter.Invoke, next).GetDelegate();
+            var inner = outer;
+            outer = [StackTraceHidden] (ctx) => newFilter.Invoke(ctx, inner);
         }
 
-        return next;
+        return outer;
     }
 
     public static Func<StreamingHubContext, ValueTask> WrapMethodBodyWithFilter(IServiceProvider serviceProvider, IEnumerable<StreamingHubFilterDescriptor> filters, Func<StreamingHubContext, ValueTask> methodBody)
     {
-        Func<StreamingHubContext, ValueTask> next = methodBody;
+        Func<StreamingHubContext, ValueTask> outer = methodBody;
 
         foreach (var filterDescriptor in filters.Reverse())
         {
             var newFilter = CreateOrGetInstance(serviceProvider, filterDescriptor);
-            next = new InvokeHelper<StreamingHubContext, Func<StreamingHubContext, ValueTask>>(newFilter.Invoke, next).GetDelegate();
+            var inner = outer;
+            outer = [StackTraceHidden] (ctx) => newFilter.Invoke(ctx, inner);
         }
 
-        return next;
+        return outer;
     }
 
     public static TFilter CreateOrGetInstance<TFilter>(IServiceProvider serviceProvider, MagicOnionFilterDescriptor<TFilter> descriptor)

diff --git a/tests/MagicOnion.Server.Tests/Filter/FilterHelperTest.cs b/tests/MagicOnion.Server.Tests/Filter/FilterHelperTest.cs
@@ -745,6 +745,39 @@ public async Task WrapMethodBodyWithFilter_Surround_Service()
         results.Should().Equal(1, 2, 0, 200, 100);
     }
 
+    [Fact]
+    public async Task WrapMethodBodyWithFilter_Surround_Service_ThrowStackTrace()
+    {
+        // Arrange
+        var services = new ServiceCollection();
+        var serviceProvider = services.BuildServiceProvider();
+        var callStack = default(string);
+        var filters = new[]
+        {
+            new MagicOnionServiceFilterDescriptor(new DelegateServiceFilter(async (context, next) =>
+            {
+                await next(context);
+            })),
+            new MagicOnionServiceFilterDescriptor(new DelegateServiceFilter(async (context, next) =>
+            {
+                await next(context);
+            })),
+        };
+
+        // Act
+        var body = FilterHelper.WrapMethodBodyWithFilter(serviceProvider, filters, (context) =>
+        {
+            callStack = Environment.StackTrace;
+            throw new InvalidOperationException();
+        });
+        var ex = await Record.ExceptionAsync(async () => await body(default));
+
+        // Assert
+        ex.Should().NotBeNull();
+        ex.StackTrace.Should().NotContain("MagicOnion.Server.Filters.Internal.FilterHelper.<>c__DisplayClass");
+        callStack.Should().NotContain("MagicOnion.Server.Filters.Internal.FilterHelper.<>c__DisplayClass");
+    }
+
     class DelegateServiceFilter : IMagicOnionServiceFilter
     {
         readonly Func<ServiceContext, Func<ServiceContext, ValueTask>, ValueTask> func;
@@ -793,6 +826,39 @@ public async Task WrapMethodBodyWithFilter_Surround_StreamingHub()
         results.Should().Equal(1, 2, 0, 200, 100);
     }
 
+    [Fact]
+    public async Task WrapMethodBodyWithFilter_Surround_StreamingHub_ThrowStackTrace()
+    {
+        // Arrange
+        var services = new ServiceCollection();
+        var serviceProvider = services.BuildServiceProvider();
+        var callStack = default(string);
+        var filters = new[]
+        {
+            new StreamingHubFilterDescriptor(new DelegateHubFilter(async (context, next) =>
+            {
+                await next(context);
+            })),
+            new StreamingHubFilterDescriptor(new DelegateHubFilter(async (context, next) =>
+            {
+                await next(context);
+            })),
+        };
+
+        // Act
+        var body = FilterHelper.WrapMethodBodyWithFilter(serviceProvider, filters, (context) =>
+        {
+            callStack = Environment.StackTrace;
+            throw new InvalidOperationException();
+        });
+        var ex = await Record.ExceptionAsync(async () => await body(default));
+
+        // Assert
+        ex.Should().NotBeNull();
+        ex.StackTrace.Should().NotContain("MagicOnion.Server.Filters.Internal.FilterHelper.<>c__DisplayClass");
+        callStack.Should().NotContain("MagicOnion.Server.Filters.Internal.FilterHelper.<>c__DisplayClass");
+    }
+
     class DelegateHubFilter : IStreamingHubFilter
     {
         readonly Func<StreamingHubContext, Func<StreamingHubContext, ValueTask>, ValueTask> func;