From a1358014ad627b9f1cbb7eb5e66744ec15ef9fe5 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Thu, 17 Oct 2024 18:52:32 -0500 Subject: [PATCH] refactor: Consistent exception handling in all command classes - Consistently use ExitStatus enum. - Where appropriate, do not handle exceptions in isolation in the command class itself. Rather, let exceptions through to the entrypoint so the centralized exception handler can deal with them. --- .../inspectionProfiles/Project_Default.xml | 1 + .../Console/Commands/ConfigCreateCommand.cs | 6 ++--- .../Commands/ConfigListLocalCommand.cs | 19 ++++----------- .../Commands/ConfigListTemplatesCommand.cs | 22 ++++-------------- .../Commands/DeleteCustomFormatsCommand.cs | 23 +++---------------- .../Commands/ListCustomFormatsCommand.cs | 3 ++- .../Commands/ListMediaNamingCommand.cs | 3 ++- .../Console/Commands/ListQualitiesCommand.cs | 3 ++- .../Console/Commands/MigrateCommand.cs | 6 ++--- src/Recyclarr.Cli/Program.cs | 6 ++++- 10 files changed, 31 insertions(+), 61 deletions(-) diff --git a/.idea/.idea.Recyclarr/.idea/inspectionProfiles/Project_Default.xml b/.idea/.idea.Recyclarr/.idea/inspectionProfiles/Project_Default.xml index 7e296f57..a04db514 100644 --- a/.idea/.idea.Recyclarr/.idea/inspectionProfiles/Project_Default.xml +++ b/.idea/.idea.Recyclarr/.idea/inspectionProfiles/Project_Default.xml @@ -2,5 +2,6 @@ \ No newline at end of file diff --git a/src/Recyclarr.Cli/Console/Commands/ConfigCreateCommand.cs b/src/Recyclarr.Cli/Console/Commands/ConfigCreateCommand.cs index dc0c405b..923b0767 100644 --- a/src/Recyclarr.Cli/Console/Commands/ConfigCreateCommand.cs +++ b/src/Recyclarr.Cli/Console/Commands/ConfigCreateCommand.cs @@ -1,6 +1,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using Recyclarr.Cli.Console.Settings; +using Recyclarr.Cli.Processors; using Recyclarr.Cli.Processors.Config; using Recyclarr.Repo; using Spectre.Console.Cli; @@ -43,16 +44,15 @@ public class ConfigCreateCommand(ILogger log, IConfigCreationProcessor processor { await repoUpdater.UpdateAllRepositories(settings.CancellationToken); processor.Process(settings); + return (int) ExitStatus.Succeeded; } catch (FileExistsException e) { log.Error(e, "The file {ConfigFile} already exists. Please choose another path or " + "delete/move the existing file and run this command again", e.AttemptedPath); - - return 1; } - return 0; + return (int) ExitStatus.Failed; } } diff --git a/src/Recyclarr.Cli/Console/Commands/ConfigListLocalCommand.cs b/src/Recyclarr.Cli/Console/Commands/ConfigListLocalCommand.cs index c71b8bad..791614f4 100644 --- a/src/Recyclarr.Cli/Console/Commands/ConfigListLocalCommand.cs +++ b/src/Recyclarr.Cli/Console/Commands/ConfigListLocalCommand.cs @@ -1,7 +1,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; +using Recyclarr.Cli.Processors; using Recyclarr.Cli.Processors.Config; -using Recyclarr.Config.Parsing.ErrorHandling; using Recyclarr.Repo; using Spectre.Console.Cli; @@ -9,7 +9,7 @@ namespace Recyclarr.Cli.Console.Commands; [UsedImplicitly] [Description("List local configuration files.")] -public class ConfigListLocalCommand(ILogger log, ConfigListLocalProcessor processor, IMultiRepoUpdater repoUpdater) +public class ConfigListLocalCommand(ConfigListLocalProcessor processor, IMultiRepoUpdater repoUpdater) : AsyncCommand { [SuppressMessage("Design", "CA1034:Nested types should not be visible")] @@ -17,17 +17,8 @@ public class ConfigListLocalCommand(ILogger log, ConfigListLocalProcessor proces public override async Task ExecuteAsync(CommandContext context, CliSettings settings) { - try - { - await repoUpdater.UpdateAllRepositories(settings.CancellationToken); - processor.Process(); - return 0; - } - catch (NoConfigurationFilesException e) - { - log.Error(e, "Unable to list local config files"); - } - - return 1; + await repoUpdater.UpdateAllRepositories(settings.CancellationToken); + processor.Process(); + return (int) ExitStatus.Succeeded; } } diff --git a/src/Recyclarr.Cli/Console/Commands/ConfigListTemplatesCommand.cs b/src/Recyclarr.Cli/Console/Commands/ConfigListTemplatesCommand.cs index 2a8266b5..58e84a7a 100644 --- a/src/Recyclarr.Cli/Console/Commands/ConfigListTemplatesCommand.cs +++ b/src/Recyclarr.Cli/Console/Commands/ConfigListTemplatesCommand.cs @@ -1,7 +1,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; +using Recyclarr.Cli.Processors; using Recyclarr.Cli.Processors.Config; -using Recyclarr.Config.Parsing.ErrorHandling; using Recyclarr.Repo; using Spectre.Console.Cli; @@ -9,10 +9,7 @@ namespace Recyclarr.Cli.Console.Commands; [UsedImplicitly] [Description("List local configuration files.")] -public class ConfigListTemplatesCommand( - ILogger log, - ConfigListTemplateProcessor processor, - IMultiRepoUpdater repoUpdater) +public class ConfigListTemplatesCommand(ConfigListTemplateProcessor processor, IMultiRepoUpdater repoUpdater) : AsyncCommand { [SuppressMessage("Design", "CA1034:Nested types should not be visible")] @@ -27,18 +24,9 @@ public class ConfigListTemplatesCommand( public override async Task ExecuteAsync(CommandContext context, CliSettings settings) { - try - { - await repoUpdater.UpdateAllRepositories(settings.CancellationToken); - processor.Process(settings); - return 0; - } - catch (NoConfigurationFilesException e) - { - log.Error(e, "Unable to list template files"); - } - - return 1; + await repoUpdater.UpdateAllRepositories(settings.CancellationToken); + processor.Process(settings); + return (int) ExitStatus.Succeeded; } } diff --git a/src/Recyclarr.Cli/Console/Commands/DeleteCustomFormatsCommand.cs b/src/Recyclarr.Cli/Console/Commands/DeleteCustomFormatsCommand.cs index e1be7afb..181de9a7 100644 --- a/src/Recyclarr.Cli/Console/Commands/DeleteCustomFormatsCommand.cs +++ b/src/Recyclarr.Cli/Console/Commands/DeleteCustomFormatsCommand.cs @@ -3,16 +3,13 @@ using System.Diagnostics.CodeAnalysis; using Recyclarr.Cli.Console.Settings; using Recyclarr.Cli.Processors; using Recyclarr.Cli.Processors.Delete; -using Recyclarr.Cli.Processors.ErrorHandling; using Spectre.Console.Cli; namespace Recyclarr.Cli.Console.Commands; -[Description("Delete things from services like Radarr & Sonarr")] +[Description("Delete things from services like Radarr and Sonarr")] [UsedImplicitly] -public class DeleteCustomFormatsCommand( - IDeleteCustomFormatsProcessor processor, - ConsoleExceptionHandler exceptionHandler) +public class DeleteCustomFormatsCommand(IDeleteCustomFormatsProcessor processor) : AsyncCommand { [UsedImplicitly] @@ -46,21 +43,7 @@ public class DeleteCustomFormatsCommand( [SuppressMessage("Design", "CA1031:Do not catch general exception types")] public override async Task ExecuteAsync(CommandContext context, CliSettings settings) { - try - { - await processor.Process(settings, settings.CancellationToken); - } - catch (Exception e) - { - if (!await exceptionHandler.HandleException(e)) - { - // This means we didn't handle the exception; rethrow it. - throw; - } - - return (int) ExitStatus.Failed; - } - + await processor.Process(settings, settings.CancellationToken); return (int) ExitStatus.Succeeded; } } diff --git a/src/Recyclarr.Cli/Console/Commands/ListCustomFormatsCommand.cs b/src/Recyclarr.Cli/Console/Commands/ListCustomFormatsCommand.cs index 6427e2ee..28bf9790 100644 --- a/src/Recyclarr.Cli/Console/Commands/ListCustomFormatsCommand.cs +++ b/src/Recyclarr.Cli/Console/Commands/ListCustomFormatsCommand.cs @@ -3,6 +3,7 @@ using System.Diagnostics.CodeAnalysis; using Recyclarr.Cli.Console.Helpers; using Recyclarr.Cli.Console.Settings; using Recyclarr.Cli.Pipelines.CustomFormat; +using Recyclarr.Cli.Processors; using Recyclarr.Repo; using Recyclarr.TrashGuide; using Spectre.Console.Cli; @@ -38,6 +39,6 @@ public class ListCustomFormatsCommand(CustomFormatDataLister lister, IMultiRepoU { await repoUpdater.UpdateAllRepositories(settings.CancellationToken, settings.Raw); lister.List(settings); - return 0; + return (int) ExitStatus.Succeeded; } } diff --git a/src/Recyclarr.Cli/Console/Commands/ListMediaNamingCommand.cs b/src/Recyclarr.Cli/Console/Commands/ListMediaNamingCommand.cs index de977715..fb9883ab 100644 --- a/src/Recyclarr.Cli/Console/Commands/ListMediaNamingCommand.cs +++ b/src/Recyclarr.Cli/Console/Commands/ListMediaNamingCommand.cs @@ -2,6 +2,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using Recyclarr.Cli.Console.Helpers; using Recyclarr.Cli.Pipelines.MediaNaming; +using Recyclarr.Cli.Processors; using Recyclarr.Repo; using Recyclarr.TrashGuide; using Spectre.Console.Cli; @@ -27,6 +28,6 @@ public class ListMediaNamingCommand(MediaNamingDataLister lister, IMultiRepoUpda { await repoUpdater.UpdateAllRepositories(settings.CancellationToken); lister.ListNaming(settings.Service); - return 0; + return (int) ExitStatus.Succeeded; } } diff --git a/src/Recyclarr.Cli/Console/Commands/ListQualitiesCommand.cs b/src/Recyclarr.Cli/Console/Commands/ListQualitiesCommand.cs index 36a4179d..739269f1 100644 --- a/src/Recyclarr.Cli/Console/Commands/ListQualitiesCommand.cs +++ b/src/Recyclarr.Cli/Console/Commands/ListQualitiesCommand.cs @@ -2,6 +2,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using Recyclarr.Cli.Console.Helpers; using Recyclarr.Cli.Pipelines.QualitySize; +using Recyclarr.Cli.Processors; using Recyclarr.Repo; using Recyclarr.TrashGuide; using Spectre.Console.Cli; @@ -28,6 +29,6 @@ public class ListQualitiesCommand(QualitySizeDataLister lister, IMultiRepoUpdate { await repoUpdater.UpdateAllRepositories(settings.CancellationToken); lister.ListQualities(settings.Service); - return 0; + return (int) ExitStatus.Succeeded; } } diff --git a/src/Recyclarr.Cli/Console/Commands/MigrateCommand.cs b/src/Recyclarr.Cli/Console/Commands/MigrateCommand.cs index af934cb1..15b50f4b 100644 --- a/src/Recyclarr.Cli/Console/Commands/MigrateCommand.cs +++ b/src/Recyclarr.Cli/Console/Commands/MigrateCommand.cs @@ -2,6 +2,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using System.Text; using Recyclarr.Cli.Migration; +using Recyclarr.Cli.Processors; using Spectre.Console; using Spectre.Console.Cli; @@ -25,6 +26,7 @@ public class MigrateCommand( { migration.PerformAllMigrationSteps(settings.Debug); console.WriteLine("All migration steps completed"); + return (int) ExitStatus.Succeeded; } catch (MigrationException e) { @@ -44,14 +46,12 @@ public class MigrateCommand( } console.Write(msg.ToString()); - return 1; } catch (RequiredMigrationException ex) { console.WriteLine($"ERROR: {ex.Message}"); - return 1; } - return 0; + return (int) ExitStatus.Failed; } } diff --git a/src/Recyclarr.Cli/Program.cs b/src/Recyclarr.Cli/Program.cs index 86df65e4..dfb50ace 100644 --- a/src/Recyclarr.Cli/Program.cs +++ b/src/Recyclarr.Cli/Program.cs @@ -42,7 +42,11 @@ internal static class Program { var log = scope.Resolve(); var exceptionHandler = new ConsoleExceptionHandler(log); - await exceptionHandler.HandleException(e); + if (!await exceptionHandler.HandleException(e)) + { + log.Error(e, "Exiting due to fatal error"); + } + return (int) ExitStatus.Failed; } }