diff --git a/CliFx.Tests/ErrorReportingSpecs.Commands.cs b/CliFx.Tests/ErrorReportingSpecs.Commands.cs index d8e247c..0b06b0b 100644 --- a/CliFx.Tests/ErrorReportingSpecs.Commands.cs +++ b/CliFx.Tests/ErrorReportingSpecs.Commands.cs @@ -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(); + } + } } } \ No newline at end of file diff --git a/CliFx.Tests/ErrorReportingSpecs.cs b/CliFx.Tests/ErrorReportingSpecs.cs index 329b8c9..eb88bff 100644 --- a/CliFx.Tests/ErrorReportingSpecs.cs +++ b/CliFx.Tests/ErrorReportingSpecs.cs @@ -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()); + + 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."); } } } \ No newline at end of file diff --git a/CliFx/CliApplication.cs b/CliFx/CliApplication.cs index e892e26..26ee3a5 100644 --- a/CliFx/CliApplication.cs +++ b/CliFx/CliApplication.cs @@ -147,13 +147,13 @@ namespace CliFx /// Handle s differently from the rest because we want to /// display it different based on whether we are showing the help text or not. /// - private int HandleCommandException(IReadOnlyList commandLineArguments, CommandException commandException) + private int HandleCliFxException(IReadOnlyList 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; - } - - /// - /// Handles s by printing its error message if it has a value. - /// Otherwise, it prints the full stack trace from the exception. - /// - /// The exception to handle. - /// The exception's HResult. - 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; } /// @@ -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. diff --git a/CliFx/Exceptions/BaseCliFxException.cs b/CliFx/Exceptions/BaseCliFxException.cs deleted file mode 100644 index 1b9b320..0000000 --- a/CliFx/Exceptions/BaseCliFxException.cs +++ /dev/null @@ -1,43 +0,0 @@ -using System; - -namespace CliFx.Exceptions -{ - /// - /// Provides the base functionality for exceptions thrown within CliFx - /// or from one of its commands. - /// - public abstract class BaseCliFxException : Exception - { - /// - /// Whether to show the help text after handling this exception. - /// - public bool ShowHelp { get; } - - /// - /// Whether this exception was constructed with a message. - /// - /// - /// 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. - /// - public bool HasMessage { get; } - - /// - /// Initializes an instance of . - /// - protected BaseCliFxException(string? message, bool showHelp = false) - : this(message, null, showHelp) - { - } - - /// - /// Initializes an instance of . - /// - protected BaseCliFxException(string? message, Exception? innerException, bool showHelp = false) - : base(message, innerException) - { - HasMessage = string.IsNullOrWhiteSpace(message) ? false : true; - ShowHelp = showHelp; - } - } -} \ No newline at end of file diff --git a/CliFx/Exceptions/CliFxException.cs b/CliFx/Exceptions/CliFxException.cs index ca36ca5..c8f5e07 100644 --- a/CliFx/Exceptions/CliFxException.cs +++ b/CliFx/Exceptions/CliFxException.cs @@ -9,22 +9,51 @@ namespace CliFx.Exceptions /// /// Domain exception thrown within CliFx. /// - public partial class CliFxException : BaseCliFxException + public partial class CliFxException : Exception { + /// + /// Represents the default exit code assigned to exceptions in CliFx. + /// + protected const int DefaultExitCode = -100; + + /// + /// Whether to show the help text after handling this exception. + /// + public bool ShowHelp { get; } + + /// + /// Whether this exception was constructed with a message. + /// + /// + /// 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. + /// + public bool HasMessage { get; } + + /// + /// Returns an exit code associated with this exception. + /// + public int ExitCode { get; } + /// /// Initializes an instance of . /// public CliFxException(string? message, bool showHelp = false) - : base(message, showHelp) + : this(message, null, showHelp: showHelp) { } /// /// Initializes an instance of . /// - 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 ?? ""}' 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 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 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 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); } } } \ No newline at end of file diff --git a/CliFx/Exceptions/CommandException.cs b/CliFx/Exceptions/CommandException.cs index 8d9b4fb..2ef5159 100644 --- a/CliFx/Exceptions/CommandException.cs +++ b/CliFx/Exceptions/CommandException.cs @@ -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. /// - public class CommandException : BaseCliFxException + public class CommandException : CliFxException { - private const int DefaultExitCode = -100; - - /// - /// Process exit code. - /// - public int ExitCode { get; } - /// /// Initializes an instance of . /// 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."); + } ///