Skip to content

Commit 7061fb9

Browse files
authored
fix: SentryOptions.Native.SuppressSignalAborts and SuppressExcBadAccess on iOS (#4521)
1 parent cc5b310 commit 7061fb9

File tree

3 files changed

+74
-14
lines changed

3 files changed

+74
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
- 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))
88
- Do not overwrite user IP if it is set manually in ASP.NET sdk ([#4513](https://github.com/getsentry/sentry-dotnet/pull/4513))
9+
- Fix `SentryOptions.Native.SuppressSignalAborts` and `SuppressExcBadAccess` on iOS ([#4521](https://github.com/getsentry/sentry-dotnet/pull/4521))
910

1011
## 5.15.0
1112

src/Sentry/Platforms/Cocoa/SentrySdk.cs

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -195,20 +195,15 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes(
195195
=> ProcessOnBeforeSend(options, evt, CurrentHub);
196196

197197
/// <summary>
198-
/// This overload allows us to inject an IHub for testing. During normal execution, the CurrentHub is used.
199-
/// However, since this class is static, there's no easy alternative way to inject this when executing tests.
198+
/// Apply suppression logic for redundant native `SIGABRT` and `EXC_BAD_ACCESS` crash events
199+
/// that have already been captured as managed exceptions by the Sentry.NET SDK to avoid sending
200+
/// duplicate events to Sentry - once managed and once native.
201+
///
202+
/// The managed exception is what a .NET developer would expect, and it is sent by the Sentry.NET SDK
203+
/// But we also get a native SIGABRT since it crashed the application, which is sent by the Sentry Cocoa SDK.
200204
/// </summary>
201-
internal static CocoaSdk.SentryEvent? ProcessOnBeforeSend(SentryOptions options, CocoaSdk.SentryEvent evt, IHub hub)
205+
private static bool SuppressNativeCrash(SentryOptions options, CocoaSdk.SentryEvent evt)
202206
{
203-
if (hub is DisabledHub)
204-
{
205-
return evt;
206-
}
207-
208-
// When we have an unhandled managed exception, we send that to Sentry twice - once managed and once native.
209-
// The managed exception is what a .NET developer would expect, and it is sent by the Sentry.NET SDK
210-
// But we also get a native SIGABRT since it crashed the application, which is sent by the Sentry Cocoa SDK.
211-
212207
// There should only be one exception on the event in this case
213208
if ((options.Native.SuppressSignalAborts || options.Native.SuppressExcBadAccess) && evt.Exceptions?.Length == 1)
214209
{
@@ -224,7 +219,7 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes(
224219
// Don't send it
225220
options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type,
226221
ex.Value);
227-
return null!;
222+
return true;
228223
}
229224

230225
// Similar workaround for NullReferenceExceptions. We don't have any easy way to know whether the
@@ -235,10 +230,33 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes(
235230
// Don't send it
236231
options.LogDebug("Discarded {0} error ({1}). Captured as managed exception instead.", ex.Type,
237232
ex.Value);
238-
return null!;
233+
return true;
239234
}
240235
}
241236

237+
return false;
238+
}
239+
240+
/// <summary>
241+
/// This overload allows us to inject an IHub for testing. During normal execution, the CurrentHub is used.
242+
/// However, since this class is static, there's no easy alternative way to inject this when executing tests.
243+
/// </summary>
244+
internal static CocoaSdk.SentryEvent? ProcessOnBeforeSend(SentryOptions options, CocoaSdk.SentryEvent evt, IHub hub)
245+
{
246+
// Redundant native crash events must be suppressed even if the SDK is
247+
// disabled (or not yet fully initialized) to avoid sending duplicates.
248+
// https://github.com/getsentry/sentry-dotnet/pull/4521#discussion_r2347616896
249+
if (SuppressNativeCrash(options, evt))
250+
{
251+
return null!;
252+
}
253+
254+
// If the SDK is disabled, there are no event processors or before send to run.
255+
if (hub is DisabledHub)
256+
{
257+
return evt;
258+
}
259+
242260
// We run our SIGABRT checks first before running managed processors.
243261
// Because we delegate to user code, we need to catch/log exceptions.
244262
try

test/Sentry.Tests/SentrySdkTests.cs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,6 +1041,47 @@ public void ProcessOnBeforeSend_NativeErrorSuppression(bool suppressNativeErrors
10411041
}
10421042
}
10431043

1044+
[Theory]
1045+
[InlineData(true)]
1046+
[InlineData(false)]
1047+
public void ProcessOnBeforeSend_NativeErrorSuppressionBeforeHubInit(bool suppressNativeErrors)
1048+
{
1049+
// Arrange
1050+
var options = new SentryOptions
1051+
{
1052+
Dsn = ValidDsn,
1053+
DiagnosticLogger = _logger,
1054+
IsGlobalModeEnabled = true,
1055+
Debug = true,
1056+
AutoSessionTracking = false,
1057+
BackgroundWorker = Substitute.For<IBackgroundWorker>(),
1058+
InitNativeSdks = false,
1059+
};
1060+
options.Native.SuppressExcBadAccess = suppressNativeErrors;
1061+
1062+
var scope = new Scope(options);
1063+
// `SIGABRT` must be filtered out early on startup during
1064+
// the Cocoa SDK init before the Hub instance has been created
1065+
var hub = DisabledHub.Instance;
1066+
1067+
var evt = new Sentry.CocoaSdk.SentryEvent();
1068+
var ex = new Sentry.CocoaSdk.SentryException("Not checked", "EXC_BAD_ACCESS");
1069+
evt.Exceptions = [ex];
1070+
1071+
// Act
1072+
var result = SentrySdk.ProcessOnBeforeSend(options, evt, hub);
1073+
1074+
// Assert
1075+
if (suppressNativeErrors)
1076+
{
1077+
result.Should().BeNull();
1078+
}
1079+
else
1080+
{
1081+
result.Exceptions.First().Type.Should().Be("EXC_BAD_ACCESS");
1082+
}
1083+
}
1084+
10441085
[Fact]
10451086
public void ProcessOnBeforeSend_OptionsBeforeOnSendRuns()
10461087
{

0 commit comments

Comments
 (0)