Fix missing details when aggregate exception is filtered out

CHANGELOG.md

@@ -15,6 +15,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))
 
 ## 3.21.0
 

src/Sentry/Internal/MainExceptionProcessor.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using Sentry.Extensibility;
+using Sentry.Internal.Extensions;
 using Sentry.Protocol;
 
 namespace Sentry.Internal
@@ -24,9 +25,7 @@ public void Process(Exception exception, SentryEvent sentryEvent)
         {
             _options.LogDebug("Running processor on exception: {0}", exception.Message);
 
-            var sentryExceptions = CreateSentryException(exception)
-                // Otherwise realization happens on the worker thread before sending event.
-                .ToList();
+            var sentryExceptions = CreateSentryExceptions(exception);
 
             MoveExceptionExtrasToEvent(sentryEvent, sentryExceptions);
 
@@ -80,18 +79,34 @@ keyValue.Value is string tagValue &&
             }
         }
 
-        internal IEnumerable<SentryException> CreateSentryException(Exception exception)
+        internal List<SentryException> CreateSentryExceptions(Exception exception)
         {
-            return exception.EnumerateChainedExceptions(_options)
-                .Select(BuildSentryException);
+            var exceptions = exception
+                .EnumerateChainedExceptions(_options)
+                .Select(BuildSentryException)
+                .ToList();
+
+            // If we've filtered out the aggregate exception, we'll need to copy over details from it.
+            if (exception is AggregateException && !_options.KeepAggregateException)
+            {
+                var original = BuildSentryException(exception);
+
+                // Exceptions are sent from oldest to newest, so the details belong on the LAST exception.
+                var last = exceptions.Last();
+                last.Stacktrace = original.Stacktrace;
+                last.Mechanism = original.Mechanism;
+                original.Data.TryCopyTo(last.Data);
+            }
+
+            return exceptions;
         }
 
         private SentryException BuildSentryException(Exception innerException)
         {
             var sentryEx = new SentryException
             {
-                Type = innerException.GetType()?.FullName,
-                Module = innerException.GetType()?.Assembly?.FullName,
+                Type = innerException.GetType().FullName,
+                Module = innerException.GetType().Assembly.FullName,
                 Value = innerException.Message,
                 ThreadId = Environment.CurrentManagedThreadId,
                 Mechanism = GetMechanism(innerException)

test/Sentry.Tests/Internals/MainExceptionProcessorTests.CreateSentryException_Aggregate.verified.txt

@@ -7,6 +7,12 @@
   {
     Type: System.Exception,
     Value: Inner message2,
-    Mechanism: {}
+    Mechanism: {
+      Type: AppDomain.UnhandledException,
+      Handled: false
+    },
+    Data: {
+      foo: bar
+    }
   }
 ]

test/Sentry.Tests/Internals/MainExceptionProcessorTests.CreateSentryException_Aggregate_Keep.verified.txt

@@ -12,6 +12,12 @@
   {
     Type: System.AggregateException,
     Value: ,
-    Mechanism: {}
+    Mechanism: {
+      Type: AppDomain.UnhandledException,
+      Handled: false
+    },
+    Data: {
+      foo: bar
+    }
   }
 ]

test/Sentry.Tests/Internals/MainExceptionProcessorTests.cs

@@ -49,7 +49,7 @@ public void Process_EventAndExceptionHaveExtra_DataCombined()
         sut.Process(ex, evt);
 
         Assert.Equal(2, evt.Extra.Count);
-        Assert.Contains(evt.Extra, p => p.Key == expectedKey && (int)p.Value == expectedValue);
+        Assert.Contains(evt.Extra, p => p.Key == expectedKey && p.Value is expectedValue);
     }
 
     [Fact]
@@ -144,7 +144,7 @@ public void CreateSentryException_DataHasObjectAsKey_ItemIgnored()
             Data = { [new object()] = new object() }
         };
 
-        var actual = sut.CreateSentryException(ex);
+        var actual = sut.CreateSentryExceptions(ex);
 
         Assert.Empty(actual.Single().Data);
     }
@@ -156,7 +156,7 @@ public Task CreateSentryException_Aggregate()
         var sut = _fixture.GetSut();
         var aggregateException = BuildAggregateException();
 
-        var sentryException = sut.CreateSentryException(aggregateException);
+        var sentryException = sut.CreateSentryExceptions(aggregateException);
 
         return Verify(sentryException);
     }
@@ -169,17 +169,60 @@ public Task CreateSentryException_Aggregate_Keep()
         var sut = _fixture.GetSut();
         var aggregateException = BuildAggregateException();
 
-        var sentryException = sut.CreateSentryException(aggregateException);
+        var sentryException = sut.CreateSentryExceptions(aggregateException);
 
         return Verify(sentryException)
             .ScrubLines(x => x.Contains("One or more errors occurred"));
     }
 
+    [Fact]
+    public void Process_AggregateException()
+    {
+        var sut = _fixture.GetSut();
+        _fixture.SentryStackTraceFactory = _fixture.SentryOptions.SentryStackTraceFactory;
+        var evt = new SentryEvent();
+        sut.Process(BuildAggregateException(), evt);
+
+        var last = evt.SentryExceptions!.Last();
+        Assert.NotNull(last.Stacktrace);
+        Assert.False(last.Mechanism?.Handled);
+        Assert.NotNull(last.Mechanism?.Type);
+        Assert.NotEmpty(last.Data);
+    }
+
+    [Fact]
+    public void Process_AggregateException_Keep()
+    {
+        _fixture.SentryOptions.KeepAggregateException = true;
+        _fixture.SentryStackTraceFactory = _fixture.SentryOptions.SentryStackTraceFactory;
+        var sut = _fixture.GetSut();
+        var evt = new SentryEvent();
+        sut.Process(BuildAggregateException(), evt);
+
+        var last = evt.SentryExceptions!.Last();
+        Assert.NotNull(last.Stacktrace);
+        Assert.False(last.Mechanism?.Handled);
+        Assert.NotNull(last.Mechanism?.Type);
+        Assert.NotEmpty(last.Data);
+    }
+
     private static AggregateException BuildAggregateException()
     {
-        return new AggregateException(
-            new Exception("Inner message1"),
-            new Exception("Inner message2"));
+        try
+        {
+            // Throwing will put a stack trace on the exception
+            throw new AggregateException(
+                new Exception("Inner message1"),
+                new Exception("Inner message2"));
+        }
+        catch (AggregateException ex)
+        {
+            // Add extra data to test fully
+            ex.Data[Mechanism.HandledKey] = false;
+            ex.Data[Mechanism.MechanismKey] = "AppDomain.UnhandledException";
+            ex.Data["foo"] = "bar";
+            return ex;
+        }
     }
 
     [Fact]
@@ -208,9 +251,9 @@ public void Process_HasInvalidExceptionTagsValue_TagsSetWithInvalidValue()
     {
         //Assert
         var sut = _fixture.GetSut();
-        var InvalidTag = new KeyValuePair<string, int>("Tag1", 1234);
-        var InvalidTag2 = new KeyValuePair<string, int?>("Tag2", null);
-        var InvalidTag3 = new KeyValuePair<string, string>("", "abcd");
+        var invalidTag = new KeyValuePair<string, int>("Tag1", 1234);
+        var invalidTag2 = new KeyValuePair<string, int?>("Tag2", null);
+        var invalidTag3 = new KeyValuePair<string, string>("", "abcd");
 
         var expectedTag = new KeyValuePair<string, object>("Exception[0][sentry:tag:Tag1]", 1234);
         var expectedTag2 = new KeyValuePair<string, object>("Exception[0][sentry:tag:Tag2]", null);
@@ -220,9 +263,9 @@ public void Process_HasInvalidExceptionTagsValue_TagsSetWithInvalidValue()
         var evt = new SentryEvent();
 
         //Act
-        ex.Data.Add($"{MainExceptionProcessor.ExceptionDataTagKey}{InvalidTag.Key}", InvalidTag.Value);
-        ex.Data.Add($"{MainExceptionProcessor.ExceptionDataTagKey}{InvalidTag2.Key}", InvalidTag2.Value);
-        ex.AddSentryTag(InvalidTag3.Key, InvalidTag3.Value);
+        ex.Data.Add($"{MainExceptionProcessor.ExceptionDataTagKey}{invalidTag.Key}", invalidTag.Value);
+        ex.Data.Add($"{MainExceptionProcessor.ExceptionDataTagKey}{invalidTag2.Key}", invalidTag2.Value);
+        ex.AddSentryTag(invalidTag3.Key, invalidTag3.Value);
 
         sut.Process(ex, evt);
 
@@ -295,18 +338,18 @@ public void Process_HasUnsupportedExceptionValue_ValueSetAsExtra()
     {
         //Assert
         var sut = _fixture.GetSut();
-        var InvalidData = new KeyValuePair<string, string>("sentry:attachment:filename", "./path");
-        var InvalidData2 = new KeyValuePair<string, int?>("sentry:unsupported:value", null);
+        var invalidData = new KeyValuePair<string, string>("sentry:attachment:filename", "./path");
+        var invalidData2 = new KeyValuePair<string, int?>("sentry:unsupported:value", null);
 
-        var expectedData = new KeyValuePair<string, object>($"Exception[0][{InvalidData.Key}]", InvalidData.Value);
-        var expectedData2 = new KeyValuePair<string, object>($"Exception[0][{InvalidData2.Key}]", InvalidData2.Value);
+        var expectedData = new KeyValuePair<string, object>($"Exception[0][{invalidData.Key}]", invalidData.Value);
+        var expectedData2 = new KeyValuePair<string, object>($"Exception[0][{invalidData2.Key}]", invalidData2.Value);
 
         var ex = new Exception();
         var evt = new SentryEvent();
 
         //Act
-        ex.Data.Add(InvalidData.Key, InvalidData.Value);
-        ex.Data.Add(InvalidData2.Key, InvalidData2.Value);
+        ex.Data.Add(invalidData.Key, invalidData.Value);
+        ex.Data.Add(invalidData2.Key, invalidData2.Value);
         sut.Process(ex, evt);
 
         //Assert