Check DebugStackTrace conditional execution/suppressions

[vaind]

Originally commented: dotnet/runtime#96528 (comment)

This is another good example of "whenever you feel you need to special case native AOT, you likely need to make a step back":

getsentry/sentry-dotnet@d1e5efc/src/Sentry/Internal/DebugStackTrace.cs#L511-L536

This is even suppressing the warning from the single file analyzer, incorrectly. That warning is still relevant to single file: Console.WriteLine(typeof(Program).Module.FullyQualifiedName); with PublishSingleFile=true will print so this is blindly trying to open a file named . There's a try/catch, so there's at least that. But no native AOT special casing is needed here. What is needed is single file special casing.

[bitsandfoxes]

Could you give an update or add context? I take it we should get rid of the check. What would the alternative look like?

[vaind]

I have none, I've just proxied the original linked suggestion so it is tracked

[jamescrosswell]

I think what @MichalStrehovsky is saying is that we're suppressing a warning that is relevant to publishing single file applications.

In the case that we're publishing an AOT app, yes it will be a single file app and so that warning is relevant. We then have this code to compensate for the suppression:

sentry-dotnet/src/Sentry/Internal/DebugStackTrace.cs

Lines 515 to 519 in d1e5efc

	if (AotHelper.IsNativeAot)
	{
	assemblyName = null;
	return null;
	}

So the problem code never gets executed when publishing an AOT application.

However if the application is not compiled AOT but is published as a Single file, the logic we have to avoid the problem code is not executed and we've also suppressed the warning (which is relevant to Single File apps)... so we can get into trouble.

We never actually had any analysers in place to warn us about issues publishing single file applications so we may have had problems when using Sentry in Single File apps since long before we added support for AOT.

I think the solution is to find a suppression method that is a bit more discriminating.

The analyzers themselves can be enabled or disabled independently. Adding IsAotCompatible will enable all of the following analyzers:

• IsTrimmable
• EnableTrimAnalyzer
• EnableSingleFileAnalyzer
• EnableAotAnalyzer

We probably want to enable EnableSingleFileAnalyzer unconditionally then, across all target frameworks.

The question is then what to do about the UnconditionalSuppressMessage (or alternatively what to do about the code that won't work in Single File apps). We could do something similar to what we've done for AOT - find some way to detect if the app is a single file app and just return null in this case... never executing the code that would be problematic in single file apps.

[MichalStrehovsky]

What should work is to delete the AotHelper.IsNativeAot codepath and instead add:

if (assemblyName == "<unknown>")
{
    assemblyName = null;
    return null;
}

after the assemblyName = module.FullyQualifiedName; line. This assumes the callers will be fine with this and don't assume this only returns null on native AOT or something.

Then it should be possible to keep the suppression because you're handling the single file case (native AOT is only a subset of the more broad single file case).

[jamescrosswell]

Thank you @MichalStrehovsky - very much appreciated!