diff --git a/CHANGELOG.md b/CHANGELOG.md index f74bf20a51..cf29d0999a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Fixes +- Using SentryOptions.Native.SuppressExcBadAccess and SentryOptions.Native.SuppressSignalAborts, users can now block duplicate errors from native due to dotnet NullReferenceExceptions - Defaults to false ([#3998](https://github.com/getsentry/sentry-dotnet/pull/3998)) +- Native iOS events are now exposed to the dotnet layer for users to hook through SentryOptions.BeforeSend and SentryOptions.OnCrashedLastRun ([#2102](https://github.com/getsentry/sentry-dotnet/pull/3958)) - Prevent crashes from occurring on Android during OnBeforeSend ([#4022](https://github.com/getsentry/sentry-dotnet/pull/4022)) ### Dependencies @@ -16,7 +18,6 @@ ### Features -- Native iOS events are now exposed to the dotnet layer for users to hook through SentryOptions.BeforeSend and SentryOptions.OnCrashedLastRun ([#2102](https://github.com/getsentry/sentry-dotnet/pull/3958)) - Users can now register their own MAUI controls for breadcrumb creation ([#3997](https://github.com/getsentry/sentry-dotnet/pull/3997)) - Serilog scope properties are now sent with Sentry events ([#3976](https://github.com/getsentry/sentry-dotnet/pull/3976)) - The sample seed used for sampling decisions is now propagated, for use in downstream custom trace samplers ([#3951](https://github.com/getsentry/sentry-dotnet/pull/3951)) diff --git a/src/Sentry/Platforms/Cocoa/BindableSentryOptions.cs b/src/Sentry/Platforms/Cocoa/BindableSentryOptions.cs index f8dae26213..edc529dc86 100644 --- a/src/Sentry/Platforms/Cocoa/BindableSentryOptions.cs +++ b/src/Sentry/Platforms/Cocoa/BindableSentryOptions.cs @@ -26,6 +26,8 @@ public class NativeOptions public bool? EnableUIViewControllerTracing { get; set; } public bool? EnableUserInteractionTracing { get; set; } public bool? EnableTracing { get; set; } + public bool? SuppressSignalAborts { get; set; } + public bool? SuppressExcBadAccess { get; set; } public void ApplyTo(SentryOptions.NativeOptions options) { @@ -47,6 +49,8 @@ public void ApplyTo(SentryOptions.NativeOptions options) options.EnableUIViewControllerTracing = EnableUIViewControllerTracing ?? options.EnableUIViewControllerTracing; options.EnableUserInteractionTracing = EnableUserInteractionTracing ?? options.EnableUserInteractionTracing; options.EnableTracing = EnableTracing ?? options.EnableTracing; + options.SuppressSignalAborts = SuppressSignalAborts ?? options.SuppressSignalAborts; + options.SuppressExcBadAccess = SuppressExcBadAccess ?? options.SuppressExcBadAccess; } } } diff --git a/src/Sentry/Platforms/Cocoa/SentryOptions.cs b/src/Sentry/Platforms/Cocoa/SentryOptions.cs index 91172d9938..21ead2c413 100644 --- a/src/Sentry/Platforms/Cocoa/SentryOptions.cs +++ b/src/Sentry/Platforms/Cocoa/SentryOptions.cs @@ -197,6 +197,38 @@ internal NativeOptions(SentryOptions options) /// public NSUrlSessionDelegate? UrlSessionDelegate { get; set; } = null; + /// + /// + /// Whether to suppress capturing SIGABRT errors in the Native SDK. + /// + /// + /// When managed code results in a NullReferenceException, this also causes a SIGABRT. Duplicate + /// events (one managed and one native) can be prevented by suppressing native SIGABRT, which may be + /// convenient. + /// + /// + /// Enabling this may prevent the capture of SIGABRT originating from native (not managed) code... so it may + /// prevent the capture of genuine native SIGABRT errors. + /// + /// + public bool SuppressSignalAborts { get; set; } = false; + + /// + /// + /// Whether to suppress capturing EXC_BAD_ACCESS errors in the Native SDK. + /// + /// + /// When managed code results in a NullReferenceException, this also causes a EXC_BAD_ACCESS. Duplicate + /// events (one managed and one native) can be prevented by suppressing native EXC_BAD_ACCESS, which may be + /// convenient. + /// + /// + /// Enabling this may prevent the capture of EXC_BAD_ACCESS originating from native (not managed) code... so it may + /// prevent the capture of genuine native EXC_BAD_ACCESS errors. + /// + /// + public bool SuppressExcBadAccess { get; set; } = false; + // ---------- Other ---------- /// diff --git a/src/Sentry/Platforms/Cocoa/SentrySdk.cs b/src/Sentry/Platforms/Cocoa/SentrySdk.cs index 8908cd78c0..5e60eb100d 100644 --- a/src/Sentry/Platforms/Cocoa/SentrySdk.cs +++ b/src/Sentry/Platforms/Cocoa/SentrySdk.cs @@ -86,70 +86,7 @@ private static void InitSentryCocoaSdk(SentryOptions options) } } - nativeOptions.BeforeSend = 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 (evt.Exceptions?.Length == 1) - { - // It will match the following characteristics - var ex = evt.Exceptions[0]; - - // Thankfully, sometimes we can see Xamarin's unhandled exception handler on the stack trace, so we can filter - // them out. Here is the function that calls abort(), which we will use as a filter: - // https://github.com/xamarin/xamarin-macios/blob/c55fbdfef95028ba03d0f7a35aebca03bd76f852/runtime/runtime.m#L1114-L1122 - if (ex.Type == "SIGABRT" && ex.Value == "Signal 6, Code 0" && - ex.Stacktrace?.Frames.Any(f => f.Function == "xamarin_unhandled_exception_handler") is true) - { - // Don't send it - options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type, - ex.Value); - return null!; - } - - // Similar workaround for NullReferenceExceptions. We don't have any easy way to know whether the - // exception is managed code (compiled to native) or original native code though. - // See: https://github.com/getsentry/sentry-dotnet/issues/3776 - if (ex.Type == "EXC_BAD_ACCESS") - { - // Don't send it - options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type, - ex.Value); - return null!; - } - } - - // 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; - - try - { - var sentryEvent = evt.ToSentryEvent(); - if (sentryEvent == null) - return evt; - - var result = options.BeforeSendInternal(sentryEvent, null!); - if (result == 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); - - // Note: Nullable result is allowed but delegate is generated incorrectly - // See https://github.com/xamarin/xamarin-macios/issues/15299#issuecomment-1201863294 - return evt!; - } - catch (Exception ex) - { - options.LogError(ex, "Before Send Error"); - return evt; - } - }; + nativeOptions.BeforeSend = evt => ProcessOnBeforeSend(options, evt)!; if (options.OnCrashedLastRun is { } onCrashedLastRun) { @@ -250,4 +187,69 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes( return nativeRanges; } + + internal static CocoaSdk.SentryEvent? ProcessOnBeforeSend(SentryOptions options, CocoaSdk.SentryEvent 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) + { + // It will match the following characteristics + var ex = evt.Exceptions[0]; + + // Thankfully, sometimes we can see Xamarin's unhandled exception handler on the stack trace, so we can filter + // them out. Here is the function that calls abort(), which we will use as a filter: + // https://github.com/xamarin/xamarin-macios/blob/c55fbdfef95028ba03d0f7a35aebca03bd76f852/runtime/runtime.m#L1114-L1122 + if (options.Native.SuppressSignalAborts && ex.Type == "SIGABRT" && ex.Value == "Signal 6, Code 0" && + ex.Stacktrace?.Frames.Any(f => f.Function == "xamarin_unhandled_exception_handler") is true) + { + // Don't send it + options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type, + ex.Value); + return null!; + } + + // Similar workaround for NullReferenceExceptions. We don't have any easy way to know whether the + // exception is managed code (compiled to native) or original native code though. + // See: https://github.com/getsentry/sentry-dotnet/issues/3776 + if (options.Native.SuppressExcBadAccess && ex.Type == "EXC_BAD_ACCESS") + { + // Don't send it + options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type, + ex.Value); + return null!; + } + } + + // 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; + + try + { + var sentryEvent = evt.ToSentryEvent(); + if (sentryEvent == null) + return evt; + + var result = options.BeforeSendInternal(sentryEvent, null!); + if (result == 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); + + // Note: Nullable result is allowed but delegate is generated incorrectly + // See https://github.com/xamarin/xamarin-macios/issues/15299#issuecomment-1201863294 + return evt!; + } + catch (Exception ex) + { + options.LogError(ex, "Before Send Error"); + return evt; + } + } } diff --git a/test/Sentry.Tests/SentrySdkTests.cs b/test/Sentry.Tests/SentrySdkTests.cs index 4f8a45fec6..f88876e914 100644 --- a/test/Sentry.Tests/SentrySdkTests.cs +++ b/test/Sentry.Tests/SentrySdkTests.cs @@ -1,5 +1,17 @@ +using System; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using FluentAssertions; +using NSubstitute; +using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Internal.Http; using Sentry.Internal.ScopeStack; +using Sentry.Protocol.Envelopes; +using Sentry.Testing; +using Xunit; +using Xunit.Abstractions; using static Sentry.Internal.Constants; namespace Sentry.Tests; @@ -831,6 +843,80 @@ public void InitHub_DebugEnabled_DebugLogsLogged() Arg.Any()); } +#if __IOS__ + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ProcessOnBeforeSend_NativeErrorSuppression(bool suppressNativeErrors) + { + var options = new SentryOptions + { + Dsn = ValidDsn, + DiagnosticLogger = _logger, + IsGlobalModeEnabled = true, + Debug = true, + AutoSessionTracking = false, + BackgroundWorker = Substitute.For(), + InitNativeSdks = false, + }; + options.Native.SuppressExcBadAccess = suppressNativeErrors; + + var called = false; + options.SetBeforeSend(e => + { + called = true; + return e; + }); + 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); + + if (suppressNativeErrors) + { + called.Should().BeFalse(); + result.Should().BeNull(); + } + else + { + called.Should().BeTrue(); + result.Exceptions.First().Type.Should().Be("EXC_BAD_ACCESS"); + } + } + + [Fact] + public void ProcessOnBeforeSend_OptionsBeforeOnSendRuns() + { + var options = new SentryOptions + { + Dsn = ValidDsn, + DiagnosticLogger = _logger, + IsGlobalModeEnabled = true, + Debug = true, + AutoSessionTracking = false, + BackgroundWorker = Substitute.For(), + InitNativeSdks = false + }; + + var native = new Sentry.CocoaSdk.SentryEvent(); + native.ServerName = "server name"; + native.Dist = "dist"; + native.Logger = "logger"; + 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); + result.Should().NotBeNull(); + result.Transaction.Should().Be("dotnet"); + } +#endif + public void Dispose() { SentrySdk.Close();