Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
46 changes: 32 additions & 14 deletions src/Sentry/Platforms/Cocoa/SentrySdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -195,20 +195,15 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes(
=> 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.
/// 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.
/// </summary>
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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would this be a problem? If the SDK is disabled, Sentry shouldn't be capturing any events at all right (either in the .NET or the Cocoa SDK)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a problem on startup when the previous run crashed with a duplicate SIGABRT or EXC_BAD_ACCESS that needs to be suppressed. Even if the .NET SDK is not (yet) in a fully initialized state, ProcessOnBeforeSend needs to return null for the suppression mechanism to work:

static partial class SentrySdk
{
    internal static IHub InitHub(SentryOptions options)
    {
        // ...

        if (options.InitNativeSdks)
        {
            InitSentryCocoaSdk(options); // <== ProcessOnBeforeSend for SIGABRT/EXC_BAD_ACCESS from the previous run
        }

        // We init the hub after native SDK in case the native init needs to adapt some options.
        var hub = new Hub(options);
        // ...
        return hub;
    }
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see... subtle. So the Cocoa SDK is already sending events before the .NET SDK has even been initialised!

// We init the hub after native SDK in case the native init needs to adapt some options.

I wonder what that's all about. @vaind do you recall (git blames you for that line 😜)

Copy link
Collaborator

@vaind vaind Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't recall specifically but what this is trying to get along, IMO, is that the InitSentryCocoaSdk() (in case of iOS) must be called before new Hub(options); because the former changes options which the latter uses.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, thanks @vaind. The only managed options that get set by the Cocoa SDK are these:

// Set default release and distribution
options.Release ??= GetDefaultReleaseString();
options.Distribution ??= GetDefaultDistributionString();

// Set options for the managed SDK that depend on the Cocoa SDK. (The user will not be able to modify these.)
options.AddEventProcessor(new CocoaEventProcessor());
options.CrashedLastRun = () => SentryCocoaSdk.CrashedLastRun;
options.EnableScopeSync = true;
options.ScopeObserver = new CocoaScopeObserver(options);

I guess we'd want to detect the release and distribution before we initialise the .NET SDK Hub... and possibly wire up the CrashedLastRun handler (although I'm not sure why we delegate to the Cocoa SDK for this - Matt implemented that in #1849).

Ultimately the BeforeX handlers for whatever SDK we initialise 2nd may not be executed for the first few events that get sent... unless we add some concurrency mechanism to delay sending events until both SDKs are initialised. Maybe in the future we could consider that, if this ever becomes a problem.

As an aside, we're overwriting CrashedLastRun, EnableScopeSync and ScopeObserver here, so any user configured values for these will get overwritten. If that's genuinely what we want, those options should probably be internal (not public). I'll create a separate issue for that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these being public may be related to Unity

{
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)
{
Expand All @@ -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
Expand All @@ -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;
}

/// <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)
{
// 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
Expand Down
41 changes: 41 additions & 0 deletions test/Sentry.Tests/SentrySdkTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<IBackgroundWorker>(),
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()
{
Expand Down
Loading