Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.NET 6 iOS support #1282

Closed
1 task
bruno-garcia opened this issue Oct 19, 2021 · 12 comments
Closed
1 task

.NET 6 iOS support #1282

bruno-garcia opened this issue Oct 19, 2021 · 12 comments
Assignees
Labels
Feature New feature or request iOS MAUI

Comments

@bruno-garcia
Copy link
Member

bruno-garcia commented Oct 19, 2021

Since .NET 6 landed (#939 ) we can look into adding net6-ios and including sentry-cocoa.
See: https://github.com/xamarin/xamarin-macios/blob/861c40dbe45478b848ac1d165314e05de012bf0f/tests/dotnet/NativeXCFrameworkReferencesApp/shared.csproj

When we bundle the Sentry iOS SDK we need to take into account that Mono uses some signals to interpret NullReferenceExceptions and need special case (run before other crash reports do):

See: https://www.mono-project.com/docs/advanced/signals/#incomplete-solution
See also: dotnet/runtime#44736

This is probably something we can do in the binding project, before calling Init in the iOS SDK.

Proposed solution, based on the Mono docs:

try {
} finally {
    Mono.Runtime.RemoveSignalHandlers ();
    try {
        // Init the Sentry iOS SDK:
        SentrySDK.start(...);
    } finally {
        Mono.Runtime.InstallSignalHandlers ();
    }
}

Since Mono’s signal handlers are reinstalled and not restored, Mono will now chain to the signal handlers installed by EnableCrashReporting.

The code is executed in a finally block, so that the Mono runtime will never abort it under any circumstance.

It’s recommended to do this as early as possible when launching the process, in particular before starting any secondary threads. If any signals are raised between removing and reinstalling Mono’s signal handlers (this includes NullReferenceExceptions), the app will crash.

See: https://www.mono-project.com/docs/advanced/signals/#complete-solution

@gsomix
Copy link

gsomix commented Jan 19, 2022

@lucas-zimerman Hey, any news on it?

@lucas-zimerman
Copy link
Collaborator

@lucas-zimerman Hey, any news on it?

Currently, it's on hold but we'll return to it near to MAUI release date.

@bruno-garcia
Copy link
Member Author

@gsomix anything specific you're looking for?

The SDK should work on iOS, just not capturing native crashes

@Dolfik1
Copy link

Dolfik1 commented Jan 28, 2022

The SDK should work on iOS, just not capturing native crashes

This is the main problem. Native crashes are also very useful information. Many native crashes can be caused by managed code.

It would also be important to have information about the platform, device model, os version, cpu model, etc. This is important especially on Android.

@Dolfik1
Copy link

Dolfik1 commented Jan 28, 2022

I also want to draw attention to an exception in the getsentry/sentry-xamarin#97

This exception is thrown inside sentry-dotnet:
https://github.com/getsentry/sentry-dotnet/blob/main/src/Sentry/Internal/Http/DefaultSentryHttpClientFactory.cs#L34

I don't know how this used to work, but the Xamarin.iOS implementation of HttpClient does not have an implementation for SupportsAutomaticDecompression:
https://github.com/xamarin/xamarin-macios/blob/main/src/Foundation/NSUrlSessionHandler.cs#L599
xamarin/xamarin-macios#13579

I checked our project (old Xamarin.iOS) and it uses NSUrlSessionHandler as default handler:

<MtouchHttpClientHandler>NSUrlSessionHandler</MtouchHttpClientHandler>

But it stopped working after porting the project to .Net 6 (getsentry/sentry-xamarin#97)

The recommended handler is NSUrlSessionHandler. You can read more about HTTP stack on iOS:
https://docs.microsoft.com/en-us/xamarin/cross-platform/macios/http-stack

So it seems that the problem is really in sentry-dotnet.

@bruno-garcia
Copy link
Member Author

It seems the problem is really in .NET 6 in this case. In the iOS implementation, that's throwing:

Unhandled managed exception: Operation is not supported on this platform. 
(System.PlatformNotSupportedException)
   at System.Net.Http.NSUrlSessionHandler.get_SupportsAutomaticDecompression()

Perhaps disabling compression on the client could be a work around

@bruno-garcia
Copy link
Member Author

The Sentry.Xamarin package relies on Xamarin.Essentials which is now part of .NET MAUI. So we could add the device information bits for MAUI (on a Sentry.Maui) but I don't believe we'll go through the effort of adding #if throughout and call into MonoTouch and Xamarin.Android APIs to get device data (basically replicating what Essentials does).

That said improving support (including a work around so that it doesn't throw when using a net6 iOS build) is needed here and we'd love to get come contributions

@bruno-garcia
Copy link
Member Author

We're going to tackle this soon

@mattjohnsonpint
Copy link
Contributor

I started playing with this, and and found a few things:

  • With the latest release of xamarin-macios, things have changed significantly. The Xamarin SDK now uses the .NET infrastructure instead of Mono, and is now installed via .NET Workloads (sudo dotnet workload install ios). It's unclear to me if that means Mono is still used at runtime or if it's completely CoreCLR now.

  • From a managed-only perspective, I can't seem to catch an unhandled exception anywhere. Some previous posts (like this one) mention hooking AppDomain.CurrentDomain.UnhandledException and TaskScheduler.UnobservedTaskException from FinishedLaunching in AppDelegate.cs, but that doesn't seem to work now.

  • When you throw an unhandled exception while debugging on Rider or Visual Studio, the IDE seems to catch the exception repeatedly until you stop debugging and then it eventually crashes the app. Indeed, if you hook AppDomain.CurrentDomain.FirstChanceException and just Console.Write something, then throw an exception during startup and run on the command line, you'll see the event gets called over and over, flooding the screen until you eventually get a native crash. I believe this is related to Infinite recursion with unhandled managed exceptions and exception marshaling is set to throw xamarin/xamarin-macios#14796.

  • There's some additional requests for a global exception handler for iOS already, such as Provide a global uncaught exception handler similar to WPF and other toolkits xamarin/xamarin-macios#12040

  • The same results happen with maccatalyst, as would be expected.

Hopefully we can work through some of this by leveraging our native SDK.

@bruno-garcia
Copy link
Member Author

bruno-garcia commented Jun 16, 2022

Thanks for sharing these findings.

It's unclear to me if that means Mono is still used at runtime or if it's completely CoreCLR now.

Mono is the runtime for mobile in .NET 6 👍

When you throw an unhandled exception while debugging on Rider or Visual Studio, the IDE seems to catch the exception repeatedly until you stop debugging and then it eventually crashes the app.

On iOS, you can't debug through a signal handler, so crash reporting is hard to debug. We test the iOS native SDK for hard crashes only without a debugger attached. Could it be related? Seems not to be the case now that I followed the issue you raised with .NET folks

Binding docs seem to still be under Xamarin only: https://docs.microsoft.com/en-us/xamarin/cross-platform/macios/binding/objective-c-libraries?tabs=macos

@filipnavara shared a link to some tests folks do on bindings that can help us out: https://github.com/xamarin/xamarin-macios/tree/f1e4cbcfcdd887819b493460441da187e0e2f11b/tests/bindings-xcframework-test/dotnet

A few findings from the macios tests that are not on the docs:

Relevant properties:

    <IsBindingProject>true</IsBindingProject>
    <NoBindingEmbedding>true</NoBindingEmbedding>
    <!-- Don't remove native symbols, because it makes debugging native crashes harder -->
    <MtouchNoSymbolStrip>true</MtouchNoSymbolStrip>

Binding static lib

    <ObjcBindingNativeLibrary Include="$(TestLibrariesDirectory)\.libs\$(NativeLibName)\libtest2.a">
      <Link>libtest2.a</Link>
    </ObjcBindingNativeLibrary>

framework and xcframework:

    <NativeReference Include="$(TestLibrariesDirectory)\.libs\XTest.xcframework">
      <Kind>Framework</Kind>
      <SmartLink>False</SmartLink>
      <Frameworks>CoreLocation ModelIO</Frameworks>
    </NativeReference>
    <NativeReference Include="..\..\..\tests\test-libraries\.libs\ios-fat\XTest.framework">
      <Kind>Framework</Kind>
      <SmartLink>False</SmartLink>
      <Frameworks Condition="'$(TargetFrameworkIdentifier)' != 'Xamarin.WatchOS'">CoreLocation ModelIO</Frameworks>
      <Frameworks Condition="'$(TargetFrameworkIdentifier)' == 'Xamarin.WatchOS'">CoreLocation</Frameworks>
    </NativeReference>
// Old bindings also relied on, if needed. Unclear if we still need it:
[assembly: LinkerSafe]

@mattjohnsonpint
Copy link
Contributor

mattjohnsonpint commented Jun 16, 2022

... the issue you raised with .NET folks

For reference: xamarin/xamarin-macios#15252

And I've applied that workaround here:

#if IOS || MACCATALYST
// Workaround for https://github.com/xamarin/xamarin-macios/issues/15252
ObjCRuntime.Runtime.MarshalManagedException += (_, args) =>
{
args.ExceptionMode = ObjCRuntime.MarshalManagedExceptionMode.UnwindNativeCode;
};
#endif

It's in Sentry.Maui for now. We should move it to Sentry when we add the mac targets there. It doesn't need to be in both.

@mattjohnsonpint
Copy link
Contributor

Completed in #1829

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request iOS MAUI
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants