Log each inner exception of an aggregate exception in TaskLoggingHelper.LogErrorFromException() by jeffkl · Pull Request #7998 · dotnet/msbuild

jeffkl · 2022-09-21T20:19:37Z

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 😢

…er.LogErrorFromException()

Forgind

LGTM. The bug mentions InvalidProjectFileException as well; do you think it's worth leaving it open until that part is also resolved?

jeffkl · 2022-09-21T20:32:13Z

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 · 2022-09-21T20:34:36Z

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

jeffkl · 2022-09-21T20:43:54Z

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:

msbuild/src/Utilities/Microsoft.Build.Utilities.csproj

Lines 43 to 149 in cc3db35

    
           <ItemGroup Label="Shared Code"> 
        
             <Compile Include="..\Shared\AssemblyFolders\AssemblyFoldersEx.cs"> 
        
               <Link>Shared\AssemblyFolders\AssemblyFoldersEx.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\AssemblyFolders\AssemblyFoldersFromConfig.cs"> 
        
               <Link>Shared\AssemblyFolders\AssemblyFoldersFromConfig.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\AssemblyFolders\Serialization\AssemblyFolderCollection.cs"> 
        
               <Link>Shared\AssemblyFolders\Serialization\AssemblyFolderCollection.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\AssemblyFolders\Serialization\AssemblyFolderItem.cs"> 
        
               <Link>Shared\AssemblyFolders\Serialization\AssemblyFolderItem.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\EnvironmentUtilities.cs"> 
        
               <Link>Shared\EnvironmentUtilities.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\BuildEnvironmentHelper.cs"> 
        
               <Link>Shared\BuildEnvironmentHelper.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\CanonicalError.cs"> 
        
               <Link>Shared\CanonicalError.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\ExtensionFoldersRegistryKey.cs"> 
        
               <Link>Shared\ExtensionFoldersRegistryKey.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\FileDelegates.cs"> 
        
               <Link>Shared\FileDelegates.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\CopyOnWriteDictionary.cs"> 
        
               <Link>Shared\CopyOnWriteDictionary.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\EncodingUtilities.cs"> 
        
               <Link>Shared\EncodingUtilities.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\ErrorUtilities.cs"> 
        
               <Link>Shared\ErrorUtilities.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\EscapingUtilities.cs"> 
        
               <Link>Shared\EscapingUtilities.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\EventArgsFormatting.cs"> 
        
               <Link>Shared\EventArgsFormatting.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\ExceptionHandling.cs"> 
        
               <Link>Shared\ExceptionHandling.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\FileUtilities.cs"> 
        
               <Link>Shared\FileUtilities.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\FileMatcher.cs"> 
        
               <Link>Shared\FileMatcher.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\FileUtilitiesRegex.cs"> 
        
               <Link>Shared\FileUtilitiesRegex.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\FrameworkLocationHelper.cs"> 
        
               <Link>Shared\FrameworkLocationHelper.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\IConstrainedEqualityComparer.cs"> 
        
               <Link>Shared\IConstrainedEqualityComparer.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\IKeyed.cs"> 
        
               <Link>Shared\IKeyed.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\MSBuildNameIgnoreCaseComparer.cs"> 
        
               <Link>Shared\MSBuildNameIgnoreCaseComparer.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\Modifiers.cs"> 
        
               <Link>Shared\Modifiers.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\InprocTrackingNativeMethods.cs"> 
        
               <Link>Shared\InprocTrackingNativeMethods.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\ProcessExtensions.cs"> 
        
               <Link>Shared\ProcessExtensions.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\ReadOnlyEmptyCollection.cs"> 
        
               <Link>Shared\ReadOnlyEmptyCollection.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\ReadOnlyEmptyDictionary.cs"> 
        
               <Link>Shared\Collections\ReadOnlyEmptyDictionary.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\RegistryDelegates.cs"> 
        
               <Link>Shared\RegistryDelegates.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\RegistryHelper.cs"> 
        
               <Link>Shared\RegistryHelper.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\ResourceUtilities.cs"> 
        
               <Link>Shared\ResourceUtilities.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\TaskLoggingHelper.cs"> 
        
               <Link>Shared\TaskLoggingHelper.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\TempFileUtilities.cs"> 
        
               <Link>Shared\TempFileUtilities.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\Tracing.cs"> 
        
               <Link>Shared\Tracing.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\VersionUtilities.cs"> 
        
               <Link>Shared\VersionUtilities.cs</Link> 
        
             </Compile> 
        
             <Compile Include="..\Shared\ToolsetElement.cs"> 
        
               <Link>Shared\ToolsetElement.cs</Link> 
        
             </Compile> 
        
           </ItemGroup>

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

msbuild/src/Shared/TaskLoggingHelper.cs

Lines 21 to 25 in cc3db35

    
           #if BUILD_ENGINE 
        
           namespace Microsoft.Build.BackEnd 
        
           #else 
        
           namespace Microsoft.Build.Utilities 
        
           #endif

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 · 2022-09-22T17:35:09Z

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.

Log each inner exception of an aggregate exception in TaskLoggingHelp…

23b5c09

…er.LogErrorFromException()

jeffkl self-assigned this Sep 21, 2022

rainersigwald approved these changes Sep 21, 2022

View reviewed changes

Forgind approved these changes Sep 21, 2022

View reviewed changes

Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Sep 26, 2022

Forgind merged commit 99f9c4d into dotnet:main Oct 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Log each inner exception of an aggregate exception in TaskLoggingHelper.LogErrorFromException()#7998
Forgind merged 1 commit intodotnet:mainfrom
jeffkl:logerrorfromexception-aggregateexception

jeffkl commented Sep 21, 2022

Uh oh!

Forgind left a comment

Uh oh!

jeffkl commented Sep 21, 2022

Uh oh!

Forgind commented Sep 21, 2022

Uh oh!

jeffkl commented Sep 21, 2022 •

edited

Loading

Uh oh!

Forgind commented Sep 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jeffkl commented Sep 21, 2022

Context

Changes Made

Testing

Notes

Uh oh!

Forgind left a comment

Choose a reason for hiding this comment

Uh oh!

jeffkl commented Sep 21, 2022

Uh oh!

Forgind commented Sep 21, 2022

Uh oh!

jeffkl commented Sep 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Forgind commented Sep 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeffkl commented Sep 21, 2022 •

edited

Loading