Print help text on specific domain exceptions (#51)

This commit is contained in:
Domn Werner
2020-04-25 11:59:03 -07:00
committed by GitHub
parent e04f0da318
commit 387fb72718
6 changed files with 104 additions and 100 deletions

View File

@@ -61,5 +61,17 @@ namespace CliFx.Tests
public ValueTask ExecuteAsync(IConsole console) => throw new CommandException(null);
}
[Command("inv")]
private class InvalidUserInputCommand : ICommand
{
[CommandOption("required", 'r')]
public string? RequiredOption { get; }
public ValueTask ExecuteAsync(IConsole console)
{
throw new NotImplementedException();
}
}
}
}

View File

@@ -169,8 +169,46 @@ namespace CliFx.Tests
// Assert
exitCode.Should().NotBe(0);
stdErrData.Should().Contain("Kaput");
stdErrData.Length.Should().BeGreaterThan("Kaput".Length);
stdErrData.Should().ContainAll(
"System.Exception:",
"Kaput", "at",
"CliFx.Tests");
}
[Fact]
public async Task Command_shows_help_text_on_exceptions_related_to_invalid_user_input()
{
// Arrange
await using var stdOut = new MemoryStream();
await using var stdErr = new MemoryStream();
var console = new VirtualConsole(output: stdOut, error: stdErr);
var application = new CliApplicationBuilder()
.AddCommand(typeof(InvalidUserInputCommand))
.UseConsole(console)
.Build();
// Act
var exitCode = await application.RunAsync(
new[] { "not-a-valid-command", "-r", "foo" },
new Dictionary<string, string>());
var stdErrData = console.Error.Encoding.GetString(stdErr.ToArray()).TrimEnd();
var stdOutData = console.Output.Encoding.GetString(stdOut.ToArray()).TrimEnd();
// Assert
exitCode.Should().NotBe(0);
stdErrData.Should().ContainAll(
"Can't find a command that matches the following arguments:",
"not-a-valid-command");
stdOutData.Should().ContainAll(
"Usage",
"[command]",
"Options",
"-h|--help", "Shows help text.",
"Commands",
"inv",
"You can run", "to show help on a specific command.");
}
}
}

View File

@@ -147,13 +147,13 @@ namespace CliFx
/// Handle <see cref="CommandException"/>s differently from the rest because we want to
/// display it different based on whether we are showing the help text or not.
/// </summary>
private int HandleCommandException(IReadOnlyList<string> commandLineArguments, CommandException commandException)
private int HandleCliFxException(IReadOnlyList<string> commandLineArguments, CliFxException cfe)
{
var showHelp = commandException.ShowHelp;
var showHelp = cfe.ShowHelp;
var errorMessage = commandException.HasMessage
? commandException.Message
: commandException.ToString();
var errorMessage = cfe.HasMessage
? cfe.Message
: cfe.ToString();
_console.WithForegroundColor(ConsoleColor.Red, () => _console.Error.WriteLine(errorMessage));
@@ -166,25 +166,7 @@ namespace CliFx
_helpTextWriter.Write(applicationSchema, commandSchema);
}
return commandException.ExitCode;
}
/// <summary>
/// Handles <see cref="CliFxException"/>s by printing its error message if it has a value.
/// Otherwise, it prints the full stack trace from the exception.
/// </summary>
/// <param name="cliFxException">The exception to handle.</param>
/// <returns>The exception's HResult.</returns>
private int HandleCliFxException(CliFxException cliFxException)
{
// Prefer showing message without stack trace on CliFxExceptions.
var errorMessage = cliFxException.HasMessage
? cliFxException.Message
: cliFxException.ToString();
_console.WithForegroundColor(ConsoleColor.Red, () => _console.Error.WriteLine(errorMessage));
return cliFxException.HResult;
return cfe.ExitCode;
}
/// <summary>
@@ -206,18 +188,13 @@ namespace CliFx
HandleHelpOption(applicationSchema, commandLineInput) ??
await HandleCommandExecutionAsync(applicationSchema, commandLineInput, environmentVariables);
}
catch (CommandException ce)
catch (CliFxException cfe)
{
// We want to catch exceptions in order to print errors and return correct exit codes.
// Doing this also gets rid of the annoying Windows troubleshooting dialog that shows up on unhandled exceptions.
var exitCode = HandleCommandException(commandLineArguments, ce);
var exitCode = HandleCliFxException(commandLineArguments, cfe);
return exitCode;
}
catch (CliFxException cfe)
{
var hResult = HandleCliFxException(cfe);
return hResult;
}
catch (Exception ex)
{
// For all other errors, we just write the entire thing to stderr.

View File

@@ -1,43 +0,0 @@
using System;
namespace CliFx.Exceptions
{
/// <summary>
/// Provides the base functionality for exceptions thrown within CliFx
/// or from one of its commands.
/// </summary>
public abstract class BaseCliFxException : Exception
{
/// <summary>
/// Whether to show the help text after handling this exception.
/// </summary>
public bool ShowHelp { get; }
/// <summary>
/// Whether this exception was constructed with a message.
/// </summary>
/// <remarks>
/// We cannot check against the 'Message' property because it will always return
/// a default message if it was constructed with a null value or is currently null.
/// </remarks>
public bool HasMessage { get; }
/// <summary>
/// Initializes an instance of <see cref="BaseCliFxException"/>.
/// </summary>
protected BaseCliFxException(string? message, bool showHelp = false)
: this(message, null, showHelp)
{
}
/// <summary>
/// Initializes an instance of <see cref="BaseCliFxException"/>.
/// </summary>
protected BaseCliFxException(string? message, Exception? innerException, bool showHelp = false)
: base(message, innerException)
{
HasMessage = string.IsNullOrWhiteSpace(message) ? false : true;
ShowHelp = showHelp;
}
}
}

View File

@@ -9,22 +9,51 @@ namespace CliFx.Exceptions
/// <summary>
/// Domain exception thrown within CliFx.
/// </summary>
public partial class CliFxException : BaseCliFxException
public partial class CliFxException : Exception
{
/// <summary>
/// Represents the default exit code assigned to exceptions in CliFx.
/// </summary>
protected const int DefaultExitCode = -100;
/// <summary>
/// Whether to show the help text after handling this exception.
/// </summary>
public bool ShowHelp { get; }
/// <summary>
/// Whether this exception was constructed with a message.
/// </summary>
/// <remarks>
/// We cannot check against the 'Message' property because it will always return
/// a default message if it was constructed with a null value or is currently null.
/// </remarks>
public bool HasMessage { get; }
/// <summary>
/// Returns an exit code associated with this exception.
/// </summary>
public int ExitCode { get; }
/// <summary>
/// Initializes an instance of <see cref="CliFxException"/>.
/// </summary>
public CliFxException(string? message, bool showHelp = false)
: base(message, showHelp)
: this(message, null, showHelp: showHelp)
{
}
/// <summary>
/// Initializes an instance of <see cref="CliFxException"/>.
/// </summary>
public CliFxException(string? message, Exception? innerException, bool showHelp = false)
: base(message, innerException, showHelp)
public CliFxException(string? message, Exception? innerException, int exitCode = DefaultExitCode, bool showHelp = false)
: base(message, innerException)
{
ExitCode = exitCode != 0
? exitCode
: throw new ArgumentException("Exit code must not be zero in order to signify failure.");
HasMessage = string.IsNullOrWhiteSpace(message) ? false : true;
ShowHelp = showHelp;
}
}
@@ -275,7 +304,7 @@ To fix this, ensure that all options have different fallback environment variabl
Can't find a command that matches the following arguments:
{string.Join(" ", input.UnboundArguments.Select(a => a.Value))}";
return new CliFxException(message.Trim());
return new CliFxException(message.Trim(), showHelp: true);
}
internal static CliFxException CannotConvertMultipleValuesToNonScalar(
@@ -290,7 +319,7 @@ Can't find a command that matches the following arguments:
{argumentDisplayText} expects a single value, but provided with multiple:
{string.Join(", ", values.Select(v => $"'{v}'"))}";
return new CliFxException(message.Trim());
return new CliFxException(message.Trim(), showHelp: true);
}
internal static CliFxException CannotConvertToType(
@@ -307,7 +336,7 @@ Can't find a command that matches the following arguments:
Can't convert value '{value ?? "<null>"}' to type '{type.FullName}' for {argumentDisplayText}.
{innerException?.Message ?? "This type is not supported."}";
return new CliFxException(message.Trim(), innerException);
return new CliFxException(message.Trim(), innerException, showHelp: true);
}
internal static CliFxException CannotConvertNonScalar(
@@ -325,7 +354,7 @@ Can't convert provided values to type '{type.FullName}' for {argumentDisplayText
Target type is not assignable from array and doesn't have a public constructor that takes an array.";
return new CliFxException(message.Trim());
return new CliFxException(message.Trim(), showHelp: true);
}
internal static CliFxException ParameterNotSet(CommandParameterSchema parameter)
@@ -333,7 +362,7 @@ Target type is not assignable from array and doesn't have a public constructor t
var message = $@"
Missing value for parameter <{parameter.DisplayName}>.";
return new CliFxException(message.Trim());
return new CliFxException(message.Trim(), showHelp: true);
}
internal static CliFxException RequiredOptionsNotSet(IReadOnlyList<CommandOptionSchema> options)
@@ -342,7 +371,7 @@ Missing value for parameter <{parameter.DisplayName}>.";
Missing values for one or more required options:
{string.Join(Environment.NewLine, options.Select(o => o.DisplayName))}";
return new CliFxException(message.Trim());
return new CliFxException(message.Trim(), showHelp: true);
}
internal static CliFxException UnrecognizedParametersProvided(IReadOnlyList<CommandUnboundArgumentInput> inputs)
@@ -351,7 +380,7 @@ Missing values for one or more required options:
Unrecognized parameters provided:
{string.Join(Environment.NewLine, inputs.Select(i => $"<{i.Value}>"))}";
return new CliFxException(message.Trim());
return new CliFxException(message.Trim(), showHelp: true);
}
internal static CliFxException UnrecognizedOptionsProvided(IReadOnlyList<CommandOptionInput> inputs)
@@ -360,7 +389,7 @@ Unrecognized parameters provided:
Unrecognized options provided:
{string.Join(Environment.NewLine, inputs.Select(i => i.DisplayAlias))}";
return new CliFxException(message.Trim());
return new CliFxException(message.Trim(), showHelp: true);
}
}
}

View File

@@ -7,25 +7,16 @@ namespace CliFx.Exceptions
/// Use this exception if you want to report an error that occured during execution of a command.
/// This exception also allows specifying exit code which will be returned to the calling process.
/// </summary>
public class CommandException : BaseCliFxException
public class CommandException : CliFxException
{
private const int DefaultExitCode = -100;
/// <summary>
/// Process exit code.
/// </summary>
public int ExitCode { get; }
/// <summary>
/// Initializes an instance of <see cref="CommandException"/>.
/// </summary>
public CommandException(string? message, Exception? innerException,
int exitCode = DefaultExitCode, bool showHelp = false)
: base(message, innerException, showHelp)
: base(message, innerException, exitCode, showHelp)
{
ExitCode = exitCode != 0
? exitCode
: throw new ArgumentException("Exit code must not be zero in order to signify failure.");
}
/// <summary>