fix: EventProcessors not running for native crashes on iOS (#4318)

* fix: EventProcessors not running for native crashes on iOS

Resolves #3621:
- #3621

Author: jamescrosswell

CHANGELOG.md

@@ -12,6 +12,7 @@
 
 ### Fixes
 
+- Custom ISentryEventProcessors are now run for native iOS events ([#4318](https://github.com/getsentry/sentry-dotnet/pull/4318))
 - Crontab validation when capturing checkins ([#4314](https://github.com/getsentry/sentry-dotnet/pull/4314))
 
 ### Dependencies

src/Sentry/Internal/SentryEventHelper.cs

@@ -0,0 +1,76 @@
+using Sentry.Extensibility;
+
+namespace Sentry.Internal;
+
+internal static class SentryEventHelper
+{
+    public static SentryEvent? ProcessEvent(SentryEvent? evt, IEnumerable<ISentryEventProcessor> processors, SentryHint? hint, SentryOptions options)
+    {
+        if (evt == null)
+        {
+            return evt;
+        }
+
+        var processedEvent = evt;
+        var effectiveHint = hint ?? new SentryHint(options);
+
+        foreach (var processor in processors)
+        {
+            processedEvent = processor.DoProcessEvent(processedEvent, effectiveHint);
+            if (processedEvent == null)
+            {
+                options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Error);
+                options.LogInfo("Event dropped by processor {0}", processor.GetType().Name);
+                break;
+            }
+        }
+        return processedEvent;
+    }
+
+#if NET6_0_OR_GREATER
+    [UnconditionalSuppressMessage("Trimming", "IL2026: RequiresUnreferencedCode", Justification = AotHelper.AvoidAtRuntime)]
+#endif
+    public static SentryEvent? DoBeforeSend(SentryEvent? @event, SentryHint hint, SentryOptions options)
+    {
+        if (@event is null || options.BeforeSendInternal is null)
+        {
+            return @event;
+        }
+
+        options.LogDebug("Calling the BeforeSend callback");
+        try
+        {
+            @event = options.BeforeSendInternal?.Invoke(@event, hint);
+            if (@event == null) // Rejected event
+            {
+                options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Error);
+                options.LogInfo("Event dropped by BeforeSend callback.");
+            }
+        }
+        catch (Exception e)
+        {
+            if (!AotHelper.IsTrimmed)
+            {
+                // Attempt to demystify exceptions before adding them as breadcrumbs.
+                e.Demystify();
+            }
+
+            options.LogError(e, "The BeforeSend callback threw an exception. It will be added as breadcrumb and continue.");
+            var data = new Dictionary<string, string>
+            {
+                {"message", e.Message}
+            };
+            if (e.StackTrace is not null)
+            {
+                data.Add("stackTrace", e.StackTrace);
+            }
+            @event?.AddBreadcrumb(
+                "BeforeSend callback failed.",
+                category: "SentryClient",
+                data: data,
+                level: BreadcrumbLevel.Error);
+        }
+
+        return @event;
+    }
+}

src/Sentry/Platforms/Cocoa/SentrySdk.cs

@@ -1,6 +1,7 @@
 using Sentry.Cocoa;
 using Sentry.Cocoa.Extensions;
 using Sentry.Extensibility;
+using Sentry.Internal;
 
 // ReSharper disable once CheckNamespace
 namespace Sentry;
@@ -188,8 +189,21 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes(
         return nativeRanges;
     }
 
+    [DebuggerStepThrough]
     internal static CocoaSdk.SentryEvent? ProcessOnBeforeSend(SentryOptions options, CocoaSdk.SentryEvent evt)
+        => ProcessOnBeforeSend(options, evt, CurrentHub);
+
+    /// <summary>
+    /// This overload allows us to inject an IHub for testing. During normal execution, the CurrentHub is used.
+    /// However, since this class is static, there's no easy alternative way to inject this when executing tests.
+    /// </summary>
+    internal static CocoaSdk.SentryEvent? ProcessOnBeforeSend(SentryOptions options, CocoaSdk.SentryEvent evt, IHub hub)
     {
+        if (hub is DisabledHub)
+        {
+            return evt;
+        }
+
         // When we have an unhandled managed exception, we send that to Sentry twice - once managed and once native.
         // The managed exception is what a .NET developer would expect, and it is sent by the Sentry.NET SDK
         // But we also get a native SIGABRT since it crashed the application, which is sent by the Sentry Cocoa SDK.
@@ -224,32 +238,52 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes(
             }
         }
 
-        // we run our SIGABRT checks first before handing over to user events
-        // because we delegate to user code, we need to protect anything that could happen in this event
-        if (options.BeforeSendInternal == null)
-            return evt;
-
+        // We run our SIGABRT checks first before running managed processors.
+        // Because we delegate to user code, we need to catch/log exceptions.
         try
         {
-            var sentryEvent = evt.ToSentryEvent();
-            if (sentryEvent == null)
+            // Normally the event processors would be invoked by the SentryClient, but the Cocoa SDK has its own client,
+            // so we need to manually invoke any managed event processors here to apply them to Native events.
+            var manualProcessors = GetEventProcessors(hub)
+                .Where(p => p is not MainSentryEventProcessor)
+                .ToArray();
+            if (manualProcessors.Length == 0 && options.BeforeSendInternal is null)
+            {
                 return evt;
+            }
 
-            var result = options.BeforeSendInternal(sentryEvent, null!);
-            if (result == null)
-                return null!;
+            var sentryEvent = evt.ToSentryEvent();
+            if (SentryEventHelper.ProcessEvent(sentryEvent, manualProcessors, null, options) is not { } processedEvent)
+            {
+                return null;
+            }
+
+            processedEvent = SentryEventHelper.DoBeforeSend(processedEvent, new SentryHint(), options);
+            if (processedEvent == null)
+            {
+                return null;
+            }
 
             // we only support a subset of mutated data to be passed back to the native SDK at this time
-            result.CopyToCocoaSentryEvent(evt);
+            processedEvent.CopyToCocoaSentryEvent(evt);
 
-            // Note: Nullable result is allowed but delegate is generated incorrectly
-            // See https://github.com/xamarin/xamarin-macios/issues/15299#issuecomment-1201863294
-            return evt!;
+            return evt;
         }
         catch (Exception ex)
         {
-            options.LogError(ex, "Before Send Error");
+            options.LogError(ex, "Error running managed event processors for native event");
             return evt;
         }
+
+        static IEnumerable<ISentryEventProcessor> GetEventProcessors(IHub hub)
+        {
+            if (hub is Hub fullHub)
+            {
+                return fullHub.ScopeManager.GetCurrent().Key.GetAllEventProcessors();
+            }
+            IEnumerable<ISentryEventProcessor>? eventProcessors = null;
+            hub.ConfigureScope(scope => eventProcessors = scope.GetAllEventProcessors());
+            return eventProcessors ?? [];
+        }
     }
 }

src/Sentry/SentryClient.cs

@@ -348,26 +348,15 @@ private SentryId DoSendEvent(SentryEvent @event, SentryHint? hint, Scope? scope)
             }
         }
 
-        var processedEvent = @event;
-
-        foreach (var processor in scope.GetAllEventProcessors())
+        if (SentryEventHelper.ProcessEvent(@event, scope.GetAllEventProcessors(), hint, _options) is not { } processedEvent)
         {
-            processedEvent = processor.DoProcessEvent(processedEvent, hint);
-
-            if (processedEvent == null)
-            {
-                _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Error);
-                _options.LogInfo("Event dropped by processor {0}", processor.GetType().Name);
-                return SentryId.Empty;
-            }
+            return SentryId.Empty;  // Dropped by an event processor
         }
 
-        processedEvent = BeforeSend(processedEvent, hint);
-        if (processedEvent == null) // Rejected event
+        processedEvent = SentryEventHelper.DoBeforeSend(processedEvent, hint, _options);
+        if (processedEvent == null)
         {
-            _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Error);
-            _options.LogInfo("Event dropped by BeforeSend callback.");
-            return SentryId.Empty;
+            return SentryId.Empty; // Dropped by BeforeSend callback
         }
 
         var hasTerminalException = processedEvent.HasTerminalException();
@@ -454,48 +443,6 @@ public bool CaptureEnvelope(Envelope envelope)
         return false;
     }
 
-#if NET6_0_OR_GREATER
-    [UnconditionalSuppressMessage("Trimming", "IL2026: RequiresUnreferencedCode", Justification = AotHelper.AvoidAtRuntime)]
-#endif
-    private SentryEvent? BeforeSend(SentryEvent? @event, SentryHint hint)
-    {
-        if (_options.BeforeSendInternal == null)
-        {
-            return @event;
-        }
-
-        _options.LogDebug("Calling the BeforeSend callback");
-        try
-        {
-            @event = _options.BeforeSendInternal?.Invoke(@event!, hint);
-        }
-        catch (Exception e)
-        {
-            if (!AotHelper.IsTrimmed)
-            {
-                // Attempt to demystify exceptions before adding them as breadcrumbs.
-                e.Demystify();
-            }
-
-            _options.LogError(e, "The BeforeSend callback threw an exception. It will be added as breadcrumb and continue.");
-            var data = new Dictionary<string, string>
-            {
-                {"message", e.Message}
-            };
-            if (e.StackTrace is not null)
-            {
-                data.Add("stackTrace", e.StackTrace);
-            }
-            @event?.AddBreadcrumb(
-                "BeforeSend callback failed.",
-                category: "SentryClient",
-                data: data,
-                level: BreadcrumbLevel.Error);
-        }
-
-        return @event;
-    }
-
     /// <summary>
     /// Disposes this client
     /// </summary>

test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt

@@ -6,7 +6,7 @@
       message: Exception message!,
       stackTrace:
 at Task Sentry.Tests.SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb()
-at SentryEvent Sentry.SentryClient.BeforeSend(...)
+at SentryEvent Sentry.Internal.SentryEventHelper.DoBeforeSend(...)
     },
     Category: SentryClient,
     Level: error

test/Sentry.Tests/SentrySdkTests.cs

@@ -996,6 +996,7 @@ public void InitHub_DebugEnabled_DebugLogsLogged()
     [InlineData(false)]
     public void ProcessOnBeforeSend_NativeErrorSuppression(bool suppressNativeErrors)
     {
+        // Arrange
         var options = new SentryOptions
         {
             Dsn = ValidDsn,
@@ -1014,11 +1015,20 @@ public void ProcessOnBeforeSend_NativeErrorSuppression(bool suppressNativeErrors
             called = true;
             return e;
         });
+
+        var scope = new Scope(options);
+        var hub = Substitute.For<IHub>();
+        hub.When(h => hub.ConfigureScope(Arg.Any<Action<Scope>>()))
+            .Do(callback => callback.Arg<Action<Scope>>().Invoke(scope));
+
         var evt = new Sentry.CocoaSdk.SentryEvent();
         var ex = new Sentry.CocoaSdk.SentryException("Not checked", "EXC_BAD_ACCESS");
         evt.Exceptions = [ex];
-        var result = SentrySdk.ProcessOnBeforeSend(options, evt);
 
+        // Act
+        var result = SentrySdk.ProcessOnBeforeSend(options, evt, hub);
+
+        // Assert
         if (suppressNativeErrors)
         {
             called.Should().BeFalse();
@@ -1034,6 +1044,7 @@ public void ProcessOnBeforeSend_NativeErrorSuppression(bool suppressNativeErrors
     [Fact]
     public void ProcessOnBeforeSend_OptionsBeforeOnSendRuns()
     {
+        // Arrange
         var options = new SentryOptions
         {
             Dsn = ValidDsn,
@@ -1052,20 +1063,69 @@ public void ProcessOnBeforeSend_OptionsBeforeOnSendRuns()
         native.ReleaseName = "release name";
         native.Environment = "environment";
         native.Transaction = "transaction name";
-
         options.SetBeforeSend(e =>
         {
             e.TransactionName = "dotnet";
             return e;
         });
-        var result = SentrySdk.ProcessOnBeforeSend(options, native);
+
+        var scope = new Scope(options);
+        var hub = Substitute.For<IHub>();
+        hub.When(h => hub.ConfigureScope(Arg.Any<Action<Scope>>()))
+            .Do(callback => callback.Arg<Action<Scope>>().Invoke(scope));
+
+        // Act
+        var result = SentrySdk.ProcessOnBeforeSend(options, native, hub);
+
+        // Assert
         result.Should().NotBeNull();
         result.Transaction.Should().Be("dotnet");
     }
+
+    [Fact]
+    public void ProcessOnBeforeSend_EventProcessorsInvoked()
+    {
+        // Arrange
+        var options = new SentryOptions
+        {
+            Dsn = ValidDsn,
+            DiagnosticLogger = _logger,
+            IsGlobalModeEnabled = true,
+            Debug = true,
+            AutoSessionTracking = false,
+            BackgroundWorker = Substitute.For<IBackgroundWorker>(),
+            InitNativeSdks = false,
+        };
+        var eventProcessor = new TestEventProcessor();
+        options.AddEventProcessor(eventProcessor);
+
+        var scope = new Scope(options);
+        var hub = Substitute.For<IHub>();
+        hub.When(h => hub.ConfigureScope(Arg.Any<Action<Scope>>()))
+            .Do(callback => callback.Arg<Action<Scope>>().Invoke(scope));
+
+        var native = new Sentry.CocoaSdk.SentryEvent();
+
+        // Act
+        SentrySdk.ProcessOnBeforeSend(options, native, hub);
+
+        // Assert
+        eventProcessor.Invoked.Should().BeTrue();
+    }
 #endif
 
     public void Dispose()
     {
         SentrySdk.Close();
     }
 }
+
+file class TestEventProcessor : ISentryEventProcessor
+{
+    public bool Invoked { get; private set; }
+    public SentryEvent Process(SentryEvent @event)
+    {
+        Invoked = true;
+        return @event;
+    }
+}