diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cc59b8f81..72c1388598 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Fail when building Blazor WASM with Profiling. We don't support profiling in Blazor WebAssembly projects. ([#4512](https://github.com/getsentry/sentry-dotnet/pull/4512)) - Do not overwrite user IP if it is set manually in ASP.NET sdk ([#4513](https://github.com/getsentry/sentry-dotnet/pull/4513)) +- Fix `SentryOptions.Native.SuppressSignalAborts` and `SuppressExcBadAccess` on iOS ([#4521](https://github.com/getsentry/sentry-dotnet/pull/4521)) ## 5.15.0 diff --git a/src/Sentry/Platforms/Cocoa/SentrySdk.cs b/src/Sentry/Platforms/Cocoa/SentrySdk.cs index 9cd167dc0e..32dd3e40e0 100644 --- a/src/Sentry/Platforms/Cocoa/SentrySdk.cs +++ b/src/Sentry/Platforms/Cocoa/SentrySdk.cs @@ -195,20 +195,15 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes( => ProcessOnBeforeSend(options, evt, CurrentHub); /// - /// 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. + /// Apply suppression logic for redundant native `SIGABRT` and `EXC_BAD_ACCESS` crash events + /// that have already been captured as managed exceptions by the Sentry.NET SDK to avoid sending + /// duplicate events to Sentry - 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. /// - internal static CocoaSdk.SentryEvent? ProcessOnBeforeSend(SentryOptions options, CocoaSdk.SentryEvent evt, IHub hub) + private static bool SuppressNativeCrash(SentryOptions options, CocoaSdk.SentryEvent evt) { - 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. - // There should only be one exception on the event in this case if ((options.Native.SuppressSignalAborts || options.Native.SuppressExcBadAccess) && evt.Exceptions?.Length == 1) { @@ -224,7 +219,7 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes( // Don't send it options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type, ex.Value); - return null!; + return true; } // Similar workaround for NullReferenceExceptions. We don't have any easy way to know whether the @@ -235,10 +230,33 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes( // Don't send it options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type, ex.Value); - return null!; + return true; } } + return false; + } + + /// + /// 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. + /// + internal static CocoaSdk.SentryEvent? ProcessOnBeforeSend(SentryOptions options, CocoaSdk.SentryEvent evt, IHub hub) + { + // Redundant native crash events must be suppressed even if the SDK is + // disabled (or not yet fully initialized) to avoid sending duplicates. + // https://github.com/getsentry/sentry-dotnet/pull/4521#discussion_r2347616896 + if (SuppressNativeCrash(options, evt)) + { + return null!; + } + + // If the SDK is disabled, there are no event processors or before send to run. + if (hub is DisabledHub) + { + 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 diff --git a/test/Sentry.Tests/SentrySdkTests.cs b/test/Sentry.Tests/SentrySdkTests.cs index e63b97a20b..9c5fe1eca0 100644 --- a/test/Sentry.Tests/SentrySdkTests.cs +++ b/test/Sentry.Tests/SentrySdkTests.cs @@ -1041,6 +1041,47 @@ public void ProcessOnBeforeSend_NativeErrorSuppression(bool suppressNativeErrors } } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ProcessOnBeforeSend_NativeErrorSuppressionBeforeHubInit(bool suppressNativeErrors) + { + // Arrange + var options = new SentryOptions + { + Dsn = ValidDsn, + DiagnosticLogger = _logger, + IsGlobalModeEnabled = true, + Debug = true, + AutoSessionTracking = false, + BackgroundWorker = Substitute.For(), + InitNativeSdks = false, + }; + options.Native.SuppressExcBadAccess = suppressNativeErrors; + + var scope = new Scope(options); + // `SIGABRT` must be filtered out early on startup during + // the Cocoa SDK init before the Hub instance has been created + var hub = DisabledHub.Instance; + + var evt = new Sentry.CocoaSdk.SentryEvent(); + var ex = new Sentry.CocoaSdk.SentryException("Not checked", "EXC_BAD_ACCESS"); + evt.Exceptions = [ex]; + + // Act + var result = SentrySdk.ProcessOnBeforeSend(options, evt, hub); + + // Assert + if (suppressNativeErrors) + { + result.Should().BeNull(); + } + else + { + result.Exceptions.First().Type.Should().Be("EXC_BAD_ACCESS"); + } + } + [Fact] public void ProcessOnBeforeSend_OptionsBeforeOnSendRuns() {