diff --git a/src/Build.OM.UnitTests/Definition/Project_Tests.cs b/src/Build.OM.UnitTests/Definition/Project_Tests.cs index afaf6ecf8ab..5944151bd4d 100644 --- a/src/Build.OM.UnitTests/Definition/Project_Tests.cs +++ b/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(); diff --git a/src/Build.UnitTests/BackEnd/AssemblyTaskFactory_Tests.cs b/src/Build.UnitTests/BackEnd/AssemblyTaskFactory_Tests.cs index 106e03beaf3..a5302dc1753 100644 --- a/src/Build.UnitTests/BackEnd/AssemblyTaskFactory_Tests.cs +++ b/src/Build.UnitTests/BackEnd/AssemblyTaskFactory_Tests.cs @@ -11,6 +11,7 @@ using Microsoft.Build.Construction; using InvalidProjectFileException = Microsoft.Build.Exceptions.InvalidProjectFileException; using Xunit; +using Shouldly; namespace Microsoft.Build.UnitTests.BackEnd { @@ -255,8 +256,8 @@ public void VerifyGoodTaskInstantiation() new AppDomainSetup(), #endif false); - Assert.NotNull(createdTask); - Assert.False(createdTask is TaskHostTask); + createdTask.ShouldNotBeNull(); + createdTask.ShouldNotBeOfType(); } finally { diff --git a/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs b/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs index 466bce2536a..69d73b6c770 100644 --- a/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs +++ b/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs @@ -1679,11 +1679,11 @@ public void CancelledBuildWithDelay40() [Fact] public void CancelledBuildInTaskHostWithDelay40() { - string contents = CleanupFileContents(@" + string contents = CleanupFileContents(@$" - + diff --git a/src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs b/src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs index 3b4d5164eb9..43dd70124a9 100644 --- a/src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs +++ b/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()); } } diff --git a/src/Build.UnitTests/BackEnd/TaskHostFactory_Tests.cs b/src/Build.UnitTests/BackEnd/TaskHostFactory_Tests.cs index fa4146484e9..08b3114b352 100644 --- a/src/Build.UnitTests/BackEnd/TaskHostFactory_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TaskHostFactory_Tests.cs @@ -27,7 +27,7 @@ public void TaskNodesDieAfterBuild() "; TransientTestFile project = env.CreateFile("testProject.csproj", pidTaskProject); - ProjectInstance projectInstance = new ProjectInstance(project.Path); + ProjectInstance projectInstance = new(project.Path); projectInstance.Build().ShouldBeTrue(); string processId = projectInstance.GetPropertyValue("PID"); string.IsNullOrEmpty(processId).ShouldBeFalse(); diff --git a/src/Build.UnitTests/EscapingInProjects_Tests.cs b/src/Build.UnitTests/EscapingInProjects_Tests.cs index edd37ea0144..29d7227bdc2 100644 --- a/src/Build.UnitTests/EscapingInProjects_Tests.cs +++ b/src/Build.UnitTests/EscapingInProjects_Tests.cs @@ -22,6 +22,7 @@ using Xunit; using Xunit.Abstractions; using Microsoft.Build.Shared; +using Shouldly; namespace Microsoft.Build.UnitTests.EscapingInProjects_Tests { @@ -126,11 +127,11 @@ public void SemicolonInPropertyPassedIntoStringParam_UsingTaskHost() [Fact] public void SemicolonInPropertyPassedIntoITaskItemParam() { - MockLogger logger = Helpers.BuildProjectWithNewOMExpectSuccess(String.Format(@" + MockLogger logger = Helpers.BuildProjectWithNewOMExpectSuccess(@$" - + abc %3b def %3b ghi @@ -142,7 +143,7 @@ public void SemicolonInPropertyPassedIntoITaskItemParam() - ", new Uri(Assembly.GetExecutingAssembly().EscapedCodeBase).LocalPath), + ", logger: new MockLogger(_output)); logger.AssertLogContains("Received TaskItemParam: 123 abc ; def ; ghi 789"); @@ -715,14 +716,14 @@ public void EscapedWildcardsShouldNotBeExpanded() [Trait("Category", "mono-osx-failing")] public void EscapedWildcardsShouldNotBeExpanded_InTaskHost() { - MockLogger logger = new MockLogger(); + MockLogger logger = new(); try { // Populate the project directory with three physical files on disk -- a.weirdo, b.weirdo, c.weirdo. EscapingInProjectsHelper.CreateThreeWeirdoFiles(); Project project = ObjectModelHelpers.CreateInMemoryProject(@" - + @@ -734,8 +735,7 @@ public void EscapedWildcardsShouldNotBeExpanded_InTaskHost() "); - bool success = project.Build(logger); - Assert.True(success); // "Build failed. See test output (Attachments in Azure Pipelines) for details" + project.Build(logger).ShouldBeTrue("Build failed. See test output (Attachments in Azure Pipelines) for details"); logger.AssertLogContains("[*]"); } finally diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index 98fc6c1a7fb..1eb0fdbf8fa 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/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 (IsInvalidProjectOrIORelatedException(ex)) { - 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)); @@ -1764,6 +1759,11 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM } } + private bool IsInvalidProjectOrIORelatedException(Exception e) + { + return !ExceptionHandling.IsCriticalException(e) && !ExceptionHandling.NotExpectedException(e) && e is not BuildAbortedException; + } + private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission) { try @@ -1847,7 +1847,7 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission) submission.SubmissionId, new ReadOnlyDictionary(resultsPerNode ?? new Dictionary()))); } - catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex)) + catch (Exception ex) when (IsInvalidProjectOrIORelatedException(ex)) { GraphBuildResult result = null; @@ -1855,7 +1855,7 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission) 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 +1873,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); diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs index fec15006b64..242b3194607 100644 --- a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs +++ b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs @@ -230,8 +230,7 @@ public bool ResolveConfigurationRequest(int unresolvedConfigId, int configId) { lock (GlobalLock) { - List requests = null; - if (_unresolvedConfigurations?.TryGetValue(unresolvedConfigId, out requests) != true) + if (_unresolvedConfigurations?.TryGetValue(unresolvedConfigId, out List requests) != true) { return false; } diff --git a/src/Build/BackEnd/Components/Caching/ConfigCache.cs b/src/Build/BackEnd/Components/Caching/ConfigCache.cs index 2e695a60aab..b1cb88b6b37 100644 --- a/src/Build/BackEnd/Components/Caching/ConfigCache.cs +++ b/src/Build/BackEnd/Components/Caching/ConfigCache.cs @@ -118,8 +118,7 @@ public BuildRequestConfiguration GetMatchingConfiguration(ConfigurationMetadata ErrorUtilities.VerifyThrowArgumentNull(configMetadata, nameof(configMetadata)); lock (_lockObject) { - int configId; - if (!_configurationIdsByMetadata.TryGetValue(configMetadata, out configId)) + if (!_configurationIdsByMetadata.TryGetValue(configMetadata, out int configId)) { return null; } @@ -214,10 +213,9 @@ public List ClearNonExplicitlyLoadedConfigurations() { foreach (KeyValuePair metadata in _configurationIdsByMetadata) { - BuildRequestConfiguration configuration; int configId = metadata.Value; - if (_configurations.TryGetValue(configId, out configuration)) + if (_configurations.TryGetValue(configId, out BuildRequestConfiguration configuration)) { // We do not want to retain this configuration if (!configuration.ExplicitlyLoaded) diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs index ee236fac998..90866282fb1 100644 --- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs +++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs @@ -222,8 +222,7 @@ public void ClearResultsForConfiguration(int configurationId) { lock (_resultsByConfiguration) { - BuildResult removedResult; - _resultsByConfiguration.TryRemove(configurationId, out removedResult); + _resultsByConfiguration.TryRemove(configurationId, out BuildResult removedResult); removedResult?.ClearCachedFiles(); } diff --git a/src/Build/BackEnd/Components/Communications/NodeManager.cs b/src/Build/BackEnd/Components/Communications/NodeManager.cs index 5a533e05fbb..d7719991372 100644 --- a/src/Build/BackEnd/Components/Communications/NodeManager.cs +++ b/src/Build/BackEnd/Components/Communications/NodeManager.cs @@ -126,8 +126,7 @@ public NodeInfo CreateNode(NodeConfiguration configuration, NodeAffinity nodeAff public void SendData(int node, INodePacket packet) { // Look up the node provider for this node in the mapping. - INodeProvider provider; - if (!_nodeIdToProvider.TryGetValue(node, out provider)) + if (!_nodeIdToProvider.TryGetValue(node, out INodeProvider provider)) { ErrorUtilities.ThrowInternalError("Node {0} does not have a provider.", node); } diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs index 5d6c5f8f2cc..d2f7f0da1f1 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs @@ -455,9 +455,8 @@ internal static string GetMSBuildLocationFromHostContext(HandshakeOptions hostCo /// internal bool AcquireAndSetUpHost(HandshakeOptions hostContext, INodePacketFactory factory, INodePacketHandler handler, TaskHostConfiguration configuration) { - NodeContext context; bool nodeCreationSucceeded; - if (!_nodeContexts.TryGetValue(hostContext, out context)) + if (!_nodeContexts.ContainsKey(hostContext)) { nodeCreationSucceeded = CreateNode(hostContext, factory, handler, configuration); } @@ -469,7 +468,7 @@ internal bool AcquireAndSetUpHost(HandshakeOptions hostContext, INodePacketFacto if (nodeCreationSucceeded) { - context = _nodeContexts[hostContext]; + NodeContext context = _nodeContexts[hostContext]; _nodeIdToPacketFactory[(int)hostContext] = factory; _nodeIdToPacketHandler[(int)hostContext] = handler; diff --git a/src/Build/BackEnd/Components/Logging/LoggingService.cs b/src/Build/BackEnd/Components/Logging/LoggingService.cs index 117f5195448..ff913439026 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingService.cs @@ -284,8 +284,7 @@ protected LoggingService(LoggerMode loggerMode, int nodeId) string queueCapacityEnvironment = Environment.GetEnvironmentVariable("MSBUILDLOGGINGQUEUECAPACITY"); if (!String.IsNullOrEmpty(queueCapacityEnvironment)) { - uint localQueueCapacity; - if (UInt32.TryParse(queueCapacityEnvironment, out localQueueCapacity)) + if (UInt32.TryParse(queueCapacityEnvironment, out uint localQueueCapacity)) { _queueCapacity = localQueueCapacity; } @@ -1315,17 +1314,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); } } @@ -1493,8 +1483,7 @@ private void RouteBuildEvent(KeyValuePair nodeEvent) TryRaiseProjectStartedEvent(nodeEvent.Value); // Get the sink which will handle the build event, then send the event to that sink - IBuildEventSink sink; - bool gotSink = _eventSinkDictionary.TryGetValue(nodeEvent.Key, out sink); + bool gotSink = _eventSinkDictionary.TryGetValue(nodeEvent.Key, out IBuildEventSink sink); if (gotSink && sink != null) { // Sinks in the eventSinkDictionary are expected to not be null. @@ -1586,17 +1575,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); } diff --git a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs index c5c3555f201..2241cfc1a88 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs @@ -133,9 +133,7 @@ public void LogError(BuildEventContext buildEventContext, string subcategoryReso { ErrorUtilities.VerifyThrow(!string.IsNullOrEmpty(messageResourceName), "Need resource string for error message."); - string errorCode; - string helpKeyword; - string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword(out errorCode, out helpKeyword, messageResourceName, messageArgs); + string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword(out string errorCode, out string helpKeyword, messageResourceName, messageArgs); LogErrorFromText(buildEventContext, subcategoryResourceName, errorCode, helpKeyword, file, message); } @@ -185,8 +183,7 @@ public void LogErrorFromText(BuildEventContext buildEventContext, string subcate buildEvent.BuildEventContext = buildEventContext; if (buildEvent.ProjectFile == null && buildEventContext.ProjectContextId != BuildEventContext.InvalidProjectContextId) { - string projectFile; - _projectFileMap.TryGetValue(buildEventContext.ProjectContextId, out projectFile); + _projectFileMap.TryGetValue(buildEventContext.ProjectContextId, out string projectFile); ErrorUtilities.VerifyThrow(projectFile != null, "ContextID {0} should have been in the ID-to-project file mapping but wasn't!", buildEventContext.ProjectContextId); buildEvent.ProjectFile = projectFile; } @@ -231,8 +228,7 @@ public void LogInvalidProjectFileError(BuildEventContext buildEventContext, Inva buildEvent.BuildEventContext = buildEventContext; if (buildEvent.ProjectFile == null && buildEventContext.ProjectContextId != BuildEventContext.InvalidProjectContextId) { - string projectFile; - _projectFileMap.TryGetValue(buildEventContext.ProjectContextId, out projectFile); + _projectFileMap.TryGetValue(buildEventContext.ProjectContextId, out string projectFile); ErrorUtilities.VerifyThrow(projectFile != null, "ContextID {0} should have been in the ID-to-project file mapping but wasn't!", buildEventContext.ProjectContextId); buildEvent.ProjectFile = projectFile; } @@ -293,9 +289,7 @@ public void LogFatalError(BuildEventContext buildEventContext, Exception excepti { ErrorUtilities.VerifyThrow(!string.IsNullOrEmpty(messageResourceName), "Need resource string for error message."); - string errorCode; - string helpKeyword; - string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword(out errorCode, out helpKeyword, messageResourceName, messageArgs); + string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword(out string errorCode, out string helpKeyword, messageResourceName, messageArgs); #if DEBUG message += Environment.NewLine + "This is an unhandled exception from a task -- PLEASE OPEN A BUG AGAINST THE TASK OWNER."; #endif @@ -332,9 +326,7 @@ public void LogTaskWarningFromException(BuildEventContext buildEventContext, Exc { ErrorUtilities.VerifyThrow(!String.IsNullOrEmpty(taskName), "Must specify the name of the task that failed."); - string warningCode; - string helpKeyword; - string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword(out warningCode, out helpKeyword, "FatalTaskError", taskName); + string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword(out string warningCode, out string helpKeyword, "FatalTaskError", taskName); #if DEBUG message += Environment.NewLine + "This is an unhandled exception from a task -- PLEASE OPEN A BUG AGAINST THE TASK OWNER."; #endif @@ -362,9 +354,7 @@ public void LogWarning(BuildEventContext buildEventContext, string subcategoryRe { ErrorUtilities.VerifyThrow(!string.IsNullOrEmpty(messageResourceName), "Need resource string for warning message."); - string warningCode; - string helpKeyword; - string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword(out warningCode, out helpKeyword, messageResourceName, messageArgs); + string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword(out string warningCode, out string helpKeyword, messageResourceName, messageArgs); LogWarningFromText(buildEventContext, subcategoryResourceName, warningCode, helpKeyword, file, message); } } @@ -410,8 +400,7 @@ public void LogWarningFromText(BuildEventContext buildEventContext, string subca buildEvent.BuildEventContext = buildEventContext; if (buildEvent.ProjectFile == null && buildEventContext.ProjectContextId != BuildEventContext.InvalidProjectContextId) { - string projectFile; - _projectFileMap.TryGetValue(buildEventContext.ProjectContextId, out projectFile); + _projectFileMap.TryGetValue(buildEventContext.ProjectContextId, out string projectFile); ErrorUtilities.VerifyThrow(projectFile != null, "ContextID {0} should have been in the ID-to-project file mapping but wasn't!", buildEventContext.ProjectContextId); buildEvent.ProjectFile = projectFile; } diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 9da66ddcc95..f76ca291bc3 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/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 (InnerExceptionsAreAllCancelledExceptions(e)) { - 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) @@ -314,6 +305,11 @@ public void WaitForCancelCompletion() _isZombie = true; } + private bool InnerExceptionsAreAllCancelledExceptions(AggregateException e) + { + return e.Flatten().InnerExceptions.All(ex => ex is TaskCanceledException || ex is OperationCanceledException); + } + #region IRequestBuilderCallback Members /// @@ -824,16 +820,6 @@ 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) { thrownException = ex; @@ -874,13 +860,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; diff --git a/src/Build/BackEnd/Node/InProcNode.cs b/src/Build/BackEnd/Node/InProcNode.cs index 81b4ab63279..a5b1655469f 100644 --- a/src/Build/BackEnd/Node/InProcNode.cs +++ b/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) { diff --git a/src/Build/BackEnd/Node/OutOfProcNode.cs b/src/Build/BackEnd/Node/OutOfProcNode.cs index ae59a7b50af..9249b93acb8 100644 --- a/src/Build/BackEnd/Node/OutOfProcNode.cs +++ b/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); } diff --git a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs index 6d55273c55c..9eb3c93194b 100644 --- a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs +++ b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs @@ -1005,14 +1005,9 @@ private ITranslator GetConfigurationTranslator(TranslationDirection direction) return BinaryTranslator.GetReadTranslator(File.OpenRead(cacheFile), null); } } - catch (Exception e) + catch (Exception e) when (e is DirectoryNotFoundException || e is UnauthorizedAccessException) { - if (e is DirectoryNotFoundException || e is UnauthorizedAccessException) - { - ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e); - } - - // UNREACHABLE + ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e); throw; } } diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 77497354b7b..0dfd676537b 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -342,13 +342,8 @@ bool ITaskExecutionHost.SetTaskParameters(IDictionary taskIdentityParameters Environment.NewLine + e.InnerException ); } - catch (Exception e) // Catching Exception, but rethrowing unless it's a well-known exception. + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - // Reflection related exception _taskLoggingContext.LogError ( @@ -1369,17 +1345,6 @@ object parameterValue _taskFactoryWrapper.SetPropertyValue(TaskInstance, parameter, parameterValue); success = true; } - catch (LoggerException) - { - // if a logger has failed, abort immediately - // Polite logger failure - throw; - } - catch (InternalLoggerException) - { - // Logger threw arbitrary exception - throw; - } catch (TargetInvocationException e) { // handle any exception thrown by the task's setter itself @@ -1395,14 +1360,10 @@ object parameterValue new BuildEventFileInfo(_taskLocation), _taskName); } - catch (Exception e) + // If a logger has failed, abort immediately. This is the polite LoggerException. + // InternalLoggerException is an arbitrary logger exception. + catch (Exception e) when (e is not LoggerException && e is not InternalLoggerException && !ExceptionHandling.NotExpectedReflectionException(e)) { - // Catching Exception, but rethrowing unless it's a well-known exception. - if (ExceptionHandling.NotExpectedReflectionException(e)) - { - throw; - } - _taskLoggingContext.LogFatalTaskError ( e, @@ -1614,13 +1575,8 @@ private IDictionary GetNamesOfPropertiesWithRequiredAttribute() { requiredParameters = _taskFactoryWrapper.GetNamesOfPropertiesWithRequiredAttribute; } - catch (Exception e) // Catching Exception, but rethrowing unless it's a well-known exception. + catch (Exception e) when (!ExceptionHandling.NotExpectedReflectionException(e)) { - if (ExceptionHandling.NotExpectedReflectionException(e)) - { - throw; - } - // Reflection related exception _targetLoggingContext.LogError(new BuildEventFileInfo(_taskLocation), "AttributeTypeLoadError", _taskName, e.Message); @@ -1640,14 +1596,10 @@ private void DisplayCancelWaitMessage() { _taskLoggingContext.LogWarningFromText(null, warningCode, helpKeyword, new BuildEventFileInfo(_taskLocation), message); } - catch (InternalErrorException) // BUGBUG, should never catch this + catch (InternalErrorException) when (!_taskLoggingContext.IsValid) { // We can get an exception from this when we encounter a race between a task finishing and a cancel occurring. In this situation // if the task logging context is no longer valid, we choose to eat the exception because we can't log the message anyway. - if (_taskLoggingContext.IsValid) - { - throw; - } } } } diff --git a/src/Build/Construction/ProjectRootElement.cs b/src/Build/Construction/ProjectRootElement.cs index cb08aee7b3e..391e9650fd8 100644 --- a/src/Build/Construction/ProjectRootElement.cs +++ b/src/Build/Construction/ProjectRootElement.cs @@ -2082,13 +2082,8 @@ private XmlDocumentWithLocation LoadDocument(string fullPath, bool preserveForma StreamTimeUtc = null; } } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.NotExpectedIoOrXmlException(ex)) { - if (ExceptionHandling.NotExpectedIoOrXmlException(ex)) - { - throw; - } - BuildEventFileInfo fileInfo = ex is XmlException xmlException ? new BuildEventFileInfo(fullPath, xmlException) : new BuildEventFileInfo(fullPath); diff --git a/src/Build/Construction/Solution/SolutionFile.cs b/src/Build/Construction/Solution/SolutionFile.cs index e0e9059c984..3c81e27896c 100644 --- a/src/Build/Construction/Solution/SolutionFile.cs +++ b/src/Build/Construction/Solution/SolutionFile.cs @@ -497,10 +497,9 @@ internal void ParseSolutionFile() SolutionReader = new StreamReader(fileStream, Encoding.GetEncoding(0)); // HIGHCHAR: If solution files have no byte-order marks, then assume ANSI rather than ASCII. ParseSolution(); } - catch (Exception e) + catch (Exception e) when (ExceptionUtilities.IsIoRelatedException(e)) { - ProjectFileErrorUtilities.VerifyThrowInvalidProjectFile(!ExceptionUtilities.IsIoRelatedException(e), new BuildEventFileInfo(_solutionFile), "InvalidProjectFile", e.Message); - throw; + ProjectFileErrorUtilities.ThrowInvalidProjectFile(new BuildEventFileInfo(_solutionFile), "InvalidProjectFile", e.Message); } finally { diff --git a/src/Build/Construction/Solution/SolutionProjectGenerator.cs b/src/Build/Construction/Solution/SolutionProjectGenerator.cs index 43318152844..47a037ca3ae 100644 --- a/src/Build/Construction/Solution/SolutionProjectGenerator.cs +++ b/src/Build/Construction/Solution/SolutionProjectGenerator.cs @@ -2191,14 +2191,8 @@ private void ScanProjectDependencies(string childProjectToolsVersion, string ful AddDependencyByGuid(project, referencedWebProjectGuid); } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) // We don't want any problems scanning the project file to result in aborting the build. { - // We don't want any problems scanning the project file to result in aborting the build. - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - _loggingService.LogWarning ( _projectBuildEventContext, diff --git a/src/Build/Definition/Project.cs b/src/Build/Definition/Project.cs index df1d8ed8eb4..8a909e72f63 100644 --- a/src/Build/Definition/Project.cs +++ b/src/Build/Definition/Project.cs @@ -470,17 +470,14 @@ private Project(string projectFile, IDictionary globalProperties { defaultImplementation.Initialize(globalProperties, toolsVersion, subToolsetVersion, loadSettings, evaluationContext); } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex)) { // If possible, clear out the XML we just loaded into the XML cache: // if we had loaded the XML from disk into the cache within this constructor, // and then are are bailing out because there is a typo in the XML such that // evaluation failed, we don't want to leave the bad XML in the cache; // the user wouldn't be able to fix the XML file and try again. - if (!ExceptionHandling.IsCriticalException(ex)) - { - projectCollection.TryUnloadProject(Xml); - } + projectCollection.TryUnloadProject(Xml); throw; } diff --git a/src/Build/Definition/Toolset.cs b/src/Build/Definition/Toolset.cs index b623fed7040..a973b6827ac 100644 --- a/src/Build/Definition/Toolset.cs +++ b/src/Build/Definition/Toolset.cs @@ -568,13 +568,8 @@ internal static bool Dev10IsInstalled s_dev10IsInstalled = false; } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.NotExpectedRegistryException(e)) { - if (ExceptionHandling.NotExpectedRegistryException(e)) - { - throw; - } - // if it's a registry exception, just shrug, eat it, and move on with life on the assumption that whatever // went wrong, it's pretty clear that Dev10 probably isn't installed. s_dev10IsInstalled = false; diff --git a/src/Build/Evaluation/Expander.cs b/src/Build/Evaluation/Expander.cs index 6e7151fcc60..cee1b3735c5 100644 --- a/src/Build/Evaluation/Expander.cs +++ b/src/Build/Evaluation/Expander.cs @@ -1644,13 +1644,8 @@ private static string ExpandRegistryValue(string registryExpression, IElementLoc result = String.Empty; } } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.NotExpectedRegistryException(ex)) { - if (ExceptionHandling.NotExpectedRegistryException(ex)) - { - throw; - } - ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "InvalidRegistryPropertyExpression", "$(" + registryExpression + ")", ex.Message); } } @@ -2297,15 +2292,10 @@ internal static IEnumerable> ItemSpecModifierFunction(Expander

properties, // First use InvokeMember using the standard binder - this will match and coerce as needed functionResult = _receiverType.InvokeMember(_methodMethodName, _bindingFlags, Type.DefaultBinder, objectInstance, args, CultureInfo.InvariantCulture); } - catch (MissingMethodException ex) // Don't catch and retry on any other exception + // If we're invoking a method, then there are deeper attempts that can be made to invoke the method. + // If not, we were asked to get a property or field but found that we cannot locate it. No further argument coersion is possible, so throw. + catch (MissingMethodException ex) when ((_bindingFlags & BindingFlags.InvokeMethod) == BindingFlags.InvokeMethod) { - // If we're invoking a method, then there are deeper attempts that - // can be made to invoke the method - if ((_bindingFlags & BindingFlags.InvokeMethod) == BindingFlags.InvokeMethod) - { - // The standard binder failed, so do our best to coerce types into the arguments for the function - // This may happen if the types need coercion, but it may also happen if the object represents a type that contains open type parameters, that is, ContainsGenericParameters returns true. - functionResult = LateBindExecute(ex, _bindingFlags, objectInstance, args, false /* is not constructor */); - } - else - { - // We were asked to get a property or field, and we found that we cannot - // locate it. Since there is no further argument coersion possible - // we'll throw right now. - throw; - } + // The standard binder failed, so do our best to coerce types into the arguments for the function + // This may happen if the types need coercion, but it may also happen if the object represents a type that contains open type parameters, that is, ContainsGenericParameters returns true. + functionResult = LateBindExecute(ex, _bindingFlags, objectInstance, args, false /* is not constructor */); } } } @@ -3565,13 +3545,8 @@ internal object Execute(object objectInstance, IPropertyProvider properties, } // Any other exception was thrown by trying to call it - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.NotExpectedFunctionException(ex)) { - if (ExceptionHandling.NotExpectedFunctionException(ex)) - { - throw; - } - // If there's a :: in the expression, they were probably trying for a static function // invocation. Give them some more relevant info in that case if (s_invariantCompareInfo.IndexOf(_expression, "::", CompareOptions.OrdinalIgnoreCase) > -1) diff --git a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs index 4596be57f52..fa39f2023c3 100644 --- a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs +++ b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs @@ -302,13 +302,8 @@ string taskProjectFile // taskName may be null ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "TaskLoadFailure", taskName, loadInfo.AssemblyLocation, e.Message); } - catch (Exception e) // Catching Exception, but rethrowing unless it's a well-known exception. + catch (Exception e) when (!ExceptionHandling.NotExpectedReflectionException(e)) { - if (ExceptionHandling.NotExpectedReflectionException(e)) - { - throw; - } - ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "TaskLoadFailure", taskName, loadInfo.AssemblyLocation, e.Message); } @@ -440,13 +435,8 @@ internal bool TaskNameCreatableByFactory(string taskName, IDictionary MergeTaskFactoryParameterSets(IDictio { mergedParameters = new Dictionary(StringComparer.OrdinalIgnoreCase); - string taskRuntime; - taskIdentityParameters.TryGetValue(XMakeAttributes.runtime, out taskRuntime); - string usingTaskRuntime; - factoryIdentityParameters.TryGetValue(XMakeAttributes.runtime, out usingTaskRuntime); + taskIdentityParameters.TryGetValue(XMakeAttributes.runtime, out string taskRuntime); + factoryIdentityParameters.TryGetValue(XMakeAttributes.runtime, out string usingTaskRuntime); if (!XMakeAttributes.TryMergeRuntimeValues(taskRuntime, usingTaskRuntime, out mergedRuntime)) { @@ -586,10 +574,8 @@ private static IDictionary MergeTaskFactoryParameterSets(IDictio mergedParameters.Add(XMakeAttributes.runtime, mergedRuntime); } - string taskArchitecture; - taskIdentityParameters.TryGetValue(XMakeAttributes.architecture, out taskArchitecture); - string usingTaskArchitecture; - factoryIdentityParameters.TryGetValue(XMakeAttributes.architecture, out usingTaskArchitecture); + taskIdentityParameters.TryGetValue(XMakeAttributes.architecture, out string taskArchitecture); + factoryIdentityParameters.TryGetValue(XMakeAttributes.architecture, out string usingTaskArchitecture); if (!XMakeAttributes.TryMergeArchitectureValues(taskArchitecture, usingTaskArchitecture, out mergedArchitecture)) { diff --git a/src/Build/Instance/TaskRegistry.cs b/src/Build/Instance/TaskRegistry.cs index a44626cccae..e233f960207 100644 --- a/src/Build/Instance/TaskRegistry.cs +++ b/src/Build/Instance/TaskRegistry.cs @@ -1234,19 +1234,14 @@ internal bool CanTaskBeCreatedByFactory(string taskName, string taskProjectFile, creatableByFactory = null; } } - catch (Exception e) // Catching Exception, but rethrowing unless it's a well-known exception. + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - // Log e.ToString to give as much information about the failure of a "third party" call as possible. - string message = String.Empty; + string message = #if DEBUG - message += UnhandledFactoryError; + UnhandledFactoryError + #endif - message += e.ToString(); + e.ToString(); ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "TaskLoadFailure", taskName, _taskFactoryWrapperInstance.Name, message); } } @@ -1360,13 +1355,8 @@ private bool GetTaskFactory(TargetLoggingContext targetLoggingContext, ElementLo ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "TaskFactoryLoadFailure", TaskFactoryAttributeName, taskFactoryLoadInfo.AssemblyLocation, e.Message); } - catch (Exception e) // Catching Exception, but rethrowing unless it's a well-known exception. + catch (Exception e) when (!ExceptionHandling.NotExpectedReflectionException(e)) { - if (ExceptionHandling.NotExpectedReflectionException(e)) - { - throw; - } - ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "TaskFactoryLoadFailure", TaskFactoryAttributeName, taskFactoryLoadInfo.AssemblyLocation, e.Message); } @@ -1447,18 +1437,13 @@ private bool GetTaskFactory(TargetLoggingContext targetLoggingContext, ElementLo return false; } - catch (Exception e) // Catching Exception, but rethrowing unless it's a well-known exception. + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - - string message = String.Empty; + string message = #if DEBUG - message += UnhandledFactoryError; + UnhandledFactoryError + #endif - message += e.Message; + e.Message; ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "TaskFactoryLoadFailure", TaskFactoryAttributeName, taskFactoryLoadInfo.AssemblyLocation, message); } diff --git a/src/Build/Logging/LoggerDescription.cs b/src/Build/Logging/LoggerDescription.cs index f50a53d0e68..3e4978adf3c 100644 --- a/src/Build/Logging/LoggerDescription.cs +++ b/src/Build/Logging/LoggerDescription.cs @@ -167,22 +167,10 @@ internal IForwardingLogger CreateForwardingLogger() InternalLoggerException.Throw(null, null, "LoggerNotFoundError", true, this.Name); } } - catch (Exception e /* Wrap all other exceptions in a more meaningful exception*/) + catch (Exception e) // Wrap other exceptions in a more meaningful exception. LoggerException and InternalLoggerException are already meaningful. + when (!(e is LoggerException /* Polite logger Failure*/ || e is InternalLoggerException /* LoggerClass not found*/ || ExceptionHandling.IsCriticalException(e))) { - // Two of the possible exceptions are already in reasonable exception types - if (e is LoggerException /* Polite logger Failure*/ || e is InternalLoggerException /* LoggerClass not found*/) - { - throw; - } - else - { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - - InternalLoggerException.Throw(e, null, "LoggerCreationError", true, Name); - } + InternalLoggerException.Throw(e, null, "LoggerCreationError", true, Name); } return forwardingLogger; @@ -238,25 +226,15 @@ private ILogger CreateLogger(bool forwardingLogger) string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword("LoggerInstantiationFailureErrorInvalidCast", _loggerClassName, _loggerAssembly.AssemblyLocation, e.Message); throw new LoggerException(message, e.InnerException); } - catch (TargetInvocationException e) + catch (TargetInvocationException e) when (e.InnerException is LoggerException le) { // At this point, the interesting stack is the internal exception; // the outer exception is System.Reflection stuff that says nothing // about the nature of the logger failure. - Exception innerException = e.InnerException; - - if (innerException is LoggerException) - { - // Logger failed politely during construction. In order to preserve - // the stack trace at which the error occurred we wrap the original - // exception instead of throwing. - LoggerException l = ((LoggerException)innerException); - throw new LoggerException(l.Message, innerException, l.ErrorCode, l.HelpKeyword); - } - else - { - throw; - } + // Logger failed politely during construction. In order to preserve + // the stack trace at which the error occurred we wrap the original + // exception instead of throwing. + throw new LoggerException(le.Message, le, le.ErrorCode, le.HelpKeyword); } return logger; diff --git a/src/Build/Utilities/RegistryKeyWrapper.cs b/src/Build/Utilities/RegistryKeyWrapper.cs index c1503c698ef..5bb89264ffc 100644 --- a/src/Build/Utilities/RegistryKeyWrapper.cs +++ b/src/Build/Utilities/RegistryKeyWrapper.cs @@ -81,11 +81,8 @@ public virtual string Name { return Exists() ? WrappedKey.Name : string.Empty; } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.NotExpectedRegistryException(ex)) { - if (ExceptionHandling.NotExpectedRegistryException(ex)) - throw; - throw new RegistryException(ex.Message, ex); } } @@ -114,11 +111,8 @@ public virtual object GetValue(string name) { return Exists() ? WrappedKey.GetValue(name) : null; } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.NotExpectedRegistryException(ex)) { - if (ExceptionHandling.NotExpectedRegistryException(ex)) - throw; - throw new RegistryException(ex.Message, Name + "@" + name, ex); } } @@ -133,11 +127,8 @@ public virtual string[] GetValueNames() { return Exists() ? WrappedKey.GetValueNames() : Array.Empty(); } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.NotExpectedRegistryException(ex)) { - if (ExceptionHandling.NotExpectedRegistryException(ex)) - throw; - throw new RegistryException(ex.Message, Name, ex); } } @@ -152,11 +143,8 @@ public virtual string[] GetSubKeyNames() { return Exists() ? WrappedKey.GetSubKeyNames() : Array.Empty(); } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.NotExpectedRegistryException(ex)) { - if (ExceptionHandling.NotExpectedRegistryException(ex)) - throw; - throw new RegistryException(ex.Message, Name, ex); } } @@ -217,11 +205,8 @@ private RegistryKey WrappedKey { _wrappedKey = _registryHive.OpenSubKey(_registryKeyPath); } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.NotExpectedRegistryException(ex)) { - if (ExceptionHandling.NotExpectedRegistryException(ex)) - throw; - throw new RegistryException(ex.Message, _wrappedKey == null ? string.Empty : Name, ex); } finally diff --git a/src/Framework/NativeMethods.cs b/src/Framework/NativeMethods.cs index 8f92cf66d93..a8174a2cbbb 100644 --- a/src/Framework/NativeMethods.cs +++ b/src/Framework/NativeMethods.cs @@ -1225,14 +1225,10 @@ internal static void KillTree(int processIdToKill) // Kill this process, so that no further children can be created. thisProcess.Kill(); } - catch (Win32Exception e) + catch (Win32Exception e) when (e.NativeErrorCode == ERROR_ACCESS_DENIED) { // Access denied is potentially expected -- it happens when the process that // we're attempting to kill is already dead. So just ignore in that case. - if (e.NativeErrorCode != ERROR_ACCESS_DENIED) - { - throw; - } } // Now enumerate our children. Children of this process are any process which has this process id as its parent diff --git a/src/MSBuild/OutOfProcTaskAppDomainWrapperBase.cs b/src/MSBuild/OutOfProcTaskAppDomainWrapperBase.cs index b1f085e2da1..a29ca3803dd 100644 --- a/src/MSBuild/OutOfProcTaskAppDomainWrapperBase.cs +++ b/src/MSBuild/OutOfProcTaskAppDomainWrapperBase.cs @@ -117,29 +117,19 @@ IDictionary taskParams TypeLoader typeLoader = new TypeLoader(TaskLoader.IsTaskClass); taskType = typeLoader.Load(taskName, AssemblyLoadInfo.Create(null, taskLocation)); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - - Exception exceptionToReturn = e; - // If it's a TargetInvocationException, we only care about the contents of the inner exception, - // so just save that instead. - if (e is TargetInvocationException) - { - exceptionToReturn = e.InnerException; - } + // so just save that instead. + Exception exceptionToReturn = e is TargetInvocationException ? e.InnerException : e; return new OutOfProcTaskHostTaskResult - ( - TaskCompleteType.CrashedDuringInitialization, - exceptionToReturn, - "TaskInstantiationFailureError", - new string[] { taskName, taskLocation, String.Empty } - ); + ( + TaskCompleteType.CrashedDuringInitialization, + exceptionToReturn, + "TaskInstantiationFailureError", + new string[] { taskName, taskLocation, String.Empty } + ); } OutOfProcTaskHostTaskResult taskResult; @@ -239,13 +229,8 @@ IDictionary taskParams taskParams ); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - exceptionFromExecution = e; } finally @@ -321,13 +306,8 @@ IDictionary taskParams wrappedTask.BuildEngine = oopTaskHostNode; } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Exception exceptionToReturn = e; // If it's a TargetInvocationException, we only care about the contents of the inner exception, @@ -353,29 +333,16 @@ IDictionary taskParams PropertyInfo paramInfo = wrappedTask.GetType().GetProperty(param.Key, BindingFlags.Instance | BindingFlags.Public); paramInfo.SetValue(wrappedTask, param.Value?.WrappedParameter, null); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - - Exception exceptionToReturn = e; - - // If it's a TargetInvocationException, we only care about the contents of the inner exception, - // so just save that instead. - if (e is TargetInvocationException) - { - exceptionToReturn = e.InnerException; - } - return new OutOfProcTaskHostTaskResult - ( - TaskCompleteType.CrashedDuringInitialization, - exceptionToReturn, - "InvalidTaskAttributeError", - new string[] { param.Key, param.Value.ToString(), taskName } - ); + ( + TaskCompleteType.CrashedDuringInitialization, + // If it's a TargetInvocationException, we only care about the contents of the inner exception, so save that instead. + e is TargetInvocationException ? e.InnerException : e, + "InvalidTaskAttributeError", + new string[] { param.Key, param.Value.ToString(), taskName } + ); } } @@ -390,13 +357,8 @@ IDictionary taskParams // If it didn't crash and return before now, we're clear to go ahead and execute here. success = wrappedTask.Execute(); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - return new OutOfProcTaskHostTaskResult(TaskCompleteType.CrashedDuringExecution, e); } @@ -412,13 +374,8 @@ IDictionary taskParams { finalParameterValues[value.Name] = value.GetValue(wrappedTask, null); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - // If it's not a critical exception, we assume there's some sort of problem in the parameter getter -- // so save the exception, and we'll re-throw once we're back on the main node side of the // communications pipe. diff --git a/src/MSBuild/OutOfProcTaskHostNode.cs b/src/MSBuild/OutOfProcTaskHostNode.cs index e747dbcc9a3..0f26eb588c3 100644 --- a/src/MSBuild/OutOfProcTaskHostNode.cs +++ b/src/MSBuild/OutOfProcTaskHostNode.cs @@ -905,22 +905,14 @@ private void RunTask(object state) taskParams ); } - catch (Exception e) + catch (ThreadAbortException) { - if (e is ThreadAbortException) - { - // This thread was aborted as part of Cancellation, we will return a failure task result - taskResult = new OutOfProcTaskHostTaskResult(TaskCompleteType.Failure); - } - else - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - else - { - taskResult = new OutOfProcTaskHostTaskResult(TaskCompleteType.CrashedDuringExecution, e); - } + // This thread was aborted as part of Cancellation, we will return a failure task result + taskResult = new OutOfProcTaskHostTaskResult(TaskCompleteType.Failure); + } + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) + { + taskResult = new OutOfProcTaskHostTaskResult(TaskCompleteType.CrashedDuringExecution, e); } finally { @@ -931,10 +923,7 @@ private void RunTask(object state) IDictionary currentEnvironment = CommunicationsUtilities.GetEnvironmentVariables(); currentEnvironment = UpdateEnvironmentForMainNode(currentEnvironment); - if (taskResult == null) - { - taskResult = new OutOfProcTaskHostTaskResult(TaskCompleteType.Failure); - } + taskResult ??= new OutOfProcTaskHostTaskResult(TaskCompleteType.Failure); lock (_taskCompleteLock) { diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index ae95d608193..cd16eb39454 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -137,17 +137,12 @@ static MSBuildApp() s_initialized = true; } - catch (TypeInitializationException ex) - { - if (ex.InnerException == null -#if !FEATURE_SYSTEM_CONFIGURATION - ) -#else - || ex.InnerException.GetType() != typeof(ConfigurationErrorsException)) + catch (TypeInitializationException ex) when (ex.InnerException is not null +#if FEATURE_SYSTEM_CONFIGURATION + && ex.InnerException is ConfigurationErrorsException #endif - { - throw; - } + ) + { HandleConfigurationException(ex); } #if FEATURE_SYSTEM_CONFIGURATION diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index 6f089b3e8ad..3c46ce9f165 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -2527,14 +2527,9 @@ private string[] GetFilesImplementation( taskOptions); } // Catch exceptions that are thrown inside the Parallel.ForEach - catch (AggregateException ex) + catch (AggregateException ex) when (InnerExceptionsAreAllIoRelated(ex)) { - // Flatten to get exceptions than are thrown inside a nested Parallel.ForEach - if (ex.Flatten().InnerExceptions.All(ExceptionHandling.IsIoRelatedException)) - { - return CreateArrayWithSingleItemIfNotExcluded(filespecUnescaped, excludeSpecsUnescaped); - } - throw; + return CreateArrayWithSingleItemIfNotExcluded(filespecUnescaped, excludeSpecsUnescaped); } catch (Exception ex) when (ExceptionHandling.IsIoRelatedException(ex)) { @@ -2552,6 +2547,11 @@ private string[] GetFilesImplementation( return files; } + private bool InnerExceptionsAreAllIoRelated(AggregateException ex) + { + return ex.Flatten().InnerExceptions.All(ExceptionHandling.IsIoRelatedException); + } + private static bool IsSubdirectoryOf(string possibleChild, string possibleParent) { if (possibleParent == string.Empty) diff --git a/src/Shared/LogMessagePacketBase.cs b/src/Shared/LogMessagePacketBase.cs index 536d31986cb..36f08754dcf 100644 --- a/src/Shared/LogMessagePacketBase.cs +++ b/src/Shared/LogMessagePacketBase.cs @@ -487,17 +487,13 @@ private static Delegate CreateDelegateRobust(Type type, Object firstArgument, Me delegateMethod = methodInfo.CreateDelegate(type, firstArgument); #endif } - catch (FileLoadException) + catch (FileLoadException) when (i < 5) { // Sometimes, in 64-bit processes, the fusion load of Microsoft.Build.Framework.dll - // spontaneously fails when trying to bind to the delegate. However, it seems to - // not repeat on additional tries -- so we'll try again a few times. However, if - // it keeps happening, it's probably a real problem, so we want to go ahead and - // throw to let the user know what's up. - if (i == 5) - { - throw; - } + // spontaneously fails when trying to bind to the delegate. However, it seems to + // not repeat on additional tries -- so we'll try again a few times. However, if + // it keeps happening, it's probably a real problem, so we want to go ahead and + // throw to let the user know what's up. } } diff --git a/src/Shared/NodeEndpointOutOfProcBase.cs b/src/Shared/NodeEndpointOutOfProcBase.cs index 4562c4309d7..7880aa1690c 100644 --- a/src/Shared/NodeEndpointOutOfProcBase.cs +++ b/src/Shared/NodeEndpointOutOfProcBase.cs @@ -447,13 +447,8 @@ private void PacketPumpProc() ChangeLinkStatus(LinkStatus.Active); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - CommunicationsUtilities.Trace("Client connection failed. Exiting comm thread. {0}", e); if (localPipeServer.IsConnected) { diff --git a/src/Shared/RegisteredTaskObjectCacheBase.cs b/src/Shared/RegisteredTaskObjectCacheBase.cs index 3e8cabaf744..91e69812149 100644 --- a/src/Shared/RegisteredTaskObjectCacheBase.cs +++ b/src/Shared/RegisteredTaskObjectCacheBase.cs @@ -147,13 +147,8 @@ private static void DisposeObjects(Lazy> li IDisposable disposable = obj as IDisposable; disposable?.Dispose(); } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex)) { - if (ExceptionHandling.IsCriticalException(ex)) - { - throw; - } - // Eat it. We don't have a way to log here because at a minimum the build has already completed. } } diff --git a/src/Shared/TaskParameterTypeVerifier.cs b/src/Shared/TaskParameterTypeVerifier.cs index 0b0f0cac815..685227c85ec 100644 --- a/src/Shared/TaskParameterTypeVerifier.cs +++ b/src/Shared/TaskParameterTypeVerifier.cs @@ -3,8 +3,8 @@ using System; using Microsoft.Build.Framework; -using System.Reflection; using Microsoft.Build.Shared; +using System.Reflection; namespace Microsoft.Build.BackEnd { @@ -16,11 +16,8 @@ internal static class TaskParameterTypeVerifier ///

/// Is the parameter type a valid scalar input value /// - internal static bool IsValidScalarInputParameter(Type parameterType) - { - bool result = (parameterType.GetTypeInfo().IsValueType || parameterType == typeof(string) || parameterType == typeof(ITaskItem)); - return result; - } + internal static bool IsValidScalarInputParameter(Type parameterType) => + parameterType.GetTypeInfo().IsValueType || parameterType == typeof(string) || parameterType == typeof(ITaskItem); /// /// Is the passed in parameterType a valid vector input parameter @@ -71,4 +68,4 @@ internal static bool IsValidOutputParameter(Type parameterType) return IsValueTypeOutputParameter(parameterType) || IsAssignableToITask(parameterType); } } -} \ No newline at end of file +} diff --git a/src/Shared/UnitTests/ObjectModelHelpers.cs b/src/Shared/UnitTests/ObjectModelHelpers.cs index e647fd709c0..142b9c361c3 100644 --- a/src/Shared/UnitTests/ObjectModelHelpers.cs +++ b/src/Shared/UnitTests/ObjectModelHelpers.cs @@ -877,19 +877,10 @@ internal static void DeleteDirectory(string dir) break; } - catch (Exception ex) + // After all the retries fail, we 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()); } } } @@ -924,19 +915,10 @@ internal static string CreateFileInTempProjectDirectory(string fileRelativePath, } break; } - catch (Exception ex) + // After all the retries fail, we 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()); } } diff --git a/src/Tasks/AppConfig/BindingRedirect.cs b/src/Tasks/AppConfig/BindingRedirect.cs index 8a226ede9c5..9e78746aa0b 100644 --- a/src/Tasks/AppConfig/BindingRedirect.cs +++ b/src/Tasks/AppConfig/BindingRedirect.cs @@ -54,13 +54,8 @@ internal void Read(XmlReader reader) OldVersionHigh = new Version(oldVersion); } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - ErrorUtilities.ThrowArgument(e, "AppConfig.InvalidOldVersionAttribute", e.Message); } @@ -73,13 +68,8 @@ internal void Read(XmlReader reader) { NewVersion = new Version(newVersionAttribute); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - ErrorUtilities.ThrowArgument(e, "AppConfig.InvalidNewVersionAttribute", e.Message); } } diff --git a/src/Tasks/AssemblyDependency/AssemblyInformation.cs b/src/Tasks/AssemblyDependency/AssemblyInformation.cs index 171e223a5d7..722fec9079b 100644 --- a/src/Tasks/AssemblyDependency/AssemblyInformation.cs +++ b/src/Tasks/AssemblyDependency/AssemblyInformation.cs @@ -330,12 +330,8 @@ private FrameworkName GetFrameworkName() } } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } } return frameworkAttribute; diff --git a/src/Tasks/AssemblyDependency/ReferenceTable.cs b/src/Tasks/AssemblyDependency/ReferenceTable.cs index 3b214c55a9f..d08f1e780a7 100644 --- a/src/Tasks/AssemblyDependency/ReferenceTable.cs +++ b/src/Tasks/AssemblyDependency/ReferenceTable.cs @@ -2983,13 +2983,8 @@ private bool VerifyArchitectureOfImplementationDll(string dllPath, string winmdF return true; } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - _log.LogErrorWithCodeFromResources("ResolveAssemblyReference.ProblemReadingImplementationDll", dllPath, e.Message); return false; } diff --git a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs index 53b19c17a62..c3a5aa7d546 100644 --- a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs +++ b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs @@ -2628,12 +2628,8 @@ private AssemblyNameExtension[] GetDependencies(Reference resolvedReference, Fil getAssemblyMetadata(resolvedReference.FullPath, assemblyMetadataCache, out result, out scatterFiles, out frameworkName); } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } } } diff --git a/src/Tasks/Copy.cs b/src/Tasks/Copy.cs index 1575b55d11b..b3f316b9031 100644 --- a/src/Tasks/Copy.cs +++ b/src/Tasks/Copy.cs @@ -785,55 +785,56 @@ private bool DoCopyWithRetries(FileState sourceFileState, FileState destinationF } catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) { - if (e is ArgumentException || // Invalid chars - e is NotSupportedException || // Colon in the middle of the path - e is PathTooLongException) + switch (e) { - // No use retrying these cases - throw; - } + case ArgumentException: // Invalid chars + case NotSupportedException: // Colon in the middle of the path + case PathTooLongException: + throw; + case UnauthorizedAccessException: + case IOException: // Not clear why we can get one and not the other + int code = Marshal.GetHRForException(e); + + LogDiagnostic("Got {0} copying {1} to {2} and HR is {3}", e.ToString(), sourceFileState.Name, destinationFileState.Name, code); + if (code == NativeMethods.ERROR_ACCESS_DENIED) + { + // ERROR_ACCESS_DENIED can either mean there's an ACL preventing us, or the file has the readonly bit set. + // In either case, that's likely not a race, and retrying won't help. + // Retrying is mainly for ERROR_SHARING_VIOLATION, where someone else is using the file right now. + // However, there is a limited set of circumstances where a copy failure will show up as access denied due + // to a failure to reset the readonly bit properly, in which case retrying will succeed. This seems to be + // a pretty edge scenario, but since some of our internal builds appear to be hitting it, provide a secret + // environment variable to allow overriding the default behavior and forcing retries in this circumstance as well. + if (!s_alwaysRetryCopy) + { + throw; + } + else + { + LogDiagnostic("Retrying on ERROR_ACCESS_DENIED because MSBUILDALWAYSRETRY = 1"); + } + } - if (e is UnauthorizedAccessException || e is IOException) // Not clear why we can get one and not the other - { - int code = Marshal.GetHRForException(e); + if (e is UnauthorizedAccessException) + { + break; + } - LogDiagnostic("Got {0} copying {1} to {2} and HR is {3}", e.ToString(), sourceFileState.Name, destinationFileState.Name, code); - if (code == NativeMethods.ERROR_ACCESS_DENIED) - { - // ERROR_ACCESS_DENIED can either mean there's an ACL preventing us, or the file has the readonly bit set. - // In either case, that's likely not a race, and retrying won't help. - // Retrying is mainly for ERROR_SHARING_VIOLATION, where someone else is using the file right now. - // However, there is a limited set of circumstances where a copy failure will show up as access denied due - // to a failure to reset the readonly bit properly, in which case retrying will succeed. This seems to be - // a pretty edge scenario, but since some of our internal builds appear to be hitting it, provide a secret - // environment variable to allow overriding the default behavior and forcing retries in this circumstance as well. - if (!s_alwaysRetryCopy) + if (DestinationFolder != null && FileSystems.Default.FileExists(DestinationFolder.ItemSpec)) { + // We failed to create the DestinationFolder because it's an existing file. No sense retrying. + // We don't check for this case upstream because it'd be another hit to the filesystem. throw; } - else + + // if this was just because the source and destination files are the + // same file, that's not a failure. + // Note -- we check this exceptional case here, not before the copy, for perf. + if (PathsAreIdentical(sourceFileState.Name, destinationFileState.Name)) { - LogDiagnostic("Retrying on ERROR_ACCESS_DENIED because MSBUILDALWAYSRETRY = 1"); + return true; } - } - } - - if (e is IOException && DestinationFolder != null && FileSystems.Default.FileExists(DestinationFolder.ItemSpec)) - { - // We failed to create the DestinationFolder because it's an existing file. No sense retrying. - // We don't check for this case upstream because it'd be another hit to the filesystem. - throw; - } - - if (e is IOException) - { - // if this was just because the source and destination files are the - // same file, that's not a failure. - // Note -- we check this exceptional case here, not before the copy, for perf. - if (PathsAreIdentical(sourceFileState.Name, destinationFileState.Name)) - { - return true; - } + break; } if (retries < Retries) diff --git a/src/Tasks/ErrorFromResources.cs b/src/Tasks/ErrorFromResources.cs index 83c5f106d6a..8cc2f2d34eb 100644 --- a/src/Tasks/ErrorFromResources.cs +++ b/src/Tasks/ErrorFromResources.cs @@ -55,13 +55,8 @@ public override bool Execute() Log.LogError(null, Code, HelpKeyword, File, 0, 0, 0, 0, message); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("ErrorFromResources.LogErrorFailure", Resource, e.Message); } diff --git a/src/Tasks/GenerateResource.cs b/src/Tasks/GenerateResource.cs index a5f9c7646b3..f779707ac25 100644 --- a/src/Tasks/GenerateResource.cs +++ b/src/Tasks/GenerateResource.cs @@ -1548,13 +1548,8 @@ private bool ShouldRebuildResgenOutputFile(string sourceFilePath, string outputF { resxFileInfo = _cache.GetResXFileInfo(sourceFilePath, UsePreserializedResources); } - catch (Exception e) // Catching Exception, but rethrowing unless it's a well-known exception. + catch (Exception e) when (!ExceptionHandling.NotExpectedIoOrXmlException(e) || e is MSBuildResXException) { - if (ExceptionHandling.NotExpectedIoOrXmlException(e) && !(e is MSBuildResXException)) - { - throw; - } - // Return true, so that resource processing will display the error // No point logging a duplicate error here as well return true; @@ -1938,7 +1933,7 @@ private bool NeedSeparateAppDomain() return true; } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { // DDB#9819 // Customers have reported the following exceptions coming out of this method's call to GetType(): @@ -1949,8 +1944,6 @@ private bool NeedSeparateAppDomain() // Any problem loading the type will get logged later when the resource reader tries it. // // XmlException or an IO exception is also possible from an invalid input file. - if (ExceptionHandling.IsCriticalException(e)) - throw; // If there was any problem parsing the .resx then log a message and // fall back to using a separate AppDomain. @@ -2791,13 +2784,9 @@ e is SerializationException || { Directory.Delete(currentOutputDirectory); // Remove output directory if empty } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { // Fail silently (we are not even checking if the call to File.Delete succeeded) - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } } } return false; @@ -4049,12 +4038,8 @@ private Assembly GetAssemblyByPath(string pathToAssembly, bool throwOnError) { _cachedAssemblies[pathToAssembly] = Assembly.UnsafeLoadFrom(pathToAssembly); } - catch + catch when (!throwOnError) { - if (throwOnError) - { - throw; - } } } diff --git a/src/Tasks/GenerateTrustInfo.cs b/src/Tasks/GenerateTrustInfo.cs index 9dbd8e79e4d..1561626eac4 100644 --- a/src/Tasks/GenerateTrustInfo.cs +++ b/src/Tasks/GenerateTrustInfo.cs @@ -85,16 +85,9 @@ public override bool Execute() Log.LogErrorWithCodeFromResources("GenerateManifest.NoPermissionSetForTargetZone", dotNetVersion); return false; } - catch (ArgumentException ex) + catch (ArgumentException ex) when (String.Equals(ex.ParamName, "TargetZone", StringComparison.OrdinalIgnoreCase)) { - if (String.Equals(ex.ParamName, "TargetZone", StringComparison.OrdinalIgnoreCase)) - { - Log.LogWarningWithCodeFromResources("GenerateManifest.InvalidItemValue", "TargetZone", TargetZone); - } - else - { - throw; - } + Log.LogWarningWithCodeFromResources("GenerateManifest.InvalidItemValue", "TargetZone", TargetZone); } // Write trust-info back to a stand-alone trust file diff --git a/src/Tasks/GetInstalledSDKLocations.cs b/src/Tasks/GetInstalledSDKLocations.cs index e5e8195e1f8..b54684a84a3 100644 --- a/src/Tasks/GetInstalledSDKLocations.cs +++ b/src/Tasks/GetInstalledSDKLocations.cs @@ -137,13 +137,8 @@ public override bool Execute() Version platformVersion = Version.Parse(TargetPlatformVersion); installedSDKs = ToolLocationHelper.GetPlatformExtensionSDKLocationsAndVersions(SDKDirectoryRoots, SDKExtensionDirectoryRoots, SDKRegistryRoot, TargetPlatformIdentifier, platformVersion); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("GetInstalledSDKs.CouldNotGetSDKList", e.Message); } diff --git a/src/Tasks/GetSDKReferenceFiles.cs b/src/Tasks/GetSDKReferenceFiles.cs index dfc8d0050ac..115ac80274c 100644 --- a/src/Tasks/GetSDKReferenceFiles.cs +++ b/src/Tasks/GetSDKReferenceFiles.cs @@ -279,13 +279,8 @@ internal bool Execute(GetAssemblyName getAssemblyName, GetAssemblyRuntimeVersion } } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("GetSDKReferenceFiles.CouldNotGetSDKReferenceFiles", e.Message); } @@ -1095,13 +1090,8 @@ internal bool IsAssemblyListCacheFileUpToDate(string sdkIdentity, string sdkRoot return true; } } - catch (Exception ex) + catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex)) { - if (ExceptionHandling.IsCriticalException(ex)) - { - throw; - } - // Queue up for later logging, does not matter if the cache got written _exceptionMessages.Enqueue(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("GetSDKReferenceFiles.ProblemGeneratingHash", currentAssembly, ex.Message)); @@ -1134,13 +1124,8 @@ private SdkReferenceInfo GetSDKReferenceInfo(string referencePath) } } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - // Queue up for later logging, does not matter if the cache got written _exceptionMessages.Enqueue(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("GetSDKReferenceFiles.ProblemGettingAssemblyMetadata", referencePath, e.Message)); } diff --git a/src/Tasks/ManifestUtil/AssemblyIdentity.cs b/src/Tasks/ManifestUtil/AssemblyIdentity.cs index 772c790c23e..e9d60173927 100644 --- a/src/Tasks/ManifestUtil/AssemblyIdentity.cs +++ b/src/Tasks/ManifestUtil/AssemblyIdentity.cs @@ -268,12 +268,8 @@ public static AssemblyIdentity FromManagedAssembly(string path) { identity = new AssemblyIdentity(r.Name, r.Version, r.PublicKeyToken, r.Culture, r.ProcessorArchitecture); } - catch (ArgumentException e) + catch (ArgumentException e) when (e.HResult == unchecked((int)0x80070057)) { - if (e.HResult != unchecked((int)0x80070057)) - { - throw; - } // 0x80070057 - "Value does not fall within the expected range." is returned from // GetAssemblyIdentityFromFile for WinMD components } diff --git a/src/Tasks/ManifestUtil/SecurityUtil.cs b/src/Tasks/ManifestUtil/SecurityUtil.cs index d74182f6ec8..9aa6d5590e4 100644 --- a/src/Tasks/ManifestUtil/SecurityUtil.cs +++ b/src/Tasks/ManifestUtil/SecurityUtil.cs @@ -702,15 +702,11 @@ private static void SignPEFile(X509Certificate2 cert, Uri timestampUrl, string p { SignPEFileInternal(cert, timestampUrl, path, resources, useSha256, true); } - catch(ApplicationException) + catch(ApplicationException) when (timestampUrl != null) { // error, retry with signtool /t if timestamp url was given - if (timestampUrl != null) - { - SignPEFileInternal(cert, timestampUrl, path, resources, useSha256, false); - return; - } - throw; + SignPEFileInternal(cert, timestampUrl, path, resources, useSha256, false); + return; } } diff --git a/src/Tasks/XamlTaskFactory/TaskParser.cs b/src/Tasks/XamlTaskFactory/TaskParser.cs index ad7d411177f..3f40137fefa 100644 --- a/src/Tasks/XamlTaskFactory/TaskParser.cs +++ b/src/Tasks/XamlTaskFactory/TaskParser.cs @@ -126,13 +126,8 @@ private bool ParseAsContentOrFile(string contentOrFile, string desiredRule) if (!isRootedPath) maybeFullPath = Path.GetFullPath(contentOrFile); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - // We will get an exception if the contents are not a path (for instance, they are actual XML.) } diff --git a/src/Tasks/XmlPeek.cs b/src/Tasks/XmlPeek.cs index 4b16bbbb33b..7ee125c84dd 100644 --- a/src/Tasks/XmlPeek.cs +++ b/src/Tasks/XmlPeek.cs @@ -84,13 +84,8 @@ public override bool Execute() { xmlinput = new XmlInput(XmlInputPath, XmlContent); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XmlPeek.ArgumentError", e.Message); return false; } @@ -105,13 +100,8 @@ public override bool Execute() xr.Dispose(); } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XmlPeekPoke.InputFileError", XmlInputPath.ItemSpec, e.Message); return false; } @@ -127,13 +117,8 @@ public override bool Execute() // Create the expression from query expr = nav.Compile(_query); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XmlPeekPoke.XPathError", _query, e.Message); return false; } @@ -145,13 +130,8 @@ public override bool Execute() { LoadNamespaces(ref xmlNamespaceManager, Namespaces); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XmlPeek.NamespacesError", e.Message); return false; } diff --git a/src/Tasks/XmlPoke.cs b/src/Tasks/XmlPoke.cs index 8f42cfe910c..d82178315da 100644 --- a/src/Tasks/XmlPoke.cs +++ b/src/Tasks/XmlPoke.cs @@ -110,13 +110,8 @@ public override bool Execute() } } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XmlPeekPoke.InputFileError", _xmlInputPath.ItemSpec, e.Message); return false; } @@ -129,13 +124,8 @@ public override bool Execute() // Create the expression from query expr = nav.Compile(_query); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XmlPeekPoke.XPathError", _query, e.Message); return false; } @@ -148,13 +138,8 @@ public override bool Execute() { LoadNamespaces(ref xmlNamespaceManager, Namespaces); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XmlPoke.NamespacesError", e.Message); return false; } @@ -180,13 +165,8 @@ public override bool Execute() iter.Current.InnerXml = _value.ItemSpec; Log.LogMessageFromResources(MessageImportance.Low, "XmlPoke.Replaced", iter.Current.Name, _value.ItemSpec); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XmlPoke.PokeError", _value.ItemSpec, e.Message); return false; } diff --git a/src/Tasks/XslTransformation.cs b/src/Tasks/XslTransformation.cs index 0f47a2edfe2..456b5334d38 100644 --- a/src/Tasks/XslTransformation.cs +++ b/src/Tasks/XslTransformation.cs @@ -98,13 +98,8 @@ public override bool Execute() xmlinput = new XmlInput(XmlInputPaths, XmlContent); xsltinput = new XsltInput(XslInputPath, XslContent, XslCompiledDllPath, Log); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XslTransform.ArgumentError", e.Message); return false; } @@ -130,13 +125,8 @@ public override bool Execute() { arguments = ProcessXsltArguments(Parameters); } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XslTransform.XsltArgumentsError", e.Message); return false; } @@ -153,13 +143,8 @@ public override bool Execute() Log.LogErrorWithCodeFromResources("XslTransform.PrecompiledXsltError"); return false; } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XslTransform.XsltLoadError", e.Message); return false; } @@ -180,13 +165,8 @@ public override bool Execute() } } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - Log.LogErrorWithCodeFromResources("XslTransform.TransformError", e.Message); return false; } diff --git a/src/Utilities/MuxLogger.cs b/src/Utilities/MuxLogger.cs index fdb9263ac2e..f1461dfcf47 100644 --- a/src/Utilities/MuxLogger.cs +++ b/src/Utilities/MuxLogger.cs @@ -644,6 +644,7 @@ private void RaiseMessageEvent(object sender, BuildMessageEventArgs buildEvent) // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -690,6 +691,7 @@ private void RaiseErrorEvent(object sender, BuildErrorEventArgs buildEvent) // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -734,6 +736,7 @@ private void RaiseWarningEvent(object sender, BuildWarningEventArgs buildEvent) // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -773,6 +776,7 @@ private void RaiseBuildStartedEvent(object sender, BuildStartedEventArgs buildEv // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -817,6 +821,7 @@ private void RaiseBuildFinishedEvent(object sender, BuildFinishedEventArgs build // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -871,6 +876,7 @@ private void RaiseProjectStartedEvent(object sender, ProjectStartedEventArgs bui // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -912,6 +918,7 @@ private void RaiseProjectFinishedEvent(object sender, ProjectFinishedEventArgs b // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -953,6 +960,7 @@ private void RaiseTargetStartedEvent(object sender, TargetStartedEventArgs build // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -994,6 +1002,7 @@ private void RaiseTargetFinishedEvent(object sender, TargetFinishedEventArgs bui // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -1035,6 +1044,7 @@ private void RaiseTaskStartedEvent(object sender, TaskStartedEventArgs buildEven // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -1076,6 +1086,7 @@ private void RaiseTaskFinishedEvent(object sender, TaskFinishedEventArgs buildEv // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -1117,6 +1128,7 @@ private void RaiseCustomEvent(object sender, CustomBuildEventArgs buildEvent) // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -1163,6 +1175,7 @@ private void RaiseStatusEvent(object sender, BuildStatusEventArgs buildEvent, bo // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -1224,6 +1237,7 @@ private void RaiseAnyEvent(object sender, BuildEventArgs buildEvent) // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) @@ -1269,6 +1283,7 @@ private void RaiseTelemetryEvent(object sender, TelemetryEventArgs buildEvent) // first unregister all loggers, since other loggers may receive remaining events in unexpected orderings // if a fellow logger is throwing in an event handler. UnregisterAllEventHandlers(); + throw; } catch (Exception) diff --git a/src/Utilities/PlatformManifest.cs b/src/Utilities/PlatformManifest.cs index 0be3579553d..0ed2911bbd0 100644 --- a/src/Utilities/PlatformManifest.cs +++ b/src/Utilities/PlatformManifest.cs @@ -150,13 +150,8 @@ private void LoadManifestFile() ReadErrorMessage = ResourceUtilities.FormatResourceStringStripCodeAndKeyword("PlatformManifest.MissingPlatformXml", platformManifestPath); } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - ReadErrorMessage = e.Message; } } diff --git a/src/Utilities/SDKManifest.cs b/src/Utilities/SDKManifest.cs index da9c9085073..7d1d22b230d 100644 --- a/src/Utilities/SDKManifest.cs +++ b/src/Utilities/SDKManifest.cs @@ -352,13 +352,8 @@ private void LoadManifestFile() } } } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } - ReadError = true; ReadErrorMessage = e.Message; } diff --git a/src/Utilities/ToolLocationHelper.cs b/src/Utilities/ToolLocationHelper.cs index 6237b17f4c7..fee8a61d89e 100644 --- a/src/Utilities/ToolLocationHelper.cs +++ b/src/Utilities/ToolLocationHelper.cs @@ -1467,13 +1467,9 @@ public static string GetFoldersInVSInstallsAsString(string minVersionString = nu foldersString = string.Join(";", folders); } } - catch(Exception e) + catch(Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - // this method will be used in vc props and we don't want to fail project load if it throws for some non critical reason. - if (ExceptionHandling.IsCriticalException(e)) - { - throw; - } + // This method will be used in vc props and we don't want to fail project load if it throws for some non critical reason. } return foldersString; @@ -3237,11 +3233,8 @@ internal static string ChainReferenceAssemblyPath(string targetFrameworkDirector return pathToReturn; } - catch (Exception e) + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { - if (ExceptionHandling.IsCriticalException(e)) - throw; - ErrorUtilities.ThrowInvalidOperation("ToolsLocationHelper.CouldNotCreateChain", path, pathToReturn, e.Message); }