diff --git a/CHANGELOG.md b/CHANGELOG.md index 50a6b175dd..3eb5a38d76 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/Sentry/SentryClient.cs b/src/Sentry/SentryClient.cs index 8bd09dda8b..3ce545b653 100644 --- a/src/Sentry/SentryClient.cs +++ b/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? 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; + } + /// /// Capture an envelope and queue it. /// diff --git a/test/Sentry.Tests/SentryClientTests.cs b/test/Sentry.Tests/SentryClientTests.cs index feb2354701..24acebfbb8 100644 --- a/test/Sentry.Tests/SentryClientTests.cs +++ b/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(); + 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()); + } + + public static IEnumerable GetExceptionFilterTestCases() + { + var systemExceptionFilter = new ExceptionTypeFilter(); + var applicationExceptionFilter = new ExceptionTypeFilter(); + var aggregateExceptionFilter = new ExceptionTypeFilter(); // Filtered out for it's the exact filtered type - Assert.Equal(default, sut.CaptureException(new SystemException())); - _ = _fixture.BackgroundWorker.DidNotReceive().EnqueueEnvelope(Arg.Any()); + 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()); + 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()); + 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]