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 | 59 +++++++++++-------- .../Configuration/CommandAppSettings.cs | 2 +- .../Unit/CommandAppTests.Exceptions.cs | 48 ++++++++++++++- 7 files changed, 97 insertions(+), 40 deletions(-) diff --git a/docs/input/cli/exceptions.md b/docs/input/cli/exceptions.md index aec53803c..723ece859 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 07077f21a..1c05deb09 100644 --- a/src/Spectre.Console.Cli/CommandApp.cs +++ b/src/Spectre.Console.Cli/CommandApp.cs @@ -102,7 +102,7 @@ public async Task RunAsync(IEnumerable args) 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 985e208d3..c0f1cd814 100644 --- a/src/Spectre.Console.Cli/ConfiguratorExtensions.cs +++ b/src/Spectre.Console.Cli/ConfiguratorExtensions.cs @@ -367,11 +367,11 @@ public static ICommandConfigurator AddAsyncDelegate( /// 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 IConfigurator SetExceptionHandler(this IConfigurator configurator, /// 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 c66478ce2..aaed45496 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 1f57a66e1..4bc1418a5 100644 --- a/src/Spectre.Console.Cli/Internal/CommandExecutor.cs +++ b/src/Spectre.Console.Cli/Internal/CommandExecutor.cs @@ -128,37 +128,44 @@ private static async Task Execute( 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(); -#pragma warning disable CS0618 // Type or member is obsolete - if (configuration.Settings.Interceptor != null) + try { - interceptors.Add(configuration.Settings.Interceptor); - } + // 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); + } #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) + // Execute the command. + var result = await command.Execute(context, settings); + foreach (var interceptor in interceptors) + { + interceptor.InterceptResult(context, settings, ref result); + } + + return result; + } + catch (Exception ex) when (configuration.Settings is { ExceptionHandler: not null, PropagateExceptions: false }) { - interceptor.InterceptResult(context, settings, ref result); + return configuration.Settings.ExceptionHandler(ex, resolver); } - - return result; } } \ 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 f05c50c36..c14d6a901 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 61c326a99..77b48533d 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 void Should_Handle_Exceptions_If_ExceptionHandler_Is_Set_Using_Action() app.Configure(config => { config.AddCommand("throw"); - config.SetExceptionHandler(_ => + config.SetExceptionHandler((_, _) => { exceptionHandled = true; }); @@ -74,7 +74,7 @@ public void Should_Handle_Exceptions_If_ExceptionHandler_Is_Set_Using_Function() app.Configure(config => { config.AddCommand("throw"); - config.SetExceptionHandler(_ => + config.SetExceptionHandler((_, _) => { exceptionHandled = true; return -99; @@ -88,5 +88,49 @@ public void Should_Handle_Exceptions_If_ExceptionHandler_Is_Set_Using_Function() 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(); + } } }