refactor: Improve migration step diagnostics

- Logging is more consistent and detailed
- Include remediation steps to help user fix migration issues

src/Recyclarr.Tests/Migration/MigrationExecutorTest.cs

@@ -38,7 +38,7 @@ public class MigrationExecutorTest
         executor.PerformAllMigrationSteps();
 
         step.Received().CheckIfNeeded();
-        step.DidNotReceive().Execute(Arg.Any<ILogger>());
+        step.DidNotReceive().Execute();
     }
 
     [Test]
@@ -52,7 +52,7 @@ public class MigrationExecutorTest
         executor.PerformAllMigrationSteps();
 
         step.Received().CheckIfNeeded();
-        step.Received().Execute(Arg.Any<ILogger>());
+        step.Received().Execute();
     }
 
     [Test]
@@ -88,11 +88,11 @@ public class MigrationExecutorTest
         var executor = new MigrationExecutor(new[] {step}, Substitute.For<ILogger>());
 
         step.CheckIfNeeded().Returns(true);
-        step.When(x => x.Execute(Arg.Any<ILogger>())).Throw(new ArgumentException("test message"));
+        step.When(x => x.Execute()).Throw(new ArgumentException("test message"));
 
         var act = () => executor.PerformAllMigrationSteps();
 
-        act.Should().Throw<MigrationException>().Which.FailureReason.Should().Be("test message");
+        act.Should().Throw<MigrationException>().Which.OriginalException.Message.Should().Be("test message");
     }
 
     [Test]
@@ -100,10 +100,10 @@ public class MigrationExecutorTest
     {
         var step = Substitute.For<IMigrationStep>();
         var executor = new MigrationExecutor(new[] {step}, Substitute.For<ILogger>());
-        var exception = new MigrationException("a", "b");
+        var exception = new MigrationException(new Exception(), "a", new[] {"b"});
 
         step.CheckIfNeeded().Returns(true);
-        step.When(x => x.Execute(Arg.Any<ILogger>())).Throw(exception);
+        step.When(x => x.Execute()).Throw(exception);
 
         var act = () => executor.PerformAllMigrationSteps();
 

src/Recyclarr.Tests/Migration/Steps/MigrateTrashUpdaterAppDataDirTest.cs

@@ -2,11 +2,8 @@ using System.IO.Abstractions.TestingHelpers;
 using System.Text.RegularExpressions;
 using AutoFixture.NUnit3;
 using FluentAssertions;
-using NSubstitute;
 using NUnit.Framework;
-using Recyclarr.Migration;
 using Recyclarr.Migration.Steps;
-using Serilog;
 using TestLibrary.AutoFixture;
 
 namespace Recyclarr.Tests.Migration.Steps;
@@ -39,11 +36,12 @@ public class MigrateTrashUpdaterAppDataDirTest
         [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs,
         MigrateTrashUpdaterAppDataDir sut)
     {
+        fs.AddDirectory(Path.Combine(BasePath, "trash-updater"));
         fs.AddDirectory(Path.Combine(BasePath, "recyclarr"));
 
-        var act = () => sut.Execute(Substitute.For<ILogger>());
+        var act = () => sut.Execute();
 
-        act.Should().Throw<MigrationException>().WithMessage("*already exist*");
+        act.Should().Throw<IOException>();
     }
 
     [Test, AutoMockData]
@@ -53,7 +51,7 @@ public class MigrateTrashUpdaterAppDataDirTest
     {
         fs.AddDirectory(Path.Combine(BasePath, "trash-updater"));
 
-        sut.Execute(Substitute.For<ILogger>());
+        sut.Execute();
 
         fs.AllDirectories.Should().ContainSingle(x => Regex.IsMatch(x, @"[/\\]recyclarr$"));
     }

src/Recyclarr.Tests/Migration/Steps/MigrateTrashYmlTest.cs

@@ -2,11 +2,8 @@ using System.IO.Abstractions.TestingHelpers;
 using System.Text.RegularExpressions;
 using AutoFixture.NUnit3;
 using FluentAssertions;
-using NSubstitute;
 using NUnit.Framework;
-using Recyclarr.Migration;
 using Recyclarr.Migration.Steps;
-using Serilog;
 using TestLibrary.AutoFixture;
 
 namespace Recyclarr.Tests.Migration.Steps;
@@ -41,9 +38,9 @@ public class MigrateTrashYmlTest
     {
         fs.AddFile(Path.Combine(BasePath, "recyclarr.yml"), MockFileData.NullObject);
 
-        var act = () => sut.Execute(Substitute.For<ILogger>());
+        var act = () => sut.Execute();
 
-        act.Should().Throw<MigrationException>().WithMessage("*already exist*");
+        act.Should().Throw<IOException>();
     }
 
     [Test, AutoMockData]
@@ -54,7 +51,7 @@ public class MigrateTrashYmlTest
         const string expectedData = "fake contents";
         fs.AddFile(Path.Combine(BasePath, "trash.yml"), expectedData);
 
-        sut.Execute(Substitute.For<ILogger>());
+        sut.Execute();
 
         fs.AllFiles.Should().ContainSingle(x => Regex.IsMatch(x, @"[/\\]recyclarr\.yml$"));
     }

src/Recyclarr/Migration/IMigrationStep.cs

@@ -1,11 +1,10 @@
-using Serilog;
-
 namespace Recyclarr.Migration;
 
 public interface IMigrationStep
 {
     int Order { get; }
     string Description { get; }
+    IReadOnlyCollection<string> Remediation { get; }
     bool CheckIfNeeded();
-    void Execute(ILogger log);
+    void Execute();
 }

src/Recyclarr/Migration/MigrationException.cs

@@ -2,15 +2,17 @@ namespace Recyclarr.Migration;
 
 public class MigrationException : Exception
 {
-    public MigrationException(string operationDescription, string failureReason)
+    public MigrationException(
+        Exception originalException,
+        string operationDescription,
+        IReadOnlyCollection<string> remediation)
     {
         OperationDescription = operationDescription;
-        FailureReason = failureReason;
+        OriginalException = originalException;
+        Remediation = remediation;
     }
 
+    public Exception OriginalException { get; }
     public string OperationDescription { get; }
-    public string FailureReason { get; }
+    public IReadOnlyCollection<string> Remediation { get; }
-
-    public override string Message =>
-        $"Fatal exception during migration step [Desc: {OperationDescription}] [Reason: {FailureReason}]";
 }

src/Recyclarr/Migration/MigrationExecutor.cs

@@ -30,11 +30,11 @@ public class MigrationExecutor : IMigrationExecutor
 
             try
             {
-                step.Execute(_log);
+                step.Execute();
             }
             catch (Exception e) when (e is not MigrationException)
             {
-                throw new MigrationException(step.Description, e.Message);
+                throw new MigrationException(e, step.Description, step.Remediation);
             }
         }
     }

src/Recyclarr/Migration/Steps/MigrateTrashUpdaterAppDataDir.cs

@@ -1,6 +1,5 @@
 using System.IO.Abstractions;
 using JetBrains.Annotations;
-using Serilog;
 
 namespace Recyclarr.Migration.Steps;
 
@@ -22,28 +21,27 @@ public class MigrateTrashUpdaterAppDataDir : IMigrationStep
     private readonly string _newPath =
         Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "recyclarr");
 
+    public int Order => 20;
+    public string Description { get; }
+    public IReadOnlyCollection<string> Remediation { get; }
+
     public MigrateTrashUpdaterAppDataDir(IFileSystem fileSystem)
     {
         _fileSystem = fileSystem;
-    }
+        Remediation = new[]
-
+        {
-    public int Order => 20;
+            $"Check if `{_newPath}` already exists. If so, manually copy settings you want and then delete `{_oldPath}` to fix the error.",
+            $"Ensure Recyclarr has permission to recursively delete {_oldPath}",
+            $"Ensure Recyclarr has permission to create {_newPath}"
+        };
 
-    public string Description => "Rename app data directory from 'trash-updater' to 'recyclarr'";
+        Description = $"Rename app data directory from `{_oldPath}` to `{_newPath}`";
+    }
 
     public bool CheckIfNeeded() => _fileSystem.Directory.Exists(_oldPath);
 
-    public void Execute(ILogger log)
+    public void Execute()
     {
-        try
+        _fileSystem.Directory.Move(_oldPath, _newPath);
-        {
-            _fileSystem.Directory.Move(_oldPath, _newPath);
-            log.Information("Migration: App data directory renamed from {Old} to {New}", _oldPath, _newPath);
-        }
-        catch (IOException)
-        {
-            throw new MigrationException(Description,
-                $"Unable to move due to IO Exception (does the '${_newPath}' directory already exist?)");
-        }
     }
 }

src/Recyclarr/Migration/Steps/MigrateTrashYml.cs

@@ -1,6 +1,5 @@
 using System.IO.Abstractions;
 using JetBrains.Annotations;
-using Serilog;
 
 namespace Recyclarr.Migration.Steps;
 
@@ -19,29 +18,27 @@ public class MigrateTrashYml : IMigrationStep
     // Do not use AppPaths class here since that may change yet again in the future and break this migration step.
     private readonly string _newConfigPath = Path.Combine(AppContext.BaseDirectory, "recyclarr.yml");
 
+    public int Order => 10;
+    public string Description { get; }
+    public IReadOnlyCollection<string> Remediation { get; }
+
     public MigrateTrashYml(IFileSystem fileSystem)
     {
         _fileSystem = fileSystem;
-    }
+        Remediation = new[]
-
+        {
-    public int Order => 10;
+            $"Check if `{_newConfigPath}` already exists. If so, manually copy the data you want and then delete `{_oldConfigPath}` to fix the error.",
+            $"Ensure Recyclarr has permission to delete {_oldConfigPath}",
+            $"Ensure Recyclarr has permission to create {_newConfigPath}"
+        };
 
-    public string Description => "Migration from 'trash.yml' to 'recyclarr.yml'";
+        Description = $"Migration from `{_oldConfigPath}` to `{_newConfigPath}`";
+    }
 
     public bool CheckIfNeeded() => _fileSystem.File.Exists(_oldConfigPath);
 
-    public void Execute(ILogger log)
+    public void Execute()
     {
-        try
+        _fileSystem.File.Move(_oldConfigPath, _newConfigPath);
-        {
-            _fileSystem.File.Move(_oldConfigPath, _newConfigPath);
-            log.Information("Migration: Default configuration renamed from {Old} to {New}", _oldConfigPath,
-                _newConfigPath);
-        }
-        catch (IOException)
-        {
-            throw new MigrationException(Description,
-                "Unable to move due to IO Exception (does 'recyclarr.yml' already exist next to the executable?)");
-        }
     }
 }

src/Recyclarr/Program.cs

@@ -18,15 +18,39 @@ internal static class Program
     {
         _container = CompositionRoot.Setup();
 
-        var migration = _container.Resolve<IMigrationExecutor>();
+        var console = _container.Resolve<IConsole>();
-        migration.PerformAllMigrationSteps();
+
+        try
+        {
+            var migration = _container.Resolve<IMigrationExecutor>();
+            migration.PerformAllMigrationSteps();
+        }
+        catch (MigrationException e)
+        {
+            var msg = new StringBuilder();
+            msg.AppendLine("Fatal exception during migration step. Details are below.\n");
+            msg.AppendLine($"Step That Failed:  {e.OperationDescription}");
+            msg.AppendLine($"Failure Reason:    {e.OriginalException.Message}");
+
+            if (e.Remediation.Any())
+            {
+                msg.AppendLine("\nPossible remediation steps:");
+                foreach (var remedy in e.Remediation)
+                {
+                    msg.AppendLine($" - {remedy}");
+                }
+            }
+
+            await console.Error.WriteAsync(msg);
+            return 1;
+        }
 
         return await new CliApplicationBuilder()
             .AddCommandsFromThisAssembly()
             .SetExecutableName(ExecutableName)
             .SetVersion(BuildVersion())
             .UseTypeActivator(type => CliTypeActivator.ResolveType(_container, type))
-            .UseConsole(_container.Resolve<IConsole>())
+            .UseConsole(console)
             .Build()
             .RunAsync();
     }