Remove try catches choking all exceptions (#5206)
* remove some try catches choking all exceptions * rename ExceptionExtension to ReportAsWarning, show full stack instead of just message * dotnet-migrate try catch clean-up * fix migration test failures
This commit is contained in:
parent
a442449c55
commit
5fea7c3ae6
18 changed files with 212 additions and 71 deletions
|
@ -0,0 +1,22 @@
|
|||
<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0">
|
||||
|
||||
<PropertyGroup>
|
||||
<OutputType>Exe</OutputType>
|
||||
<TargetFramework>netcoreapp1.0</TargetFramework>
|
||||
<AssemblyName>dotnet-throwingtool</AssemblyName>
|
||||
<PackageId>$(AssemblyName)</PackageId>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<BuiltProjectOutputGroupOutput Include="$(ProjectRuntimeConfigFilePath)">
|
||||
<FinalOutputPath>$(ProjectRuntimeConfigFilePath)</FinalOutputPath>
|
||||
</BuiltProjectOutputGroupOutput>
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<PackageReference Include="Microsoft.NETCore.App">
|
||||
<Version>1.0.3</Version>
|
||||
</PackageReference>
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
|
@ -0,0 +1,12 @@
|
|||
using System;
|
||||
|
||||
namespace AppThrowing
|
||||
{
|
||||
class MyException : Exception
|
||||
{
|
||||
static void Main(string[] args)
|
||||
{
|
||||
throw new MyException();
|
||||
}
|
||||
}
|
||||
}
|
|
@ -0,0 +1,20 @@
|
|||
<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0">
|
||||
|
||||
<PropertyGroup>
|
||||
<OutputType>Exe</OutputType>
|
||||
<TargetFramework>netcoreapp1.0</TargetFramework>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<PackageReference Include="Microsoft.NETCore.App">
|
||||
<Version>1.0.3</Version>
|
||||
</PackageReference>
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<DotNetCliToolReference Include="dotnet-throwingtool">
|
||||
<Version>1.0.0</Version>
|
||||
</DotNetCliToolReference>
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
|
@ -0,0 +1,6 @@
|
|||
<?xml version="1.0" encoding="utf-8"?>
|
||||
<configuration>
|
||||
<packageSources>
|
||||
<add key="test-packages" value="../pkgs" />
|
||||
</packageSources>
|
||||
</configuration>
|
|
@ -0,0 +1,8 @@
|
|||
using System;
|
||||
|
||||
class Program
|
||||
{
|
||||
static void Main(string[] args)
|
||||
{
|
||||
}
|
||||
}
|
15
src/Microsoft.DotNet.Cli.Utils/ExceptionExtensions.cs
Normal file
15
src/Microsoft.DotNet.Cli.Utils/ExceptionExtensions.cs
Normal file
|
@ -0,0 +1,15 @@
|
|||
// Copyright (c) .NET Foundation and contributors. All rights reserved.
|
||||
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
|
||||
|
||||
using System;
|
||||
|
||||
namespace Microsoft.DotNet.Cli.Utils.ExceptionExtensions
|
||||
{
|
||||
internal static class ExceptionExtensions
|
||||
{
|
||||
public static void ReportAsWarning(this Exception e)
|
||||
{
|
||||
Reporter.Verbose.WriteLine($"Warning: Ignoring exception: {e.ToString().Yellow()}");
|
||||
}
|
||||
}
|
||||
}
|
|
@ -88,6 +88,20 @@ namespace Microsoft.DotNet.Tools.Common
|
|||
}
|
||||
}
|
||||
|
||||
public static bool TryDeleteDirectory(string directoryPath)
|
||||
{
|
||||
try
|
||||
{
|
||||
Directory.Delete(directoryPath, true);
|
||||
|
||||
return true;
|
||||
}
|
||||
catch
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns childItem relative to directory, with Path.DirectorySeparatorChar as separator
|
||||
/// </summary>
|
||||
|
|
|
@ -6,3 +6,4 @@ using System.Runtime.CompilerServices;
|
|||
[assembly: InternalsVisibleTo("Microsoft.DotNet.Tools.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
|
||||
[assembly: InternalsVisibleTo("Microsoft.DotNet.Cli.Utils.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
|
||||
[assembly: InternalsVisibleTo("Microsoft.DotNet.Tools.Tests.Utilities, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
|
||||
[assembly: InternalsVisibleTo("Microsoft.DotNet.ProjectJsonMigration, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
|
||||
|
|
|
@ -36,15 +36,9 @@ namespace Microsoft.Extensions.EnvironmentAbstractions
|
|||
|
||||
public void CreateEmptyFile(string path)
|
||||
{
|
||||
try
|
||||
using (File.Create(path))
|
||||
{
|
||||
var emptyFile = File.Create(path);
|
||||
if (emptyFile != null)
|
||||
{
|
||||
emptyFile.Dispose();
|
||||
}
|
||||
}
|
||||
catch { }
|
||||
}
|
||||
}
|
||||
}
|
|
@ -3,12 +3,13 @@
|
|||
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.IO;
|
||||
using System.Linq;
|
||||
using Microsoft.Build.Construction;
|
||||
using Microsoft.DotNet.Internal.ProjectModel;
|
||||
using Microsoft.DotNet.Internal.ProjectModel.Graph;
|
||||
using Microsoft.DotNet.Cli;
|
||||
using System.Linq;
|
||||
using System.IO;
|
||||
using Microsoft.DotNet.Cli.Utils.ExceptionExtensions;
|
||||
using Microsoft.DotNet.Cli.Sln.Internal;
|
||||
using Microsoft.DotNet.ProjectJsonMigration.Rules;
|
||||
using Microsoft.DotNet.Tools.Common;
|
||||
|
@ -87,14 +88,22 @@ namespace Microsoft.DotNet.ProjectJsonMigration
|
|||
try
|
||||
{
|
||||
File.Delete(Path.Combine(rootsettings.ProjectDirectory, "project.json"));
|
||||
} catch {}
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
e.ReportAsWarning();
|
||||
}
|
||||
|
||||
foreach (var projectDependency in projectDependencies)
|
||||
{
|
||||
try
|
||||
{
|
||||
File.Delete(projectDependency.ProjectFilePath);
|
||||
} catch { }
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
e.ReportAsWarning();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -90,21 +90,7 @@ namespace Microsoft.DotNet.Tools.Archive
|
|||
return 0;
|
||||
});
|
||||
|
||||
try
|
||||
{
|
||||
return app.Execute(args);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
#if DEBUG
|
||||
//Reporter.Error.WriteLine(ex.ToString());
|
||||
Console.WriteLine(ex.ToString());
|
||||
#else
|
||||
// Reporter.Error.WriteLine(ex.Message);
|
||||
Console.WriteLine(ex.Message);
|
||||
#endif
|
||||
return 1;
|
||||
}
|
||||
return app.Execute(args);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -4,6 +4,7 @@
|
|||
using System;
|
||||
using System.Runtime.Loader;
|
||||
using Microsoft.DotNet.Cli.Utils;
|
||||
using Microsoft.DotNet.Cli.Utils.ExceptionExtensions;
|
||||
using Microsoft.DotNet.Tools.Common;
|
||||
|
||||
namespace Microsoft.DotNet.Cli
|
||||
|
@ -51,8 +52,9 @@ namespace Microsoft.DotNet.Cli
|
|||
|
||||
return true;
|
||||
}
|
||||
catch (Exception)
|
||||
catch (Exception e)
|
||||
{
|
||||
e.ReportAsWarning();
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -82,22 +82,6 @@ namespace Microsoft.DotNet.Cli
|
|||
|
||||
return 1;
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
#if DEBUG
|
||||
Reporter.Error.WriteLine(ex.ToString());
|
||||
#else
|
||||
if (Reporter.IsVerbose)
|
||||
{
|
||||
Reporter.Error.WriteLine(ex.ToString());
|
||||
}
|
||||
else
|
||||
{
|
||||
Reporter.Error.WriteLine(ex.Message);
|
||||
}
|
||||
#endif
|
||||
return 1;
|
||||
}
|
||||
finally
|
||||
{
|
||||
if (PerfTrace.Enabled)
|
||||
|
|
|
@ -309,7 +309,7 @@ namespace Microsoft.DotNet.Tools.Migrate
|
|||
|
||||
if (!projects.Any())
|
||||
{
|
||||
throw new Exception("Unable to find any projects in global.json");
|
||||
throw new GracefulException("Unable to find any projects in global.json");
|
||||
}
|
||||
}
|
||||
else if (File.Exists(projectArg) &&
|
||||
|
@ -319,7 +319,7 @@ namespace Microsoft.DotNet.Tools.Migrate
|
|||
|
||||
if (!projects.Any())
|
||||
{
|
||||
throw new Exception($"Unable to find any projects in {projectArg}");
|
||||
throw new GracefulException($"Unable to find any projects in {projectArg}");
|
||||
}
|
||||
}
|
||||
else if (Directory.Exists(projectArg))
|
||||
|
@ -328,12 +328,12 @@ namespace Microsoft.DotNet.Tools.Migrate
|
|||
|
||||
if (!projects.Any())
|
||||
{
|
||||
throw new Exception($"No project.json file found in '{projectArg}'");
|
||||
throw new GracefulException($"No project.json file found in '{projectArg}'");
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
throw new Exception($"Invalid project argument - '{projectArg}' is not a project.json, global.json, or solution.sln file and a directory named '{projectArg}' doesn't exist.");
|
||||
throw new GracefulException($"Invalid project argument - '{projectArg}' is not a project.json, global.json, or solution.sln file and a directory named '{projectArg}' doesn't exist.");
|
||||
}
|
||||
|
||||
foreach (var project in projects)
|
||||
|
@ -346,7 +346,7 @@ namespace Microsoft.DotNet.Tools.Migrate
|
|||
{
|
||||
if (variable == null)
|
||||
{
|
||||
throw new Exception(message);
|
||||
throw new GracefulException(message);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -359,14 +359,14 @@ namespace Microsoft.DotNet.Tools.Migrate
|
|||
return projectJson;
|
||||
}
|
||||
|
||||
throw new Exception($"Unable to find project file at {projectJson}");
|
||||
throw new GracefulException($"Unable to find project file at {projectJson}");
|
||||
}
|
||||
|
||||
private IEnumerable<string> GetProjectsFromGlobalJson(string globalJson)
|
||||
{
|
||||
if (!File.Exists(globalJson))
|
||||
{
|
||||
throw new Exception($"Unable to find global settings file at {globalJson}");
|
||||
throw new GracefulException($"Unable to find global settings file at {globalJson}");
|
||||
}
|
||||
|
||||
var searchPaths = ProjectDependencyFinder.GetGlobalPaths(Path.GetDirectoryName(globalJson));
|
||||
|
@ -396,7 +396,7 @@ namespace Microsoft.DotNet.Tools.Migrate
|
|||
{
|
||||
if (!File.Exists(slnPath))
|
||||
{
|
||||
throw new Exception($"Unable to find the solution file at {slnPath}");
|
||||
throw new GracefulException($"Unable to find the solution file at {slnPath}");
|
||||
}
|
||||
|
||||
_slnFile = SlnFile.Read(slnPath);
|
||||
|
|
|
@ -78,16 +78,17 @@ namespace Microsoft.DotNet.Tools.Migrate
|
|||
{
|
||||
return app.Execute(args);
|
||||
}
|
||||
catch (Exception ex)
|
||||
catch (GracefulException e)
|
||||
{
|
||||
#if DEBUG
|
||||
Reporter.Error.WriteLine(ex.ToString());
|
||||
#else
|
||||
Reporter.Error.WriteLine(ex.Message);
|
||||
#endif
|
||||
Reporter.Error.WriteLine(e.Message);
|
||||
Reporter.Error.WriteLine(LocalizableStrings.MigrationFailedError);
|
||||
return 1;
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
Reporter.Error.WriteLine(LocalizableStrings.MigrationFailedError);
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -3,6 +3,7 @@ using Microsoft.DotNet.Cli;
|
|||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.IO;
|
||||
using Microsoft.DotNet.Cli.Utils;
|
||||
using Microsoft.DotNet.ProjectJsonMigration;
|
||||
using Microsoft.Build.Evaluation;
|
||||
|
||||
|
@ -77,7 +78,7 @@ namespace Microsoft.DotNet.Tools.Migrate
|
|||
MigrationTrace.Instance.WriteLine(commandResult.StdOut);
|
||||
MigrationTrace.Instance.WriteLine(commandResult.StdErr);
|
||||
|
||||
throw new Exception($"Failed to run {commandToExecute} in directory: {workingDirectory}");
|
||||
throw new GracefulException($"Failed to run {commandToExecute} in directory: {workingDirectory}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -155,19 +155,7 @@ namespace Microsoft.DotNet.Tools.New
|
|||
return dotnetNew.CreateEmptyProject(language.Name, fullTemplateName);
|
||||
});
|
||||
|
||||
try
|
||||
{
|
||||
return app.Execute(args);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
#if DEBUG
|
||||
Reporter.Error.WriteLine(ex.ToString());
|
||||
#else
|
||||
Reporter.Error.WriteLine(ex.Message);
|
||||
#endif
|
||||
return 1;
|
||||
}
|
||||
return app.Execute(args);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,78 @@
|
|||
// Copyright (c) .NET Foundation and contributors. All rights reserved.
|
||||
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
|
||||
|
||||
using System.IO;
|
||||
using FluentAssertions;
|
||||
using Microsoft.DotNet.Tools.Test.Utilities;
|
||||
using Xunit;
|
||||
|
||||
namespace Microsoft.DotNet.Cli.Utils.Tests
|
||||
{
|
||||
public class GivenAppThrowingException : TestBase
|
||||
{
|
||||
[Fact]
|
||||
public void ItShowsStackTraceWhenRun()
|
||||
{
|
||||
var root = TestAssets.Get("NonRestoredTestProjects", "AppThrowingException")
|
||||
.CreateInstance()
|
||||
.WithSourceFiles()
|
||||
.Root;
|
||||
|
||||
var appRoot = Path.Combine(root.FullName, "App");
|
||||
|
||||
new RestoreCommand()
|
||||
.WithWorkingDirectory(appRoot)
|
||||
.Execute()
|
||||
.Should().Pass();
|
||||
|
||||
string msg1 = "Unhandled Exception: AppThrowing.MyException: "
|
||||
+ "Exception of type 'AppThrowing.MyException' was thrown.";
|
||||
string msg2 = "at AppThrowing.MyException.Main(String[] args)";
|
||||
new RunCommand()
|
||||
.WithWorkingDirectory(appRoot)
|
||||
.ExecuteWithCapturedOutput()
|
||||
.Should().Fail()
|
||||
.And.HaveStdErrContaining(msg1)
|
||||
.And.HaveStdErrContaining(msg2);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ItShowsStackTraceWhenRunAsTool()
|
||||
{
|
||||
var root = TestAssets.Get("NonRestoredTestProjects", "AppThrowingException")
|
||||
.CreateInstance()
|
||||
.WithSourceFiles()
|
||||
.Root;
|
||||
|
||||
var appRoot = Path.Combine(root.FullName, "App");
|
||||
|
||||
new RestoreCommand()
|
||||
.WithWorkingDirectory(appRoot)
|
||||
.Execute()
|
||||
.Should().Pass();
|
||||
|
||||
new PackCommand()
|
||||
.WithWorkingDirectory(appRoot)
|
||||
.Execute("-o ../pkgs")
|
||||
.Should()
|
||||
.Pass();
|
||||
|
||||
var appWithToolDepRoot = Path.Combine(root.FullName, "AppDependingOnOtherAsTool");
|
||||
|
||||
new RestoreCommand()
|
||||
.WithWorkingDirectory(appWithToolDepRoot)
|
||||
.Execute()
|
||||
.Should().Pass();
|
||||
|
||||
string msg1 = "Unhandled Exception: AppThrowing.MyException: "
|
||||
+ "Exception of type 'AppThrowing.MyException' was thrown.";
|
||||
string msg2 = "at AppThrowing.MyException.Main(String[] args)";
|
||||
new TestCommand("dotnet")
|
||||
.WithWorkingDirectory(appWithToolDepRoot)
|
||||
.ExecuteWithCapturedOutput("throwingtool")
|
||||
.Should().Fail()
|
||||
.And.HaveStdErrContaining(msg1)
|
||||
.And.HaveStdErrContaining(msg2);
|
||||
}
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue