Exception filters should consider child exceptions of an AggregateException

CHANGELOG.md

@@ -16,6 +16,7 @@
 - Reduce lock contention when sampling ([#1915](https://github.com/getsentry/sentry-dotnet/pull/1915))
 - Dont send transaction for OPTIONS web request ([#1921](https://github.com/getsentry/sentry-dotnet/pull/1921))
 - Fix missing details when aggregate exception is filtered out ([#1922](https://github.com/getsentry/sentry-dotnet/pull/1922))
+- Exception filters should consider child exceptions of an `AggregateException` ([#1924](https://github.com/getsentry/sentry-dotnet/pull/1924))
 
 ## 3.21.0
 

src/Sentry/SentryClient.cs

@@ -168,15 +168,13 @@ private SentryId DoSendEvent(SentryEvent @event, Scope? scope)
                 }
             }
 
-            if (@event.Exception != null && _options.ExceptionFilters?.Count > 0)
+            var filteredExceptions = ApplyExceptionFilters(@event.Exception);
+            if (filteredExceptions?.Count > 0)
             {
-                if (_options.ExceptionFilters.Any(f => f.Filter(@event.Exception)))
-                {
-                    _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Error);
-                    _options.LogInfo("Event with exception of type '{0}' was dropped by an exception filter.",
-                        @event.Exception.GetType());
-                    return SentryId.Empty;
-                }
+                _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Error);
+                _options.LogInfo("Event was dropped by one or more exception filters for exception(s): {0}",
+                    string.Join(", ", filteredExceptions.Select(e => e.GetType()).Distinct()));
+                return SentryId.Empty;
             }
 
             scope ??= new Scope(_options);
@@ -232,6 +230,34 @@ private SentryId DoSendEvent(SentryEvent @event, Scope? scope)
                 : SentryId.Empty;
         }
 
+        private IReadOnlyCollection<Exception>? ApplyExceptionFilters(Exception? exception)
+        {
+            var filters = _options.ExceptionFilters;
+            if (exception == null || filters == null || filters.Count == 0)
+            {
+                // There was nothing to filter.
+                return null;
+            }
+
+            if (filters.Any(f => f.Filter(exception)))
+            {
+                // The event should be filtered based on the given exception
+                return new[] {exception};
+            }
+
+            if (exception is AggregateException aggregate &&
+                aggregate.InnerExceptions.All(e => ApplyExceptionFilters(e) != null))
+            {
+                // All inner exceptions of the aggregate matched a filter, so the event should be filtered.
+                // Note that _options.KeepAggregateException is not relevant here.  Even if we want to keep aggregate
+                // exceptions, we would still never send one if all of its children are supposed to be filtered.
+                return aggregate.InnerExceptions;
+            }
+
+            // The event should not be filtered.
+            return null;
+        }
+
         /// <summary>
         /// Capture an envelope and queue it.
         /// </summary>

test/Sentry.Tests/SentryClientTests.cs

@@ -41,24 +41,92 @@ public SentryClientTests(ITestOutputHelper output)
         _output = output;
     }
 
-    [Fact]
-    public void CaptureEvent_ExceptionFiltered_EmptySentryId()
+    [Theory]
+    [MemberData(nameof(GetExceptionFilterTestCases))]
+    public void CaptureEvent_ExceptionFilteredForType(bool shouldFilter, Exception exception, params IExceptionFilter[] filters)
     {
-        _fixture.SentryOptions.AddExceptionFilterForType<SystemException>();
+        foreach (var filter in filters)
+        {
+            _fixture.SentryOptions.AddExceptionFilter(filter);
+        }
 
         var sut = _fixture.GetSut();
+        var result = sut.CaptureException(exception);
+
+        Assert.Equal(shouldFilter, result == default);
+        _fixture.BackgroundWorker.Received(result == default ? 0 : 1).EnqueueEnvelope(Arg.Any<Envelope>());
+    }
+
+    public static IEnumerable<object[]> GetExceptionFilterTestCases()
+    {
+        var systemExceptionFilter = new ExceptionTypeFilter<SystemException>();
+        var applicationExceptionFilter = new ExceptionTypeFilter<ApplicationException>();
+        var aggregateExceptionFilter = new ExceptionTypeFilter<AggregateException>();
 
         // Filtered out for it's the exact filtered type
-        Assert.Equal(default, sut.CaptureException(new SystemException()));
-        _ = _fixture.BackgroundWorker.DidNotReceive().EnqueueEnvelope(Arg.Any<Envelope>());
+        yield return new object[]
+        {
+            true,
+            new SystemException(),
+            systemExceptionFilter
+        };
 
         // Filtered for it's a derived type
-        Assert.Equal(default, sut.CaptureException(new ArithmeticException()));
-        _ = _fixture.BackgroundWorker.DidNotReceive().EnqueueEnvelope(Arg.Any<Envelope>());
+        yield return new object[]
+        {
+            true,
+            new ArithmeticException(),
+            systemExceptionFilter
+        };
 
         // Not filtered since it's not in the inheritance chain
-        Assert.NotEqual(default, sut.CaptureException(new Exception()));
-        _ = _fixture.BackgroundWorker.Received(1).EnqueueEnvelope(Arg.Any<Envelope>());
+        yield return new object[]
+        {
+            false,
+            new Exception(),
+            systemExceptionFilter
+        };
+
+        // Filtered because it's the only exception under an aggregate exception
+        yield return new object[]
+        {
+            true,
+            new AggregateException(new SystemException()),
+            systemExceptionFilter
+        };
+
+        // Filtered because all exceptions under the aggregate exception are the filtered or derived type
+        yield return new object[]
+        {
+            true,
+            new AggregateException(new SystemException(), new ArithmeticException()),
+            systemExceptionFilter
+        };
+
+        // Filtered because all exceptions under the aggregate exception are covered by all of the filters
+        yield return new object[]
+        {
+            true,
+            new AggregateException(new SystemException(), new ApplicationException()),
+            systemExceptionFilter,
+            applicationExceptionFilter
+        };
+
+        // Not filtered because there's an exception under the aggregate not covered by the filters
+        yield return new object[]
+        {
+            false,
+            new AggregateException(new SystemException(), new Exception()),
+            systemExceptionFilter
+        };
+
+        // Filtered because we're specifically filtering out aggregate exceptions (strange, but should work)
+        yield return new object[]
+        {
+            true,
+            new AggregateException(),
+            aggregateExceptionFilter
+        };
     }
 
     [Fact]