From 544e6a92df827de693e3c2f7ff2678fa10a1bec1 Mon Sep 17 00:00:00 2001 From: Nils Andresen Date: Sun, 7 Jan 2024 00:34:02 +0100 Subject: [PATCH] Added the ITypeResolver to the ExceptionHandler (#1411) --- docs/input/cli/exceptions.md | 14 +++-- src/Spectre.Console.Cli/CommandApp.cs | 2 +- .../ConfiguratorExtensions.cs | 8 +-- .../ICommandAppSettings.cs | 4 +- .../Internal/CommandExecutor.cs | 61 +++++++++++-------- .../Configuration/CommandAppSettings.cs | 2 +- .../Unit/CommandAppTests.Exceptions.cs | 48 ++++++++++++++- 7 files changed, 98 insertions(+), 41 deletions(-) diff --git a/docs/input/cli/exceptions.md b/docs/input/cli/exceptions.md index aec5380..723ece8 100644 --- a/docs/input/cli/exceptions.md +++ b/docs/input/cli/exceptions.md @@ -53,7 +53,11 @@ Using the `SetExceptionHandler()` during configuration it is possible to handle This method comes in two flavours: One that uses the default exitCode (or `return` value) of `-1` and one where the exitCode needs to be supplied. -### Using `SetExceptionHandler(Func handler)` +The `ITypeResolver?` parameter will be null, when the exception occurs while no `ITypeResolver` is available. +(Basically the `ITypeResolver` will be set, when the exception occurs during a command execution, but not +during the parsing phase and construction of the command.) + +### Using `SetExceptionHandler(Func handler)` Using this method exceptions can be handled in a custom way. The return value of the handler is used as the exitCode for the application. @@ -71,7 +75,7 @@ namespace MyApp app.Configure(config => { - config.SetExceptionHandler(ex => + config.SetExceptionHandler((ex, resolver) => { AnsiConsole.WriteException(ex, ExceptionFormats.ShortenEverything); return -99; @@ -84,9 +88,9 @@ namespace MyApp } ``` -### Using `SetExceptionHandler(Action handler)` +### Using `SetExceptionHandler(Action handler)` -Using this method exceptions can be handled in a custom way, much the same as with the `SetExceptionHandler(Func handler)`. +Using this method exceptions can be handled in a custom way, much the same as with the `SetExceptionHandler(Func handler)`. Using the `Action` as the handler however, it is not possible (or required) to supply a return value. The exitCode for the application will be `-1`. @@ -103,7 +107,7 @@ namespace MyApp app.Configure(config => { - config.SetExceptionHandler(ex => + config.SetExceptionHandler((ex, resolver) => { AnsiConsole.WriteException(ex, ExceptionFormats.ShortenEverything); }); diff --git a/src/Spectre.Console.Cli/CommandApp.cs b/src/Spectre.Console.Cli/CommandApp.cs index 07077f2..1c05deb 100644 --- a/src/Spectre.Console.Cli/CommandApp.cs +++ b/src/Spectre.Console.Cli/CommandApp.cs @@ -102,7 +102,7 @@ public sealed class CommandApp : ICommandApp if (_configurator.Settings.ExceptionHandler != null) { - return _configurator.Settings.ExceptionHandler(ex); + return _configurator.Settings.ExceptionHandler(ex, null); } // Render the exception. diff --git a/src/Spectre.Console.Cli/ConfiguratorExtensions.cs b/src/Spectre.Console.Cli/ConfiguratorExtensions.cs index 985e208..c0f1cd8 100644 --- a/src/Spectre.Console.Cli/ConfiguratorExtensions.cs +++ b/src/Spectre.Console.Cli/ConfiguratorExtensions.cs @@ -367,11 +367,11 @@ public static class ConfiguratorExtensions /// The configurator. /// The Action that handles the exception. /// A configurator that can be used to configure the application further. - public static IConfigurator SetExceptionHandler(this IConfigurator configurator, Action exceptionHandler) + public static IConfigurator SetExceptionHandler(this IConfigurator configurator, Action exceptionHandler) { - return configurator.SetExceptionHandler(ex => + return configurator.SetExceptionHandler((ex, resolver) => { - exceptionHandler(ex); + exceptionHandler(ex, resolver); return -1; }); } @@ -382,7 +382,7 @@ public static class ConfiguratorExtensions /// The configurator. /// The Action that handles the exception. /// A configurator that can be used to configure the application further. - public static IConfigurator SetExceptionHandler(this IConfigurator configurator, Func? exceptionHandler) + public static IConfigurator SetExceptionHandler(this IConfigurator configurator, Func? exceptionHandler) { if (configurator == null) { diff --git a/src/Spectre.Console.Cli/ICommandAppSettings.cs b/src/Spectre.Console.Cli/ICommandAppSettings.cs index c66478c..aaed454 100644 --- a/src/Spectre.Console.Cli/ICommandAppSettings.cs +++ b/src/Spectre.Console.Cli/ICommandAppSettings.cs @@ -91,6 +91,8 @@ public interface ICommandAppSettings /// /// Gets or sets a handler for Exceptions. /// This handler will not be called, if is set to true. + /// The argument will only be not-null, when the exception occurs during execution of + /// a command. I.e. only when the resolver is available. /// - public Func? ExceptionHandler { get; set; } + public Func? ExceptionHandler { get; set; } } \ No newline at end of file diff --git a/src/Spectre.Console.Cli/Internal/CommandExecutor.cs b/src/Spectre.Console.Cli/Internal/CommandExecutor.cs index 1f57a66..4bc1418 100644 --- a/src/Spectre.Console.Cli/Internal/CommandExecutor.cs +++ b/src/Spectre.Console.Cli/Internal/CommandExecutor.cs @@ -128,37 +128,44 @@ internal sealed class CommandExecutor ITypeResolver resolver, IConfiguration configuration) { - // Bind the command tree against the settings. - var settings = CommandBinder.Bind(tree, leaf.Command.SettingsType, resolver); - var interceptors = - ((IEnumerable?)resolver.Resolve(typeof(IEnumerable)) - ?? Array.Empty()).ToList(); + try + { + // Bind the command tree against the settings. + var settings = CommandBinder.Bind(tree, leaf.Command.SettingsType, resolver); + var interceptors = + ((IEnumerable?)resolver.Resolve(typeof(IEnumerable)) + ?? Array.Empty()).ToList(); #pragma warning disable CS0618 // Type or member is obsolete - if (configuration.Settings.Interceptor != null) - { - interceptors.Add(configuration.Settings.Interceptor); - } + if (configuration.Settings.Interceptor != null) + { + interceptors.Add(configuration.Settings.Interceptor); + } #pragma warning restore CS0618 // Type or member is obsolete - foreach (var interceptor in interceptors) - { - interceptor.Intercept(context, settings); - } + foreach (var interceptor in interceptors) + { + interceptor.Intercept(context, settings); + } - // Create and validate the command. - var command = leaf.CreateCommand(resolver); - var validationResult = command.Validate(context, settings); - if (!validationResult.Successful) - { - throw CommandRuntimeException.ValidationFailed(validationResult); - } + // Create and validate the command. + var command = leaf.CreateCommand(resolver); + var validationResult = command.Validate(context, settings); + if (!validationResult.Successful) + { + throw CommandRuntimeException.ValidationFailed(validationResult); + } - // Execute the command. - var result = await command.Execute(context, settings); - foreach (var interceptor in interceptors) - { - interceptor.InterceptResult(context, settings, ref result); - } + // Execute the command. + var result = await command.Execute(context, settings); + foreach (var interceptor in interceptors) + { + interceptor.InterceptResult(context, settings, ref result); + } - return result; + return result; + } + catch (Exception ex) when (configuration.Settings is { ExceptionHandler: not null, PropagateExceptions: false }) + { + return configuration.Settings.ExceptionHandler(ex, resolver); + } } } \ No newline at end of file diff --git a/src/Spectre.Console.Cli/Internal/Configuration/CommandAppSettings.cs b/src/Spectre.Console.Cli/Internal/Configuration/CommandAppSettings.cs index f05c50c..c14d6a9 100644 --- a/src/Spectre.Console.Cli/Internal/Configuration/CommandAppSettings.cs +++ b/src/Spectre.Console.Cli/Internal/Configuration/CommandAppSettings.cs @@ -21,7 +21,7 @@ internal sealed class CommandAppSettings : ICommandAppSettings public ParsingMode ParsingMode => StrictParsing ? ParsingMode.Strict : ParsingMode.Relaxed; - public Func? ExceptionHandler { get; set; } + public Func? ExceptionHandler { get; set; } public CommandAppSettings(ITypeRegistrar registrar) { diff --git a/test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.Exceptions.cs b/test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.Exceptions.cs index 61c326a..77b4853 100644 --- a/test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.Exceptions.cs +++ b/test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.Exceptions.cs @@ -51,7 +51,7 @@ public sealed partial class CommandAppTests app.Configure(config => { config.AddCommand("throw"); - config.SetExceptionHandler(_ => + config.SetExceptionHandler((_, _) => { exceptionHandled = true; }); @@ -74,7 +74,7 @@ public sealed partial class CommandAppTests app.Configure(config => { config.AddCommand("throw"); - config.SetExceptionHandler(_ => + config.SetExceptionHandler((_, _) => { exceptionHandled = true; return -99; @@ -88,5 +88,49 @@ public sealed partial class CommandAppTests result.ExitCode.ShouldBe(-99); exceptionHandled.ShouldBeTrue(); } + + [Fact] + public void Should_Have_Resolver_Set_When_Exception_Occurs_In_Command_Execution() + { + // Given + ITypeResolver resolver = null; + var app = new CommandAppTester(); + app.Configure(config => + { + config.AddCommand("throw"); + config.SetExceptionHandler((_, r) => + { + resolver = r; + }); + }); + + // When + app.Run(new[] { "throw" }); + + // Then + resolver.ShouldNotBeNull(); + } + + [Fact] + public void Should_Not_Have_Resolver_Set_When_Exception_Occurs_Before_Command_Execution() + { + // Given + ITypeResolver resolver = null; + var app = new CommandAppTester(); + app.Configure(config => + { + config.AddCommand("Лайка"); + config.SetExceptionHandler((_, r) => + { + resolver = r; + }); + }); + + // When + app.Run(new[] { "throw", "on", "arg", "parsing" }); + + // Then + resolver.ShouldBeNull(); + } } }