refactor: Improved error messaging for backward breaking config changes

src/Recyclarr.TrashLib.Tests/Config/Parsing/ConfigurationFinderTest.cs

@@ -1,6 +1,7 @@
 using System.IO.Abstractions;
 using System.IO.Abstractions.Extensions;
 using Recyclarr.TrashLib.Config.Parsing;
+using Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
 using Recyclarr.TrashLib.Startup;
 
 namespace Recyclarr.TrashLib.Tests.Config.Parsing;

src/Recyclarr.TrashLib/Config/Parsing/ConfigDeprecations.cs

@@ -1,21 +0,0 @@
-using YamlDotNet.Core;
-
-namespace Recyclarr.TrashLib.Config.Parsing;
-
-public static class ConfigDeprecations
-{
-    public static string? GetContextualErrorFromException(YamlException e)
-    {
-        if (e.Message.Contains("Expected 'MappingStart', got 'SequenceStart'"))
-        {
-            return "Found array-style list of instances instead of named-style. " +
-                "Array-style lists of Sonarr/Radarr instances are not supported. " +
-                "See: https://recyclarr.dev/wiki/upgrade-guide/v5.0/#instances-must-now-be-named";
-        }
-
-        // "DEPRECATION: Support for using `reset_unmatched_scores` under `custom_formats.quality_profiles` " +
-        //     "will be removed in a future release. Move it to the top level `quality_profiles` instead"
-
-        return null;
-    }
-}

src/Recyclarr.TrashLib/Config/Parsing/ConfigParser.cs

@@ -1,6 +1,7 @@
 using System.IO.Abstractions;
 using FluentValidation;
 using JetBrains.Annotations;
+using Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
 using Recyclarr.TrashLib.Config.Yaml;
 using Serilog.Context;
 using YamlDotNet.Core;
@@ -56,6 +57,10 @@ public class ConfigParser
 
             return config;
         }
+        catch (FeatureRemovalException e)
+        {
+            _log.Error(e, "Unsupported feature");
+        }
         catch (YamlException e)
         {
             _log.Debug(e, "Exception while parsing config file");
@@ -69,17 +74,14 @@ public class ConfigParser
                     break;
 
                 default:
-                    // Check for Configuration-specific deprecation messages
+                    var msg = ContextualMessages.GetContextualErrorFromException(e) ??
-                    var msg = ConfigDeprecations.GetContextualErrorFromException(e) ??
                         e.InnerException?.Message ?? e.Message;
-
                     _log.Error("Exception at line {Line}: {Msg}", line, msg);
                     break;
             }
-
-            _log.Error("Due to previous exception, this config will be skipped");
         }
 
+        _log.Error("Due to previous exception, this config will be skipped");
         return null;
     }
 }

src/Recyclarr.TrashLib/Config/Parsing/ConfigYamlDataObjects.cs

@@ -8,7 +8,6 @@ public record QualityScoreConfigYaml
 {
     public string? Name { get; [UsedImplicitly] init; }
     public int? Score { get; [UsedImplicitly] init; }
-    public bool? ResetUnmatchedScores { get; [UsedImplicitly] init; }
 }
 
 public record CustomFormatConfigYaml

src/Recyclarr.TrashLib/Config/Parsing/ConfigYamlDataObjectsValidation.cs

@@ -57,13 +57,6 @@ public class QualityScoreConfigYamlValidator : AbstractValidator<QualityScoreCon
     {
         RuleFor(x => x.Name).NotEmpty()
             .WithMessage("'name' is required for elements under 'quality_profiles'");
-
-        RuleFor(x => x.ResetUnmatchedScores).Null()
-            .WithSeverity(Severity.Warning)
-            .WithMessage(
-                "Usage of 'reset_unmatched_scores' inside 'quality_profiles' under 'custom_formats' is deprecated. " +
-                "Use the root-level 'quality_profiles' instead. " +
-                "See: https://recyclarr.dev/wiki/upgrade-guide/v5.0#reset-unmatched-scores");
     }
 }
 

src/Recyclarr.TrashLib/Config/Parsing/ConfigurationFinder.cs

@@ -1,4 +1,5 @@
 using System.IO.Abstractions;
+using Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
 using Recyclarr.TrashLib.Startup;
 
 namespace Recyclarr.TrashLib.Config.Parsing;

src/Recyclarr.TrashLib/Config/Parsing/ErrorHandling/ConfigFeatureRemovalBehavior.cs

@@ -0,0 +1,12 @@
+using Recyclarr.TrashLib.Config.Yaml;
+using YamlDotNet.Serialization;
+
+namespace Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
+
+public class ConfigFeatureRemovalBehavior : IYamlBehavior
+{
+    public void Setup(DeserializerBuilder builder)
+    {
+        builder.WithNodeTypeResolver(new FeatureRemovalChecker());
+    }
+}

src/Recyclarr.TrashLib/Config/Parsing/ErrorHandling/ConfigurationException.cs

@@ -2,7 +2,7 @@ using System.Runtime.Serialization;
 using System.Text;
 using FluentValidation.Results;
 
-namespace Recyclarr.TrashLib.Config.Parsing;
+namespace Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
 
 [Serializable]
 public class ConfigurationException : Exception

src/Recyclarr.TrashLib/Config/Parsing/ErrorHandling/ContextualMessages.cs

@@ -0,0 +1,21 @@
+using YamlDotNet.Core;
+
+namespace Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
+
+public static class ContextualMessages
+{
+    public static string? GetContextualErrorFromException(YamlException e)
+    {
+        if (e.Message.Contains(
+                "Property 'reset_unmatched_scores' not found on type " +
+                $"'{typeof(QualityScoreConfigYaml).FullName}'"))
+        {
+            return
+                "Usage of 'reset_unmatched_scores' inside 'quality_profiles' under 'custom_formats' is no " +
+                "longer supported. Use the root-level 'quality_profiles' instead. " +
+                "See: https://recyclarr.dev/wiki/upgrade-guide/v5.0#reset-unmatched-scores";
+        }
+
+        return null;
+    }
+}

src/Recyclarr.TrashLib/Config/Parsing/ErrorHandling/FeatureRemovalChecker.cs

@@ -0,0 +1,34 @@
+using YamlDotNet.Core.Events;
+using YamlDotNet.Serialization;
+
+namespace Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
+
+// Note: Backward breaking changes involving node removals cannot be handled here, since that will cause exceptions
+// before the Node Type Resolver gets invoked. Those are handled reactively by inspecting the YamlException object
+// passed to the ContextualMessages static class.
+public sealed class FeatureRemovalChecker : INodeTypeResolver
+{
+    public bool Resolve(NodeEvent? nodeEvent, ref Type currentType)
+    {
+        if (IsDictionaryOfType(currentType, typeof(RadarrConfigYaml), typeof(SonarrConfigYaml)) &&
+            nodeEvent is SequenceStart)
+        {
+            throw new FeatureRemovalException(
+                "Found array-style list of instances instead of named-style. " +
+                "Array-style lists of Sonarr/Radarr instances are not supported.",
+                "https://recyclarr.dev/wiki/upgrade-guide/v5.0/#instances-must-now-be-named");
+        }
+
+        return false;
+    }
+
+    private static bool IsDictionaryOfType(Type dictType, params Type[] valueTypes)
+    {
+        if (!dictType.IsGenericType || dictType.GetGenericTypeDefinition() != typeof(IReadOnlyDictionary<,>))
+        {
+            return false;
+        }
+
+        return valueTypes.Contains(dictType.GenericTypeArguments[1]);
+    }
+}

src/Recyclarr.TrashLib/Config/Parsing/ErrorHandling/FeatureRemovalException.cs

@@ -0,0 +1,17 @@
+using System.Runtime.Serialization;
+
+namespace Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
+
+[Serializable]
+public class FeatureRemovalException : Exception
+{
+    protected FeatureRemovalException(SerializationInfo info, StreamingContext context)
+        : base(info, context)
+    {
+    }
+
+    public FeatureRemovalException(string message, string docLink)
+        : base($"{message} See: {docLink}")
+    {
+    }
+}

src/Recyclarr.TrashLib/Config/Parsing/ErrorHandling/NoConfigurationFilesException.cs

@@ -1,4 +1,4 @@
-namespace Recyclarr.TrashLib.Config.Parsing;
+namespace Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
 
 public class NoConfigurationFilesException : Exception
 {

src/Recyclarr.TrashLib/Config/Parsing/ErrorHandling/SyntaxErrorHelper.cs

@@ -0,0 +1,29 @@
+using System.Reflection;
+using YamlDotNet.Core;
+using YamlDotNet.Core.Events;
+using YamlDotNet.Serialization;
+
+namespace Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
+
+public sealed class SyntaxErrorHelper : INodeTypeResolver
+{
+    private static readonly string[] CollectionKeywords = {"Collection", "List"};
+
+    public bool Resolve(NodeEvent? nodeEvent, ref Type currentType)
+    {
+        CheckSequenceAssignedToNonSequence(nodeEvent, currentType);
+        return false;
+    }
+
+    // If the user tries to specify an array as the value for a node type that is not a list type, then we provide our
+    // own exception type. The default error message that YamlDotNet would output doesn't make much sense to users: It
+    // just says "no node type resolver could resolve the type", or something along those lines -- which isn't helpful!
+    private static void CheckSequenceAssignedToNonSequence(ParsingEvent? nodeEvent, MemberInfo currentType)
+    {
+        if (nodeEvent is SequenceStart && !CollectionKeywords.Any(x => currentType.Name.Contains(x)))
+        {
+            throw new YamlException(nodeEvent.Start, nodeEvent.End,
+                $"A list/array/sequence is not allowed for {currentType.Name}");
+        }
+    }
+}

src/Recyclarr.TrashLib/Config/Yaml/YamlSerializerFactory.cs

@@ -1,4 +1,5 @@
 using Recyclarr.Common.YamlDotNet;
+using Recyclarr.TrashLib.Config.Parsing.ErrorHandling;
 using YamlDotNet.Serialization;
 using YamlDotNet.Serialization.NamingConventions;
 
@@ -17,11 +18,19 @@ public class YamlSerializerFactory : IYamlSerializerFactory
 
     public IDeserializer CreateDeserializer()
     {
-        var builder = new DeserializerBuilder()
+        var builder = new DeserializerBuilder();
+
+        // This MUST be first (amongst the other node type resolvers) because that means it will be processed LAST. This
+        // is a last resort utility resolver to make error messages more clear. We do not want it interfering with other
+        // resolvers.
+        builder.WithNodeTypeResolver(new SyntaxErrorHelper());
+
+        builder
+            .IgnoreFields()
             .WithNamingConvention(UnderscoredNamingConvention.Instance)
             .WithTypeConverter(new YamlNullableEnumTypeConverter())
-            .WithNodeTypeResolver(new ReadOnlyCollectionNodeTypeResolver())
             .WithNodeDeserializer(new ForceEmptySequences(_objectFactory))
+            .WithNodeTypeResolver(new ReadOnlyCollectionNodeTypeResolver())
             .WithObjectFactory(_objectFactory);
 
         foreach (var behavior in _behaviors)