Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
10 changes: 5 additions & 5 deletions src/Sentry/Platforms/Cocoa/SentrySdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,6 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes(
/// </summary>
internal static CocoaSdk.SentryEvent? ProcessOnBeforeSend(SentryOptions options, CocoaSdk.SentryEvent evt, IHub hub)
{
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.
Expand Down Expand Up @@ -239,6 +234,11 @@ private static CocoaSdk.SentryHttpStatusCodeRange[] GetFailedRequestStatusCodes(
}
}

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