Investigate why having `Microsoft.NET.Test.Sdk` as a dependency throw exceptions while scanning for Fable plugins

[MangelMaxime]

Description

While working on #3441 (via commit https://github.com/fable-compiler/Fable/commit/d212c2f21ae5043a4b386ed496ad5fa0776a89d6) a regression has been introduced.

Indeed, when trying to compile Python and Rust tests projects, an error was reported:

Unhandled exception. FSharp.Compiler.DiagnosticsLogger+ReportedError: The exception has been reported. This internal exception should now be caught at an error recovery point on the stack. Original message: The type 'NeutralResourcesLanguageAttribute' is required here and is unavailable. You must add a reference to assembly 'System.Resources.ResourceManager, Version=4.1.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.)

This error is caused by the lines:

let scanForPlugins =
    asm.Contents.Attributes |> Seq.exists (fun attr ->
        attr.AttributeType.TryFullName = Some "Fable.ScanForPluginsAttribute")

Which to me is not obvious why these lines would report an error. To fix this issue I decided to mimic the old behaviour of Fable which consist of ignoring exceptions at this place 🤫

I am opening this issue to keep track of this behaviour and if someone has an idea on why or how we can improve the situation please don't hesitate to step in.

I tried believe this problem only occur if the project has Microsoft.NET.Test.Sdk in its dependencies. So perhaps we could be a bit more restrictive on the DLL we skip.

[inosik]

You're right, the assembly that causes this is testhost.dll from Microsoft.TestPlatform.TestHost, which is a dependency of Microsoft.NET.Test.Sdk. It has the NeutralResourcesLanguageAttribute set, which comes from an unreferenced assembly. We could either add a reference to the (old) System.Resources.ResourceManager package, or just skip assemblies we can't handle.

[MangelMaxime]

Thank you for looking into it and explaining the cause of the problem.

I tried to add System.Resources.ResourceManager to the test project but it didn't change the problem.

Also, there are 3 .dll failing that comes with Microsoft.NET.Test.Sdk. For now, my solution has been to add a log about dlls that we could not check for Fable plugins and skip them.

The old behaviour was just to skip them but had the problem of making some situation hard to understand or make the user think that the compilation was correct when in fact it was not.

The new behaviour should be transparent for most of the users because I don't think they will depends on Microsoft.NET.Test.Sdk.

[Lanayx]

I have just stumbled upon the commit that was made for this issue, and found out that the exception should not be silently swallowed, since it makes it extremely difficult to find the reason of the error.

[MangelMaxime]

@Lanayx If I understand, when working on a Fable plugin, if we included the exception message in the exist log it would have helped you?

[Lanayx]

Correct. I've started with the plugin yesterday and was desperate to see the error there without having an idea of what went wrong.

[MangelMaxime]

Ok, I will add the inner error message.

Based on my tests it never included anything meaningful but Fable compiler plugins are not often used so I can have miss something here. Hopefully, it will not add too much noise / fear if it come up in someone build.