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

fix: incorrect use of SentryNative within a WASM app #3363

Merged
merged 11 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
2 changes: 1 addition & 1 deletion .github/actions/environment/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ runs:
shell: bash
run: >
dotnet workload install \
maui-android \
wasm-tools maui-android \
${{ runner.os == 'macOS' && 'maui-ios maui-maccatalyst maui-windows macos' || '' }} \
${{ runner.os == 'Windows' && 'maui-windows' || '' }} \
--temp-dir "${{ runner.temp }}" \
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Fixes

- Remove SentryNative initialization from blazor WASM with `RunAOTCompilation` enabled. ([#3363](https://github.com/getsentry/sentry-dotnet/pull/3363))
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved

### Dependencies

- Bump Hangfire ([#3361](https://github.com/getsentry/sentry-dotnet/pull/3361))
Expand Down
4 changes: 2 additions & 2 deletions integration-test/runtime.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ internal class FakeTransport : ITransport
}

It "'dotnet publish' produces an app that's recognized as AOT by Sentry" {
runConsoleApp | Should -AnyElementMatch 'This looks like a NativeAOT application build.'
runConsoleApp | Should -AnyElementMatch 'This looks like a Native AOT application build.'
}

It "'dotnet run' produces an app that's recognized as JIT by Sentry" {
runConsoleApp $false | Should -AnyElementMatch 'This looks like a standard JIT/AOT application build.'
runConsoleApp $false | Should -AnyElementMatch 'This doesn''t look like a Native AOT application build.'
}

It 'Produces the expected exception (Managed, AOT=<_>)' -ForEach @($true, $false) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk.BlazorWebAssembly">

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>
<RunAOTCompilation>true</RunAOTCompilation>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Components.WebAssembly" Version="6.0.19" />
<PackageReference Include="Microsoft.AspNetCore.Components.WebAssembly.DevServer" Version="6.0.0" PrivateAssets="all" />
<PackageReference Include="Microsoft.AspNetCore.Components.WebAssembly" Version="8.0.3"/>
<PackageReference Include="Microsoft.AspNetCore.Components.WebAssembly.DevServer" Version="8.0.3" PrivateAssets="all"/>
<ProjectReference Include="..\..\src\Sentry\Sentry.csproj" />
<ProjectReference Include="..\..\src\Sentry.Extensions.Logging\Sentry.Extensions.Logging.csproj" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
<title>BlazorApp1</title>
<base href="/" />
</head>

<body>
Expand Down
1 change: 1 addition & 0 deletions src/Sentry/Internal/AotHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ private class AotTester
}

#if NET8_0_OR_GREATER
// TODO this probably more closely represents trimming rather than NativeAOT?
internal static bool IsNativeAot { get; }

[UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = AotHelper.SuppressionJustification)]
Expand Down
2 changes: 1 addition & 1 deletion src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ public void Dispose()
#elif ANDROID
// TODO
#elif NET8_0_OR_GREATER
if (AotHelper.IsNativeAot)
if (SentryNative.IsAvailable)
{
_options?.LogDebug("Closing native SDK");
SentrySdk.CloseNativeSdk();
Expand Down
20 changes: 20 additions & 0 deletions src/Sentry/Platforms/Native/SentryNative.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using Sentry.Internal;
using Sentry.PlatformAbstractions;

// ReSharper disable once CheckNamespace
namespace Sentry;

internal static class SentryNative
{
#if NET8_0_OR_GREATER
internal static bool IsAvailable { get; }

static SentryNative()
{
IsAvailable = AotHelper.IsNativeAot && !SentryRuntime.Current.IsBrowserWasm();
}
#else
// This is a compile-time const so that the irrelevant code is removed during compilation.
internal const bool IsAvailable = false;
#endif
}
13 changes: 0 additions & 13 deletions src/Sentry/SentryClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,6 @@ public SentryClient(SentryOptions options)

options.SetupLogging(); // Only relevant if this client wasn't created as a result of calling Init

if (AotHelper.IsNativeAot)
#pragma warning disable CS0162 // Unreachable code detected
{
#pragma warning disable 0162 // Unreachable code on old .NET frameworks
options.LogDebug("This looks like a NativeAOT application build.");
#pragma warning restore 0162
}
else
{
#pragma warning restore CS0162 // Unreachable code detected
options.LogDebug("This looks like a standard JIT/AOT application build.");
}

if (worker == null)
{
var composer = new SdkComposer(options);
Expand Down
4 changes: 2 additions & 2 deletions src/Sentry/SentryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -975,14 +975,14 @@ public StackTraceMode StackTraceMode
/// </summary>
/// <remarks>
/// Note that the highest precision value relies on <see cref="System.Diagnostics.Process.GetCurrentProcess"/>
/// which might not be available. For example on Unity's IL2CPP.
/// which might not be available. For example on Unity's IL2CPP or WebAssembly.
/// Additionally, "Best" mode is not available on mobile platforms.
/// </remarks>
public StartupTimeDetectionMode DetectStartupTime { get; set; } =
#if __MOBILE__
StartupTimeDetectionMode.Fast;
#else
StartupTimeDetectionMode.Best;
SentryRuntime.Current.IsBrowserWasm() ? StartupTimeDetectionMode.Fast : StartupTimeDetectionMode.Best;
#endif

/// <summary>
Expand Down
19 changes: 16 additions & 3 deletions src/Sentry/SentrySdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,19 @@ internal static IHub InitHub(SentryOptions options)
options.LogWarning("The provided DSN that contains a secret key. This is not required and will be ignored.");
}

if (AotHelper.IsNativeAot)
#pragma warning disable CS0162 // Unreachable code detected
{
#pragma warning disable 0162 // Unreachable code on old .NET frameworks
options.LogDebug("This looks like a Native AOT application build.");
#pragma warning restore 0162
}
else
{
#pragma warning restore CS0162 // Unreachable code detected
options.LogDebug("This doesn't look like a Native AOT application build.");
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with the // TODO this probably more closely represents trimming rather than NativeAOT?. Enabling AOT compilation and then reading this in the logs does seem odd.

}

// Initialize native platform SDKs here
if (options.InitNativeSdks)
{
Expand All @@ -57,7 +70,7 @@ internal static IHub InitHub(SentryOptions options)
#elif ANDROID
InitSentryAndroidSdk(options);
#elif NET8_0_OR_GREATER
if (AotHelper.IsNativeAot)
if (SentryNative.IsAvailable)
{
InitNativeSdk(options);
}
Expand Down Expand Up @@ -709,9 +722,9 @@ public static void CauseCrash(CrashType crashType)
break;
#elif NET8_0_OR_GREATER
case CrashType.Native:
if (!AotHelper.IsNativeAot)
if (!SentryNative.IsAvailable)
{
CurrentOptions?.LogError("The support for capturing native crashes is limited to AOT compilation.");
CurrentOptions?.LogError("The support for capturing native crashes is limited to AOT compilation on platforms with SentryNative support.");
return;
}

Expand Down
Loading