-
-
Notifications
You must be signed in to change notification settings - Fork 226
Disable sentry-native for WASM applications #4631
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## version6 #4631 +/- ##
===========================================
Coverage ? 73.08%
===========================================
Files ? 479
Lines ? 17381
Branches ? 3430
===========================================
Hits ? 12703
Misses ? 3819
Partials ? 859 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| <!-- net8.0 or greater --> | ||
| <FrameworkSupportsNative Condition="'$(_SentryIsNet8OrGreater)' != 'true' or !('$(OutputType)' == 'Exe' or '$(OutputType)' == 'WinExe')">false</FrameworkSupportsNative> | ||
| <!-- WASM --> | ||
| <FrameworkSupportsNative Condition="'$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true'">false</FrameworkSupportsNative> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: WASM Disable Condition Overrides User Setting
The WASM disabling condition for FrameworkSupportsNative is evaluated before the explicit user opt-in/opt-out. This allows users to override the WASM disable by setting SentryNative=true, potentially reintroducing build errors for WASM applications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was intentional yes...
| ### Fixes | ||
|
|
||
| - The SDK avoids redundant scope sync after transaction finish ([#4623](https://github.com/getsentry/sentry-dotnet/pull/4623)) | ||
| - sentry-native is now automatically disabled for WASM applications ([#4631](https://github.com/getsentry/sentry-dotnet/pull/4631)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: is this clearer?
| - sentry-native is now automatically disabled for WASM applications ([#4631](https://github.com/getsentry/sentry-dotnet/pull/4631)) | |
| - sentry-native is now disabled per default for WASM applications ([#4631](https://github.com/getsentry/sentry-dotnet/pull/4631)) |
Which phrasing is conveying more clearly, that we disable it per default, but it could still be manually enabled?
On the other hand, it doesn't really make sense to manually enable sentry-native for Blazor-WASM.
I actually do prefer now as-is ... but wanted to keep the comment anyway.
Addresses this error when trying to build WASM client applications on Windows:
May also resolve #4550
Although initial feedback from the customer is that it does not. Hard to verify as I haven't been able to reproduce that issue yet.