Log each inner exception of an aggregate exception in TaskLoggingHelper.LogErrorFromException()

[jeffkl]

Fixes #7985

Context

TaskLoggingHelper.LogErrorFromException() does not currently take into account the relatively new AggregateException which has inner exceptions but an outer exception with very little details.

Changes Made

I've updated TaskLoggingHelper.LogErrorFromException() to check if the specified exception is an AggregateException and call the method again for each inner exception, respecting all of the arguments passed in around showing details or a stack trace.

Testing

A unit test was added

Notes

Unfortunately, I can't add the other improvement around an InvalidProjectFileException since TaskLoggingHelper is compiled into Microsoft.Build.Utilities.Core and that assembly does not reference Microsoft.Build.dll so it doesn't have access to the InvalidProjectFileException class 😢

[jeffkl]

Unfortunately, I can't add the other improvement around an InvalidProjectFileException since TaskLoggingHelper is compiled into Microsoft.Build.Utilities.Core and that assembly does not reference Microsoft.Build.dll so it doesn't have access to the InvalidProjectFileException class 😢

The only way to make it work with the InvalidProjectFileException natively is to let Microsoft.Build.Utilities.Core have a compile-time reference to Microsoft.Build which I don't think will happen any time soon. For some reasons we didn't want that dependency so TaskLoggingHelper and a few other classes are compiled into each assembly.

I could use Reflection if anyone would approve of that 😈

[Forgind]

I'm curious how much work it'd be to make M.B.Utilities reference M.B. Probably not worth it.

[jeffkl]

I'm curious how much work it'd be to make M.B.Utilities reference M.B. Probably not worth it.

You'd need to have all of these compiled only into Microsoft.Build:

https://github.com/dotnet/msbuild/blob/cc3db358d34ad4cd1ec0c67e17582d7ca2a15040/src/Utilities/Microsoft.Build.Utilities.csproj#L43-L149

Then you'd need to update the ones that compile themselves into a different namespace depending on the project:

https://github.com/dotnet/msbuild/blob/cc3db358d34ad4cd1ec0c67e17582d7ca2a15040/src/Shared/TaskLoggingHelper.cs#L21-L25

I have this memory of trying it years ago but I can't remember now exactly why Microsoft.Build and Microsoft.Build.Utilities.Core aren't allowed to be friends. I found this issue which explains a little of what I remember. But that was six years ago, maybe its time for these DLLs to know about each other?

[Forgind]

I love the idea of removing all those extra compilations. Adding new dependencies between our assemblies can be very efficient, but that issue reminded me of what happened when I added a direct reference to Framework in SolutionFile.cs: someone had been calling Parse without loading Framework, and it suddenly started needing Framework --> their scenario crashed. I'd love to add the dependency, but now I'm worried it would be ugly work to make a breaking change we'd end up reverting.