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

Need better diagnosability if NetstandardAwareAssemblyResolver throws #1196

Closed
KirillOsenkov opened this issue Jul 13, 2021 · 4 comments · Fixed by #1230
Closed

Need better diagnosability if NetstandardAwareAssemblyResolver throws #1196

KirillOsenkov opened this issue Jul 13, 2021 · 4 comments · Fixed by #1230
Labels
enhancement General enhancement request

Comments

@KirillOsenkov
Copy link

Our CI is using code coverage on .NET SDK 3.0.100.

When trying to update to SDK to 5.0.301 the instrumentation fails:

C:\Users\kirill\.nuget\packages\coverlet.msbuild\2.6.3\build\coverlet.msbuild.targets(7,5): [coverlet] Unable to instrument module: C:\.....\Release\net472\.......dll because : The type initializer for 'Coverlet.Core.Instrumentation.NetstandardAwareAssemblyResolver' threw an exception. 

From looking at the static constructor, it only catches the FileNotFoundException:

static NetstandardAwareAssemblyResolver()
{
try
{
// To be sure to load information of "real" runtime netstandard implementation
_netStandardAssembly = System.Reflection.Assembly.LoadFile(Path.Combine(Path.GetDirectoryName(typeof(object).Assembly.Location), "netstandard.dll"));
System.Reflection.AssemblyName name = _netStandardAssembly.GetName();
_name = name.Name;
_publicKeyToken = name.GetPublicKeyToken();
_assemblyDefinition = AssemblyDefinition.ReadAssembly(_netStandardAssembly.Location);
}
catch (FileNotFoundException)
{
// netstandard not supported
}
}

I think the Instrumenter should be trying to unwrap the exception before reporting it:

_logger.LogWarning($"Unable to instrument module: '{_module}' because : {ex.Message}");

@KirillOsenkov
Copy link
Author

I'm guessing it should be calling ToString() instead of the message, and possibly report the InnerException(s) recursively. Otherwise it's really hard to diagnose what is going on.

@KirillOsenkov
Copy link
Author

Also perhaps it should be an error, not a warning?

@KirillOsenkov
Copy link
Author

I had to debug using VS Child Process Attach extension, and enable first-chance exceptions to see what's going on.

image

Turns out it was a bug where Mono.Cecil was bringing the wrong version of System.IO.FileSystem.Primitives.dll:
mono/mono#12692 (comment)

I had to update coverlet.msbuild from 2.6.3 to latest 3.0.3. 2.6.3 shipped with Cecil 0.10.1 which had the bug. The latest 3.0.3 ships with Cecil 0.11, which targets NetStandard 2.0 and doesn't have the bug.

Long story short, this would have been easier to investigate if the Instrumenter line 126 printed the entire exception instead of just the message.

I'd immediately see that it's a MissingMethodException: Method not found: 'Void System.IO.FileStream..ctor(System.String, System.IO.FileMode, System.IO.FileAccess, System.IO.FileShare)'. and I could search for it to find the Cecil bug.

@MarcoRossignoli
Copy link
Collaborator

Thx a lot for looking into this!
Next time you can also try to use some env flag to debug https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/Troubleshooting.md#enable-collector-instrumentation-debugging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General enhancement request
Projects
None yet
2 participants