Sentry and unhandled exceptions

[bruno-garcia]

This isn't really an issue, just sharing some details about Sentry: I was peeking at the API usage of Sentry and noticed you have your own implementation of UnhandledException:

StabilityMatrix/StabilityMatrix/App.xaml.cs

Lines 394 to 397 in 3866e57

	if (SentrySdk.IsEnabled)
	{
	SentrySdk.CaptureException(e.Exception);
	}

Sentry sets the mechanism, which helps display crashes in Sentry vs non-crashes. With the read banner unhandled:

https://github.com/getsentry/sentry-dotnet/blob/407dc28dbd7f1c29059fc509a886b114a2c75b0b/src/Sentry/Integrations/AppDomainUnhandledExceptionIntegration.cs#L32-L47

For example, non-crash vs crashed:

Sentry by default de-duplicates a capture that involves the same exception instance. So the second attempt to capture it will just drop the event:

https://github.com/getsentry/sentry-dotnet/blob/407dc28dbd7f1c29059fc509a886b114a2c75b0b/src/Sentry/Internal/DuplicateEventDetectionEventProcessor.cs#L14-L28

So it's possible that your code trying to capture the event with Sentry is not really resulting in a capture. Or, if yours run before Sentry's (you'd notice the unhandled missing), Sentry's default unhandled exception handling is not capturing it, but at least it's Flush is running. You can disable the default Sentry behavior:

https://docs.sentry.io/platforms/dotnet/configuration/disable-integrations/#disableappdomainunhandledexceptioncapture

But I'd recommend keeping the Sentry default integration, as we might continue to improve it and you get that for free. If you decide to remove it, worth adding Flush to your own, as we did.

One smaller thing: Enhanced is the default mode. We'd just not use that if not possible (e.g: Unity, NativeAOT, etc):

StabilityMatrix/StabilityMatrix/App.xaml.cs

Line 95 in 3866e57

o.StackTraceMode = StackTraceMode.Enhanced;

https://github.com/getsentry/sentry-dotnet/blob/407dc28dbd7f1c29059fc509a886b114a2c75b0b/src/Sentry/SentryOptions.cs#L866

[ionite34]

Thanks for the insight. I think we were getting the behavior of the AppDomain exceptions not marked as "Unhandled". In this case I assume just getting rid of our SentrySdk.CaptureException and logger?.LogCritical (Since we also use the NLog Sentry integration for critical levels), would enable Sentry to automatically handle the AppDomain exception?

Though, not sure if our App Shutdown and Environment.Exit(1) in the CurrentDomain_UnhandledException function would interfere with the default integration capture?

[bruno-garcia]

In this case I assume just getting rid of our SentrySdk.CaptureException and logger?.LogCritical

Right, the Sentry SDK will already deal with the NLog log levels

From our docs, you can configure it though:

        // Debug and higher are stored as breadcrumbs (default is Info)
        o.MinimumBreadcrumbLevel = LogLevel.Debug;
        // Error and higher is sent as event (default is Error)
        o.MinimumEventLevel = LogLevel.Error;

Though, not sure if our App Shutdown and Environment.Exit(1) in the CurrentDomain_UnhandledException function would interfere with the default integration capture?

Before calling Environment.Exit ideally you call SentrySdk.Close to force flush all events out.
We have an integration by default that should do this: https://github.com/getsentry/sentry-dotnet/blob/aa5fc75ff0f3b91e4d3c90f7e25b615ad501f0f9/src/Sentry/Integrations/AppDomainProcessExitIntegration.cs
But prob best to do it before hand to make sure (I found this hook from .NET kind of flaky back in the day)

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove the stale label or comment, else this will be closed in 5 days.

[github-actions]

This issue was closed because it has been stale for 5 days with no activity.