Random cleanup

src/Build.OM.UnitTests/Definition/Project_Tests.cs

@@ -2107,10 +2107,6 @@ public void BuildEvaluationUsesCustomLoggers()
             {
                 result = project.Build(new ILogger[] { mockLogger });
             }
-            catch
-            {
-                throw;
-            }
             finally
             {
                 project.ProjectCollection.UnregisterAllLoggers();

src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs

@@ -1599,19 +1599,10 @@ private ProjectInstance CreateTestProject(string projectBodyContents, string ini
                     File.Create("testProject.proj").Dispose();
                     break;
                 }
-                catch (Exception ex)
+                // If all the retries failed, fail with the actual problem instead of some difficult-to-understand issue later.
+                catch (Exception ex) when (retries < 4)
                 {
-                    if (retries < 4)
-                    {
-                        Console.WriteLine(ex.ToString());
-                    }
-                    else
-                    {
-                        // All the retries have failed. We will now fail with the
-                        // actual problem now instead of with some more difficult-to-understand
-                        // issue later.
-                        throw;
-                    }
+                    Console.WriteLine(ex.ToString());
                 }
             }
 

src/Build/BackEnd/BuildManager/BuildManager.cs

@@ -1719,10 +1719,9 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM
                         HandleNewRequest(Scheduler.VirtualNode, blocker);
                     }
                 }
-                catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex))
+                catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex) && !ExceptionHandling.NotExpectedException(ex) && ex is not BuildAbortedException)
                 {
-                    var projectException = ex as InvalidProjectFileException;
-                    if (projectException != null)
+                    if (ex is InvalidProjectFileException projectException)
                     {
                         if (!projectException.HasBeenLogged)
                         {
@@ -1731,10 +1730,6 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM
                             projectException.HasBeenLogged = true;
                         }
                     }
-                    else if ((ex is BuildAbortedException) || ExceptionHandling.NotExpectedException(ex))
-                    {
-                        throw;
-                    }
 
                     lock (_syncLock)
                     {
@@ -1744,7 +1739,7 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM
                             _legacyThreadingData.MainThreadSubmissionId = -1;
                         }
 
-                        if (projectException == null)
+                        if (ex is not InvalidProjectFileException)
                         {
                             var buildEventContext = new BuildEventContext(submission.SubmissionId, 1, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId);
                             ((IBuildComponentHost)this).LoggingService.LogFatalBuildError(buildEventContext, ex, new BuildEventFileInfo(submission.BuildRequestData.ProjectFullPath));
@@ -1847,15 +1842,15 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission)
                         submission.SubmissionId,
                         new ReadOnlyDictionary<ProjectGraphNode, BuildResult>(resultsPerNode ?? new Dictionary<ProjectGraphNode, BuildResult>())));
             }
-            catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex))
+            catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex) && !ExceptionHandling.NotExpectedException(ex) && ex is not BuildAbortedException)
             {
                 GraphBuildResult result = null;
 
                 // ProjectGraph throws an aggregate exception with InvalidProjectFileException inside when evaluation fails
                 if (ex is AggregateException aggregateException && aggregateException.InnerExceptions.All(innerException => innerException is InvalidProjectFileException))
                 {
                     // Log each InvalidProjectFileException encountered during ProjectGraph creation
-                    foreach (var innerException in aggregateException.InnerExceptions)
+                    foreach (Exception innerException in aggregateException.InnerExceptions)
                     {
                         var projectException = (InvalidProjectFileException) innerException;
                         if (!projectException.HasBeenLogged)
@@ -1873,23 +1868,16 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission)
                     BuildEventContext projectBuildEventContext = new BuildEventContext(submission.SubmissionId, 1, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId);
                     ((IBuildComponentHost)this).LoggingService.LogInvalidProjectFileError(projectBuildEventContext, new InvalidProjectFileException(ex.Message, ex));
                 }
-                else if (ex is BuildAbortedException || ExceptionHandling.NotExpectedException(ex))
-                {
-                    throw;
-                }
                 else
                 {
                     // Arbitrarily just choose the first entry point project's path
-                    var projectFile = submission.BuildRequestData.ProjectGraph?.EntryPointNodes.First().ProjectInstance.FullPath
+                    string projectFile = submission.BuildRequestData.ProjectGraph?.EntryPointNodes.First().ProjectInstance.FullPath
                         ?? submission.BuildRequestData.ProjectGraphEntryPoints?.First().ProjectFile;
                     BuildEventContext buildEventContext = new BuildEventContext(submission.SubmissionId, 1, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId);
                     ((IBuildComponentHost)this).LoggingService.LogFatalBuildError(buildEventContext, ex, new BuildEventFileInfo(projectFile));
                 }
 
-                if (result == null)
-                {
-                    result = new GraphBuildResult(submission.SubmissionId, ex);
-                }
+                result ??= new GraphBuildResult(submission.SubmissionId, ex);
 
                 ReportResultsToSubmission(result);
 

src/Build/BackEnd/Components/Logging/LoggingService.cs

@@ -1315,17 +1315,8 @@ private void ShutdownLogger(ILogger logger)
             {
                 logger?.Shutdown();
             }
-            catch (LoggerException)
+            catch (Exception e) when (!ExceptionHandling.IsCriticalException(e) && e is not LoggerException)
             {
-                throw;
-            }
-            catch (Exception e)
-            {
-                if (ExceptionHandling.IsCriticalException(e))
-                {
-                    throw;
-                }
-
                 InternalLoggerException.Throw(e, null, "FatalErrorDuringLoggerShutdown", false, logger.GetType().Name);
             }
         }
@@ -1586,17 +1577,8 @@ private void InitializeLogger(ILogger logger, IEventSource sourceForLogger)
                     logger.Initialize(sourceForLogger);
                 }
             }
-            catch (LoggerException)
+            catch (Exception e) when (!ExceptionHandling.IsCriticalException(e) && e is not LoggerException)
             {
-                throw;
-            }
-            catch (Exception e)
-            {
-                if (ExceptionHandling.IsCriticalException(e))
-                {
-                    throw;
-                }
-
                 InternalLoggerException.Throw(e, null, "FatalErrorWhileInitializingLogger", true, logger.GetType().Name);
             }
 

src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs

@@ -288,19 +288,10 @@ public void WaitForCancelCompletion()
                 {
                     taskCleanedUp = _requestTask.Wait(BuildParameters.RequestBuilderShutdownTimeout);
                 }
-                catch (AggregateException e)
+                catch (AggregateException e) when (e.Flatten().InnerExceptions.All(ex => ex is TaskCanceledException || ex is OperationCanceledException))
                 {
-                    AggregateException flattenedException = e.Flatten();
-
-                    if (flattenedException.InnerExceptions.All(ex => (ex is TaskCanceledException || ex is OperationCanceledException)))
-                    {
-                        // ignore -- just indicates that the task finished cancelling before we got a chance to wait on it.  
-                        taskCleanedUp = true;
-                    }
-                    else
-                    {
-                        throw;
-                    }
+                    // ignore -- just indicates that the task finished cancelling before we got a chance to wait on it.  
+                    taskCleanedUp = true;
                 }
 
                 if (!taskCleanedUp)
@@ -824,17 +815,7 @@ private async Task BuildAndReport()
 
                 thrownException = ex;
             }
-            catch (LoggerException ex)
-            {
-                // Polite logger failure
-                thrownException = ex;
-            }
-            catch (InternalLoggerException ex)
-            {
-                // Logger threw arbitrary exception
-                thrownException = ex;
-            }
-            catch (Exception ex)
+            catch (Exception ex) // LoggerException is a polite logger failure. InternalLoggerException is an arbitrary exception. Handle them the same.
             {
                 thrownException = ex;
 
@@ -874,13 +855,8 @@ private void ReportResultAndCleanUp(BuildResult result)
                 {
                     _projectLoggingContext.LogProjectFinished(result.OverallResult == BuildResultCode.Success);
                 }
-                catch (Exception ex)
+                catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex))
                 {
-                    if (ExceptionHandling.IsCriticalException(ex))
-                    {
-                        throw;
-                    }
-
                     if (result.Exception == null)
                     {
                         result.Exception = ex;

src/Build/BackEnd/Node/InProcNode.cs

@@ -309,13 +309,8 @@ private NodeEngineShutdownReason HandleShutdown(out Exception exception)
                     _buildRequestEngine.CleanupForBuild();
                 }
             }
-            catch (Exception ex)
+            catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex))
             {
-                if (ExceptionHandling.IsCriticalException(ex))
-                {
-                    throw;
-                }
-
                 // If we had some issue shutting down, don't reuse the node because we may be in some weird state.
                 if (_shutdownReason == NodeEngineShutdownReason.BuildCompleteReuse)
                 {

src/Build/BackEnd/Node/OutOfProcNode.cs

@@ -758,13 +758,8 @@ private void HandleNodeConfiguration(NodeConfiguration configuration)
                     _loggingService.InitializeNodeLoggers(configuration.LoggerDescriptions, sink, configuration.NodeId);
                 }
             }
-            catch (Exception ex)
+            catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex))
             {
-                if (ExceptionHandling.IsCriticalException(ex))
-                {
-                    throw;
-                }
-
                 OnEngineException(ex);
             }
 

src/Build/BackEnd/Shared/BuildRequestConfiguration.cs

@@ -1005,14 +1005,14 @@ private ITranslator GetConfigurationTranslator(TranslationDirection direction)
                     return BinaryTranslator.GetReadTranslator(File.OpenRead(cacheFile), null);
                 }
             }
-            catch (Exception e)
+            catch (DirectoryNotFoundException e)
             {
-                if (e is DirectoryNotFoundException || e is UnauthorizedAccessException)
-                {
-                    ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e);
-                }
-
-                // UNREACHABLE
+                ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e);
+                throw;
+            }
+            catch (UnauthorizedAccessException e)
+            {
+                ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e);
                 throw;
             }
         }