fix: [iOS / Native AOT] - prevent use of unsupported marshalling behaviour

bricefriha commented 3 weeks ago

This PR aims to fix #3280, preventing the use of unsupported marshalling behaviour UnwindNativeCode for Native AOT by disabling Marshal Exceptions when the code is compiled via native AOT (as advised here).

Testing

Testing these changes is more tricky than usual since we did find a way to run this from Unit Tests.

The best way to do this is to: [on a MAC with xcode installed]

Clone the sentry-dotnet repository (make sure you don't do it on a directory that is linked to iCloud)
Git checkout fix/#3680
From the open the terminal and move to the sample directory: cd /Samples/Sentry.Samples.Maui
Plug a physical device via USB (if your device is not in developer mode: open xcode, it will setup the device)
then run `dotnet publish -c Release --framework net8.0-ios /t:Run /p:_DeviceName={DeviceID}

If you don't know the ID of your device (DeviceID): either run xcrun xctrace list devices, or head to Window -> Devices and Simulators -> device id is under Identifier
wait until the app opens on your device
terminate the process
Open the app again on your phone
Once open press the "Throw Unhandled .NET Exception (Crash)"
Head to Sentry, you should see the event popping up

Known error while testing (and their workaround)

error : strip exited with code 13: this is a known issue with Xcode 16 (now fixed on Xcode16.2 beta) If you get this error, add the following line to the .csproj of the maui project:
```
<MtouchNoSymbolStrip Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'ios'">True</MtouchNoSymbolStrip>
```

jamescrosswell commented 3 weeks ago

Also it was enabled by default in .NET for a reason: https://github.com/xamarin/xamarin-macios/wiki/.NET-release-notes-Xcode-13.3#exception-marshalling-is-enabled-by-default

That link is interesting... it seems to imply this setting could be controlled at build time, rather than runtime (what we're doing)... so it might be possible to set this automatically to the appropriate value via our transitive build script rather than waiting until runtime.

If we are going to do it at runtime, it might be possible to pick an appropriate value automatically based on whether or not the user's application is compiled AOT or not (rather than hoping they configure the right value via the native options). We have an AotHelper that might be used to detect whether the app is AOT compiled or not, but I'm a bit concerned that the current implementation of this picks up apps that are trimmed but not AOT... so if we can do it at compile time that would be better: https://github.com/getsentry/sentry-dotnet/blob/9f36d22f3b216351d5957b74e831e8b92636e6ba/src/Sentry/Internal/AotHelper.cs#L15-L14

bricefriha commented 2 weeks ago

@jamescrosswell

We have an AotHelper that might be used to detect whether the app is AOT compiled or not, but I'm a bit concerned that the current implementation of this picks up apps that are trimmed but not AOT...

Looking at the implementation of AotHelper .IsAOT, I got the same impression. Should we do !(!RuntimeFeature.IsDynamicCodeCompiled && !RuntimeFeature.IsDynamicCodeSupported) instead then? https://github.com/dotnet/runtime/issues/76739#issuecomment-1272236523

so it might be possible to set this automatically to the appropriate value via our transitive build script rather than waiting until runtime.

I like this idea

jamescrosswell commented 2 weeks ago

then run `dotnet publish -c Release --framework net8.0-ios /t:Run /p:_DeviceName={DeviceID}

There are a few more hoops to jump through before you can do this right? You have to setup a provisioning profile for the app in the apple developer portal first?

I'm just waiting for Apple to renew my annual membership to the developer portal so I can test.

bricefriha commented 2 weeks ago

There are a few more hoops to jump through before you can do this right? You have to setup a provisioning profile for the app in the apple developer portal first?

maybe? I'm not sure. I think Xcode creates a temporary one

you can also use a simulator I think, btw

bruno-garcia commented 1 week ago

is this waiting on validation? Can I help? or we put an alpha out and as folks reporting the issue?

bricefriha commented 1 week ago

is this waiting on validation? Can I help?

Yes @jamescrosswell is going to run some tests to validate this.

or we put an alpha out and as folks reporting the issue?

I think it would be a good idea so James doesn't have to jump through hoops to test this, and I can also create a repo. However, I think James wanted to compare the behaviours of this branch with the one of the main branch

jamescrosswell commented 1 week ago

Yes @jamescrosswell is going to run some tests to validate this.

Sorry for the delay. I've been trying to resolve https://github.com/getsentry/sentry-dotnet/pull/3764 (which is blocking this PR and anything else that targets main).

jamescrosswell commented 1 week ago

@bricefriha I finally carved out some time to setup a provisioning profile etc. so that I can test this. At the moment, I'm not able to publish this to test though - I'm running into trim analysis errors like:

  Sentry.Samples.Maui net9.0-ios failed with 4 error(s) (23.1s) → bin/Release/net9.0-ios/ios-arm64/Sentry.Samples.Maui.dll
    /code/sentry-dotnet/samples/Sentry.Samples.Maui/obj/Release/net9.0-ios/ios-arm64/Microsoft.Maui.Controls.SourceGen/Microsoft.Maui.Controls.SourceGen.CodeBehindGenerator/Resources_Styles_Styles.xaml.sg.cs(33): Trim analysis error IL2026: __XamlGeneratedCode__.__Type68A1B57D17E9473E.InitializeComponent(): Using member 'Microsoft.Maui.Controls.Xaml.OnPlatformExtension.Default.get' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. The OnPlatformExtension is not trim safe. Use OnPlatform<T> instead.
    /code/sentry-dotnet/samples/Sentry.Samples.Maui/obj/Release/net9.0-ios/ios-arm64/Microsoft.Maui.Controls.SourceGen/Microsoft.Maui.Controls.SourceGen.CodeBehindGenerator/Resources_Styles_Styles.xaml.sg.cs(33): Trim analysis error IL2026: __XamlGeneratedCode__.__Type68A1B57D17E9473E.InitializeComponent(): Using member 'Microsoft.Maui.Controls.Xaml.OnPlatformExtension.Default.set' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. The OnPlatformExtension is not trim safe. Use OnPlatform<T> instead.
    /code/sentry-dotnet/samples/Sentry.Samples.Maui/obj/Release/net9.0-ios/ios-arm64/linked/System.Text.Json.dll : error IL3053: Assembly 'System.Text.Json' produced AOT analysis warnings.

This may be unrelated to your PR and more related to the fact that we bumped to net9.0... it's coming from the source generators, which is a bit odd. We might be referencing the wrong version of something.

It does, however, still prevent me from being able to test this 😞

bricefriha commented 1 week ago

This may be unrelated to your PR and more related to the fact that we bumped to net9.0... it's coming from the source generators, which is a bit odd. We might be referencing the wrong version of something.

That shouldn't be related because you should at least be able to publish the app entirely. I would try again with .net8.0 (for troubleshooting sake)

bricefriha commented 6 days ago

Sentry.Samples.Maui net9.0-ios failed with 4 error(s) (23.1s) → bin/Release/net9.0-ios/ios-arm64/Sentry.Samples.Maui.dll /code/sentry-dotnet/samples/Sentry.Samples.Maui/obj/Release/net9.0-ios/ios-arm64/Microsoft.Maui.Controls.SourceGen/Microsoft.Maui.Controls.SourceGen.CodeBehindGenerator/Resources_Styles_Styles.xaml.sg.cs(33): Trim analysis error IL2026: __XamlGeneratedCode__.__Type68A1B57D17E9473E.InitializeComponent(): Using member 'Microsoft.Maui.Controls.Xaml.OnPlatformExtension.Default.get' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. The OnPlatformExtension is not trim safe. Use OnPlatform<T> instead. /code/sentry-dotnet/samples/Sentry.Samples.Maui/obj/Release/net9.0-ios/ios-arm64/Microsoft.Maui.Controls.SourceGen/Microsoft.Maui.Controls.SourceGen.CodeBehindGenerator/Resources_Styles_Styles.xaml.sg.cs(33): Trim analysis error IL2026: __XamlGeneratedCode__.__Type68A1B57D17E9473E.InitializeComponent(): Using member 'Microsoft.Maui.Controls.Xaml.OnPlatformExtension.Default.set' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. The OnPlatformExtension is not trim safe. Use OnPlatform<T> instead. /code/sentry-dotnet/samples/Sentry.Samples.Maui/obj/Release/net9.0-ios/ios-arm64/linked/System.Text.Json.dll : error IL3053: Assembly 'System.Text.Json' produced AOT analysis warnings.

~~it sounds like something we have to remove /bin /obj file and rebuild to get it fixed.~~ but I'll dig deeper on my side

Update

I experience the same issue now with .net9

getsentry / sentry-dotnet