Skip to content

Commit b330e67

Browse files
aritchiearitchie
authored
Add option to enable/disable EXC_BAD_ACCESS suppression (#3998)
1 parent 1f56e7e commit b330e67

File tree

5 files changed

+190
-65
lines changed

5 files changed

+190
-65
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
### Fixes
66

7+
- 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))
8+
- 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))
79
- Prevent crashes from occurring on Android during OnBeforeSend ([#4022](https://github.com/getsentry/sentry-dotnet/pull/4022))
810

911
### Dependencies
@@ -16,7 +18,6 @@
1618

1719
### Features
1820

19-
- 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))
2021
- Users can now register their own MAUI controls for breadcrumb creation ([#3997](https://github.com/getsentry/sentry-dotnet/pull/3997))
2122
- Serilog scope properties are now sent with Sentry events ([#3976](https://github.com/getsentry/sentry-dotnet/pull/3976))
2223
- 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))

‎src/Sentry/Platforms/Cocoa/BindableSentryOptions.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ public class NativeOptions
2626
public bool? EnableUIViewControllerTracing { get; set; }
2727
public bool? EnableUserInteractionTracing { get; set; }
2828
public bool? EnableTracing { get; set; }
29+
public bool? SuppressSignalAborts { get; set; }
30+
public bool? SuppressExcBadAccess { get; set; }
2931

3032
public void ApplyTo(SentryOptions.NativeOptions options)
3133
{
@@ -47,6 +49,8 @@ public void ApplyTo(SentryOptions.NativeOptions options)
4749
options.EnableUIViewControllerTracing = EnableUIViewControllerTracing ?? options.EnableUIViewControllerTracing;
4850
options.EnableUserInteractionTracing = EnableUserInteractionTracing ?? options.EnableUserInteractionTracing;
4951
options.EnableTracing = EnableTracing ?? options.EnableTracing;
52+
options.SuppressSignalAborts = SuppressSignalAborts ?? options.SuppressSignalAborts;
53+
options.SuppressExcBadAccess = SuppressExcBadAccess ?? options.SuppressExcBadAccess;
5054
}
5155
}
5256
}

‎src/Sentry/Platforms/Cocoa/SentryOptions.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,38 @@ internal NativeOptions(SentryOptions options)
197197
/// </remarks>
198198
public NSUrlSessionDelegate? UrlSessionDelegate { get; set; } = null;
199199

200+
/// <summary>
201+
/// <para>
202+
/// Whether to suppress capturing SIGABRT errors in the Native SDK.
203+
/// </para>
204+
/// <para>
205+
/// When managed code results in a NullReferenceException, this also causes a SIGABRT. Duplicate
206+
/// events (one managed and one native) can be prevented by suppressing native SIGABRT, which may be
207+
/// convenient.
208+
/// </para>
209+
/// <para>
210+
/// Enabling this may prevent the capture of SIGABRT originating from native (not managed) code... so it may
211+
/// prevent the capture of genuine native SIGABRT errors.
212+
/// </para>
213+
/// </summary>
214+
public bool SuppressSignalAborts { get; set; } = false;
215+
216+
/// <summary>
217+
/// <para>
218+
/// Whether to suppress capturing EXC_BAD_ACCESS errors in the Native SDK.
219+
/// </para>
220+
/// <para>
221+
/// When managed code results in a NullReferenceException, this also causes a EXC_BAD_ACCESS. Duplicate
222+
/// events (one managed and one native) can be prevented by suppressing native EXC_BAD_ACCESS, which may be
223+
/// convenient.
224+
/// </para>
225+
/// <para>
226+
/// Enabling this may prevent the capture of EXC_BAD_ACCESS originating from native (not managed) code... so it may
227+
/// prevent the capture of genuine native EXC_BAD_ACCESS errors.
228+
/// </para>
229+
/// </summary>
230+
public bool SuppressExcBadAccess { get; set; } = false;
231+
200232
// ---------- Other ----------
201233

202234
/// <summary>

‎src/Sentry/Platforms/Cocoa/SentrySdk.cs

Lines changed: 66 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -86,70 +86,7 @@ private static void InitSentryCocoaSdk(SentryOptions options)
8686
}
8787
}
8888

89-
nativeOptions.BeforeSend = evt =>
90-
{
91-
// When we have an unhandled managed exception, we send that to Sentry twice - once managed and once native.
92-
// The managed exception is what a .NET developer would expect, and it is sent by the Sentry.NET SDK
93-
// But we also get a native SIGABRT since it crashed the application, which is sent by the Sentry Cocoa SDK.
94-
95-
// There should only be one exception on the event in this case
96-
if (evt.Exceptions?.Length == 1)
97-
{
98-
// It will match the following characteristics
99-
var ex = evt.Exceptions[0];
100-
101-
// Thankfully, sometimes we can see Xamarin's unhandled exception handler on the stack trace, so we can filter
102-
// them out. Here is the function that calls abort(), which we will use as a filter:
103-
// https://github.com/xamarin/xamarin-macios/blob/c55fbdfef95028ba03d0f7a35aebca03bd76f852/runtime/runtime.m#L1114-L1122
104-
if (ex.Type == "SIGABRT" && ex.Value == "Signal 6, Code 0" &&
105-
ex.Stacktrace?.Frames.Any(f => f.Function == "xamarin_unhandled_exception_handler") is true)
106-
{
107-
// Don't send it
108-
options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type,
109-
ex.Value);
110-
return null!;
111-
}
112-
113-
// Similar workaround for NullReferenceExceptions. We don't have any easy way to know whether the
114-
// exception is managed code (compiled to native) or original native code though.
115-
// See: https://github.com/getsentry/sentry-dotnet/issues/3776
116-
if (ex.Type == "EXC_BAD_ACCESS")
117-
{
118-
// Don't send it
119-
options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type,
120-
ex.Value);
121-
return null!;
122-
}
123-
}
124-
125-
// we run our SIGABRT checks first before handing over to user events
126-
// because we delegate to user code, we need to protect anything that could happen in this event
127-
if (options.BeforeSendInternal == null)
128-
return evt;
129-
130-
try
131-
{
132-
var sentryEvent = evt.ToSentryEvent();
133-
if (sentryEvent == null)
134-
return evt;
135-
136-
var result = options.BeforeSendInternal(sentryEvent, null!);
137-
if (result == null)
138-
return null!;
139-
140-
// we only support a subset of mutated data to be passed back to the native SDK at this time
141-
result.CopyToCocoaSentryEvent(evt);
142-
143-
// Note: Nullable result is allowed but delegate is generated incorrectly
144-
// See https://github.com/xamarin/xamarin-macios/issues/15299#issuecomment-1201863294
145-
return evt!;
146-
}
147-
catch (Exception ex)
148-
{
149-
options.LogError(ex, "Before Send Error");
150-
return evt;
151-
}
152-
};
89+
nativeOptions.BeforeSend = evt => ProcessOnBeforeSend(options, evt)!;
15390

15491
if (options.OnCrashedLastRun is { } onCrashedLastRun)
15592
{
@@ -250,4 +187,69 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes(
250187

251188
return nativeRanges;
252189
}
190+
191+
internal static CocoaSdk.SentryEvent? ProcessOnBeforeSend(SentryOptions options, CocoaSdk.SentryEvent evt)
192+
{
193+
// When we have an unhandled managed exception, we send that to Sentry twice - once managed and once native.
194+
// The managed exception is what a .NET developer would expect, and it is sent by the Sentry.NET SDK
195+
// But we also get a native SIGABRT since it crashed the application, which is sent by the Sentry Cocoa SDK.
196+
197+
// There should only be one exception on the event in this case
198+
if ((options.Native.SuppressSignalAborts || options.Native.SuppressExcBadAccess) && evt.Exceptions?.Length == 1)
199+
{
200+
// It will match the following characteristics
201+
var ex = evt.Exceptions[0];
202+
203+
// Thankfully, sometimes we can see Xamarin's unhandled exception handler on the stack trace, so we can filter
204+
// them out. Here is the function that calls abort(), which we will use as a filter:
205+
// https://github.com/xamarin/xamarin-macios/blob/c55fbdfef95028ba03d0f7a35aebca03bd76f852/runtime/runtime.m#L1114-L1122
206+
if (options.Native.SuppressSignalAborts && ex.Type == "SIGABRT" && ex.Value == "Signal 6, Code 0" &&
207+
ex.Stacktrace?.Frames.Any(f => f.Function == "xamarin_unhandled_exception_handler") is true)
208+
{
209+
// Don't send it
210+
options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type,
211+
ex.Value);
212+
return null!;
213+
}
214+
215+
// Similar workaround for NullReferenceExceptions. We don't have any easy way to know whether the
216+
// exception is managed code (compiled to native) or original native code though.
217+
// See: https://github.com/getsentry/sentry-dotnet/issues/3776
218+
if (options.Native.SuppressExcBadAccess && ex.Type == "EXC_BAD_ACCESS")
219+
{
220+
// Don't send it
221+
options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type,
222+
ex.Value);
223+
return null!;
224+
}
225+
}
226+
227+
// we run our SIGABRT checks first before handing over to user events
228+
// because we delegate to user code, we need to protect anything that could happen in this event
229+
if (options.BeforeSendInternal == null)
230+
return evt;
231+
232+
try
233+
{
234+
var sentryEvent = evt.ToSentryEvent();
235+
if (sentryEvent == null)
236+
return evt;
237+
238+
var result = options.BeforeSendInternal(sentryEvent, null!);
239+
if (result == null)
240+
return null!;
241+
242+
// we only support a subset of mutated data to be passed back to the native SDK at this time
243+
result.CopyToCocoaSentryEvent(evt);
244+
245+
// Note: Nullable result is allowed but delegate is generated incorrectly
246+
// See https://github.com/xamarin/xamarin-macios/issues/15299#issuecomment-1201863294
247+
return evt!;
248+
}
249+
catch (Exception ex)
250+
{
251+
options.LogError(ex, "Before Send Error");
252+
return evt;
253+
}
254+
}
253255
}

‎test/Sentry.Tests/SentrySdkTests.cs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,17 @@
1+
using System;
2+
using System.Linq;
3+
using System.Threading;
4+
using System.Threading.Tasks;
5+
using FluentAssertions;
6+
using NSubstitute;
7+
using Sentry.Extensibility;
8+
using Sentry.Internal;
19
using Sentry.Internal.Http;
210
using Sentry.Internal.ScopeStack;
11+
using Sentry.Protocol.Envelopes;
12+
using Sentry.Testing;
13+
using Xunit;
14+
using Xunit.Abstractions;
315
using static Sentry.Internal.Constants;
416

517
namespace Sentry.Tests;
@@ -831,6 +843,80 @@ public void InitHub_DebugEnabled_DebugLogsLogged()
831843
Arg.Any<object[]>());
832844
}
833845

846+
#if __IOS__
847+
[Theory]
848+
[InlineData(true)]
849+
[InlineData(false)]
850+
public void ProcessOnBeforeSend_NativeErrorSuppression(bool suppressNativeErrors)
851+
{
852+
var options = new SentryOptions
853+
{
854+
Dsn = ValidDsn,
855+
DiagnosticLogger = _logger,
856+
IsGlobalModeEnabled = true,
857+
Debug = true,
858+
AutoSessionTracking = false,
859+
BackgroundWorker = Substitute.For<IBackgroundWorker>(),
860+
InitNativeSdks = false,
861+
};
862+
options.Native.SuppressExcBadAccess = suppressNativeErrors;
863+
864+
var called = false;
865+
options.SetBeforeSend(e =>
866+
{
867+
called = true;
868+
return e;
869+
});
870+
var evt = new Sentry.CocoaSdk.SentryEvent();
871+
var ex = new Sentry.CocoaSdk.SentryException("Not checked", "EXC_BAD_ACCESS");
872+
evt.Exceptions = [ex];
873+
var result = SentrySdk.ProcessOnBeforeSend(options, evt);
874+
875+
if (suppressNativeErrors)
876+
{
877+
called.Should().BeFalse();
878+
result.Should().BeNull();
879+
}
880+
else
881+
{
882+
called.Should().BeTrue();
883+
result.Exceptions.First().Type.Should().Be("EXC_BAD_ACCESS");
884+
}
885+
}
886+
887+
[Fact]
888+
public void ProcessOnBeforeSend_OptionsBeforeOnSendRuns()
889+
{
890+
var options = new SentryOptions
891+
{
892+
Dsn = ValidDsn,
893+
DiagnosticLogger = _logger,
894+
IsGlobalModeEnabled = true,
895+
Debug = true,
896+
AutoSessionTracking = false,
897+
BackgroundWorker = Substitute.For<IBackgroundWorker>(),
898+
InitNativeSdks = false
899+
};
900+
901+
var native = new Sentry.CocoaSdk.SentryEvent();
902+
native.ServerName = "server name";
903+
native.Dist = "dist";
904+
native.Logger = "logger";
905+
native.ReleaseName = "release name";
906+
native.Environment = "environment";
907+
native.Transaction = "transaction name";
908+
909+
options.SetBeforeSend(e =>
910+
{
911+
e.TransactionName = "dotnet";
912+
return e;
913+
});
914+
var result = SentrySdk.ProcessOnBeforeSend(options, native);
915+
result.Should().NotBeNull();
916+
result.Transaction.Should().Be("dotnet");
917+
}
918+
#endif
919+
834920
public void Dispose()
835921
{
836922
SentrySdk.Close();

0 commit comments

Comments
 (0)