diff --git a/Source/AppScaffolding/AppScaffolding.csproj b/Source/AppScaffolding/AppScaffolding.csproj index c61dff9e..94738dc9 100644 --- a/Source/AppScaffolding/AppScaffolding.csproj +++ b/Source/AppScaffolding/AppScaffolding.csproj @@ -2,7 +2,7 @@ net10.0 - 13.7.5 + 13.7.6 enable diff --git a/Source/AppScaffolding/LibationScaffolding.cs b/Source/AppScaffolding/LibationScaffolding.cs index c6b3a38f..150828ed 100644 --- a/Source/AppScaffolding/LibationScaffolding.cs +++ b/Source/AppScaffolding/LibationScaffolding.cs @@ -94,6 +94,7 @@ public static class LibationScaffolding { config.LoadPersistentSettings(config.LibationFiles.SettingsFilePath); } + config.ValidateEnumSettings(); DeleteOpenSqliteFiles(config); AudibleApiStorage.EnsureAccountsSettingsFileExists(); IdentityTokenStorageWiring.Apply(config); @@ -102,9 +103,6 @@ public static class LibationScaffolding // migrations go below here // - Migrations.migrate_to_v6_6_9(config); - Migrations.migrate_to_v11_5_0(config); - Migrations.migrate_to_v11_6_5(config); Migrations.migrate_to_v12_0_1(config); } @@ -163,65 +161,7 @@ public static class LibationScaffolding } private static void ensureSerilogConfig(Configuration config) - { - if (config.GetObject("Serilog") is JObject serilog) - { - bool fileChanged = false; - if (serilog.SelectToken("$.WriteTo[?(@.Name == 'ZipFile')]", false) is JObject zipFileSink) - { - zipFileSink["Name"] = "File"; - fileChanged = true; - } - var hooks = typeof(FileSinkHook).AssemblyQualifiedName; - if (serilog.SelectToken("$.WriteTo[?(@.Name == 'File')].Args", false) is JObject fileSinkArgs - && fileSinkArgs["hooks"]?.Value() != hooks) - { - fileSinkArgs["hooks"] = hooks; - fileChanged = true; - } - - if (fileChanged) - config.SetNonString(serilog.DeepClone(), "Serilog"); - return; - } - - var serilogObj = new JObject - { - { "MinimumLevel", "Information" }, - { "WriteTo", new JArray - { - // ABOUT SINKS - // Only File sink is currently used. By user request (June 2024) others packages are included for experimental use. - - // new JObject { {"Name", "Console" } }, // this has caused more problems than it's solved - new JObject - { - { "Name", "File" }, - { "Args", - new JObject - { - // for this sink to work, a path must be provided. we override this below - { "path", Path.Combine(config.LibationFiles.Location, "Log.log") }, - { "rollingInterval", "Month" }, - // Serilog template formatting examples - // - default: "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] {Message:lj}{NewLine}{Exception}" - // output example: 2019-11-26 08:48:40.224 -05:00 [DBG] Begin Libation - // - with class and method info: "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] (at {Caller}) {Message:lj}{NewLine}{Exception}"; - // output example: 2019-11-26 08:48:40.224 -05:00 [DBG] (at LibationWinForms.Program.init()) Begin Libation - // {Properties:j} needed for expanded exception logging - { "outputTemplate", "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] (at {Caller}) {Message:lj}{NewLine}{Exception} {Properties:j}" }, - { "hooks", typeof(FileSinkHook).AssemblyQualifiedName }, // for FileSinkHook - } - } - } - } - }, - // better exception logging with: Serilog.Exceptions library -- WithExceptionDetails - { "Using", new JArray{ "Dinah.Core", "Serilog.Exceptions" } }, // dll's name, NOT namespace - { "Enrich", new JArray{ "WithCaller", "WithExceptionDetails" } }, - }; - config.SetNonString(serilogObj, "Serilog"); - } + => config.EnsureSerilogConfig(); // to restore original: Console.SetOut(origOut); private static TextWriter origOut { get; } = Console.Out; @@ -463,59 +403,6 @@ public static class LibationScaffolding internal static class Migrations { - public static void migrate_to_v6_6_9(Configuration config) - { - var writeToPath = $"Serilog.WriteTo"; - - // remove WriteTo[].Name == Console - { - if (UNSAFE_MigrationHelper.Settings_TryGetArrayLength(writeToPath, out var length1)) - { - for (var i = length1 - 1; i >= 0; i--) - { - var exists = UNSAFE_MigrationHelper.Settings_TryGetFromJsonPath($"{writeToPath}[{i}].Name", out var value); - - if (exists && value == "Console") - UNSAFE_MigrationHelper.Settings_RemoveFromArray(writeToPath, i); - } - } - } - - // add Serilog.Exceptions -- WithExceptionDetails - { - // outputTemplate should contain "{Properties:j}" - { - // re-calculate. previous loop may have changed the length - if (UNSAFE_MigrationHelper.Settings_TryGetArrayLength(writeToPath, out var length2)) - { - var propertyName = "outputTemplate"; - for (var i = 0; i < length2; i++) - { - var jsonPath = $"{writeToPath}[{i}].Args"; - var exists = UNSAFE_MigrationHelper.Settings_TryGetFromJsonPath($"{jsonPath}.{propertyName}", out var value); - - var newValue = "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] (at {Caller}) {Message:lj}{NewLine}{Exception} {Properties:j}"; - - if (exists && value != newValue) - UNSAFE_MigrationHelper.Settings_SetWithJsonPath(jsonPath, propertyName, newValue); - } - } - } - - // Serilog.Using must include "Serilog.Exceptions" - UNSAFE_MigrationHelper.Settings_AddUniqueToArray("Serilog.Using", "Serilog.Exceptions"); - - // Serilog.Enrich must include "WithExceptionDetails" - UNSAFE_MigrationHelper.Settings_AddUniqueToArray("Serilog.Enrich", "WithExceptionDetails"); - } - } - - class FilterState_6_6_9 - { - public bool UseDefault { get; set; } - public List Filters { get; set; } = []; - } - public static void migrate_to_v12_0_1(Configuration config) { //Migrate from version 1 file cache to the dictionary-based version 2 cache @@ -588,59 +475,4 @@ internal static class Migrations catch { /* eat */ } } } - - public static void migrate_to_v11_6_5(Configuration config) - { - //Settings migration for unsupported sample rates (#1116) - if (config.MaxSampleRate < AAXClean.SampleRate.Hz_8000) - config.MaxSampleRate = AAXClean.SampleRate.Hz_8000; - else if (config.MaxSampleRate > AAXClean.SampleRate.Hz_48000) - config.MaxSampleRate = AAXClean.SampleRate.Hz_48000; - } - - public static void migrate_to_v11_5_0(Configuration config) - { - // Read file, but convert old format to new (with Name field) as necessary. - if (!File.Exists(QuickFilters.JsonFile)) - { - QuickFilters.InMemoryState = new(); - return; - } - - try - { - if (JsonConvert.DeserializeObject(File.ReadAllText(QuickFilters.JsonFile)) - is QuickFilters.FilterState inMemState) - { - QuickFilters.InMemoryState = inMemState; - return; - } - } - catch - { - // Eat - } - - try - { - if (JsonConvert.DeserializeObject(File.ReadAllText(QuickFilters.JsonFile)) - is FilterState_6_6_9 inMemState) - { - // Copy old structure to new. - QuickFilters.InMemoryState = new() - { - UseDefault = inMemState.UseDefault - }; - foreach (var oldFilter in inMemState.Filters) - QuickFilters.InMemoryState.Filters.Add(new QuickFilters.NamedFilter(oldFilter, null)); - - return; - } - Debug.Assert(false, "Should not get here, QuickFilters.json deserialization issue"); - } - catch - { - // Eat - } - } } diff --git a/Source/AudibleUtilities/AudibleUtilities.csproj b/Source/AudibleUtilities/AudibleUtilities.csproj index 09d9f3d7..a3f4ebcd 100644 --- a/Source/AudibleUtilities/AudibleUtilities.csproj +++ b/Source/AudibleUtilities/AudibleUtilities.csproj @@ -7,7 +7,7 @@ - + diff --git a/Source/FileManager/IJsonBackedDictionary.cs b/Source/FileManager/IJsonBackedDictionary.cs index 89510fa3..f83a0721 100644 --- a/Source/FileManager/IJsonBackedDictionary.cs +++ b/Source/FileManager/IJsonBackedDictionary.cs @@ -1,6 +1,5 @@ using Newtonsoft.Json.Linq; using System; -using System.Linq; namespace FileManager; @@ -20,21 +19,66 @@ public interface IJsonBackedDictionary string? GetStringFromJsonPath(string jsonPath, string propertyName) => GetStringFromJsonPath($"{jsonPath}.{propertyName}"); - static T? UpCast(object obj) + static T? UpCast(object obj, string? propertyName = null) { if (obj.GetType().IsAssignableTo(typeof(T))) return (T)obj; if (obj is JObject jObject) return jObject.ToObject(); if (obj is JValue jValue) { if (typeof(T).IsAssignableTo(typeof(Enum))) - { - return - Enum.TryParse(typeof(T), jValue.Value(), out var enumVal) - ? (T)enumVal - : Enum.GetValues(typeof(T)).Cast().First(); - } + return ParseEnum(jValue, propertyName ?? typeof(T).Name); + return jValue.Value(); } throw new InvalidCastException($"{obj.GetType()} is not convertible to {typeof(T)}"); } + + private static T ParseEnum(JValue jValue, string propertyPath) + { + var enumType = typeof(T); + + if (TryGetEnumFromNumber(jValue, enumType, propertyPath, out var fromNumber)) + return (T)fromNumber!; + + var text = jValue.Type == JTokenType.String + ? jValue.Value() + : jValue.Value?.ToString(); + + if (text is not null + && Enum.TryParse(enumType, text, ignoreCase: true, out var parsed) + && parsed is not null + && Enum.IsDefined(enumType, parsed)) + { + return (T)parsed; + } + + throw InvalidConfigurationValueException.ForEnum(propertyPath, text ?? jValue.ToString(), enumType); + } + + private static bool TryGetEnumFromNumber(JValue jValue, Type enumType, string propertyPath, out object? value) + { + value = null; + if (jValue.Type is not (JTokenType.Integer or JTokenType.Float)) + return false; + + object? raw = jValue.Value; + if (raw is null) + return false; + + object converted; + try + { + converted = Convert.ChangeType(raw, Enum.GetUnderlyingType(enumType)); + } + catch (Exception ex) when (ex is InvalidCastException or FormatException or OverflowException) + { + throw InvalidConfigurationValueException.ForEnum(propertyPath, raw.ToString(), enumType); + } + + if (!Enum.IsDefined(enumType, converted)) + throw InvalidConfigurationValueException.ForEnum(propertyPath, raw.ToString(), enumType); + + value = Enum.ToObject(enumType, converted); + return true; + } } diff --git a/Source/FileManager/InvalidConfigurationValueException.cs b/Source/FileManager/InvalidConfigurationValueException.cs new file mode 100644 index 00000000..1128071e --- /dev/null +++ b/Source/FileManager/InvalidConfigurationValueException.cs @@ -0,0 +1,39 @@ +using System; + +namespace FileManager; + +/// +/// Settings.json (or other JSON-backed config) contained a value that cannot be mapped to the expected type. +/// +public sealed class InvalidConfigurationValueException : Exception +{ + public string PropertyPath { get; } + public string? InvalidValue { get; } + public Type? ExpectedType { get; } + + public InvalidConfigurationValueException(string propertyPath, string? invalidValue, Type? expectedType, string message) + : base(message) + { + PropertyPath = propertyPath; + InvalidValue = invalidValue; + ExpectedType = expectedType; + } + + public static InvalidConfigurationValueException ForEnum(string propertyPath, string? invalidValue, Type enumType) + { + var allowed = string.Join(", ", Enum.GetNames(enumType)); + var display = FormatValue(invalidValue); + var message = + $"Invalid value for '{propertyPath}': {display}. " + + $"Expected one of: {allowed}."; + return new InvalidConfigurationValueException(propertyPath, invalidValue, enumType, message); + } + + public static InvalidConfigurationValueException ForPath(string propertyPath, string? invalidValue, string message) + => new(propertyPath, invalidValue, expectedType: null, message); + + public static string FormatValue(string? value) + => value is null ? "[null]" + : value.Length == 0 ? "[empty]" + : $"\"{value}\""; +} diff --git a/Source/FileManager/PersistentDictionary.cs b/Source/FileManager/PersistentDictionary.cs index be31a484..bc28e7f3 100644 --- a/Source/FileManager/PersistentDictionary.cs +++ b/Source/FileManager/PersistentDictionary.cs @@ -58,7 +58,7 @@ public class PersistentDictionary : IJsonBackedDictionary objectCache[propertyName] = defaultValue; return defaultValue; } - return IJsonBackedDictionary.UpCast(obj); + return IJsonBackedDictionary.UpCast(obj, propertyName); } public object? GetObject(string propertyName) diff --git a/Source/LibationCli/Options/_OptionsBase.cs b/Source/LibationCli/Options/_OptionsBase.cs index 9790281e..cbf6d06f 100644 --- a/Source/LibationCli/Options/_OptionsBase.cs +++ b/Source/LibationCli/Options/_OptionsBase.cs @@ -33,7 +33,20 @@ public abstract class OptionsBase // do not use Configuration before this line // // // //***********************************************// - Setup.Initialize(); + try + { + Setup.Initialize(); + } + catch (Exception ex) when (StartupAssemblyBootstrap.TryFindInvalidConfigurationValue(ex, out var configEx) && configEx is not null) + { + Environment.ExitCode = (int)ExitCode.RunTimeError; + Console.Error.WriteLine("Invalid configuration"); + Console.Error.WriteLine("====================="); + Console.Error.WriteLine(configEx.Message); + Console.Error.WriteLine(); + Console.Error.WriteLine("Edit Settings.json to use a valid value, then retry."); + return; + } if (SettingOverrides is not null) ProcessSettingsOverrides(); diff --git a/Source/LibationFileManager/Configuration.Logging.cs b/Source/LibationFileManager/Configuration.Logging.cs index 2231015f..a02ef15e 100644 --- a/Source/LibationFileManager/Configuration.Logging.cs +++ b/Source/LibationFileManager/Configuration.Logging.cs @@ -1,12 +1,17 @@ using Dinah.Core.Logging; using FileManager; using Microsoft.Extensions.Configuration; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; using Serilog; using Serilog.Events; using Serilog.Exceptions; using Serilog.Settings.Configuration; using System; using System.ComponentModel; +using System.IO; +using System.Linq; +using System.Text; namespace LibationFileManager; @@ -16,16 +21,81 @@ public partial class Configuration public bool SerilogInitialized { get; private set; } + /// + /// Create default Serilog config if missing, and migrate legacy ZipFile sink to File. + /// Must run before / . + /// + public void EnsureSerilogConfig() + { + if (GetObject("Serilog") is JObject serilog) + { + bool fileChanged = false; + foreach (var zipFileSink in serilog.SelectTokens("$.WriteTo[?(@.Name == 'ZipFile')]", false).OfType()) + { + zipFileSink["Name"] = "File"; + fileChanged = true; + } + var hooks = typeof(FileSinkHook).AssemblyQualifiedName; + foreach (var fileSinkArgs in serilog.SelectTokens("$.WriteTo[?(@.Name == 'File')].Args", false).OfType()) + { + if (fileSinkArgs["hooks"]?.Value() != hooks) + { + fileSinkArgs["hooks"] = hooks; + fileChanged = true; + } + } + + if (fileChanged) + SetNonString(serilog.DeepClone(), "Serilog"); + return; + } + + var serilogObj = new JObject + { + { "MinimumLevel", "Information" }, + { "WriteTo", new JArray + { + // ABOUT SINKS + // Only File sink is currently used. By user request (June 2024) others packages are included for experimental use. + + // new JObject { {"Name", "Console" } }, // this has caused more problems than it's solved + new JObject + { + { "Name", "File" }, + { "Args", + new JObject + { + // for this sink to work, a path must be provided. we override this below + { "path", Path.Combine(LibationFiles.Location, "Log.log") }, + { "rollingInterval", "Month" }, + // Serilog template formatting examples + // - default: "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] {Message:lj}{NewLine}{Exception}" + // output example: 2019-11-26 08:48:40.224 -05:00 [DBG] Begin Libation + // - with class and method info: "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] (at {Caller}) {Message:lj}{NewLine}{Exception}"; + // output example: 2019-11-26 08:48:40.224 -05:00 [DBG] (at LibationWinForms.Program.init()) Begin Libation + // {Properties:j} needed for expanded exception logging + { "outputTemplate", "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] (at {Caller}) {Message:lj}{NewLine}{Exception} {Properties:j}" }, + { "hooks", typeof(FileSinkHook).AssemblyQualifiedName }, // for FileSinkHook + } + } + } + } + }, + // better exception logging with: Serilog.Exceptions library -- WithExceptionDetails + { "Using", new JArray{ "Dinah.Core", "Serilog.Exceptions" } }, // dll's name, NOT namespace + { "Enrich", new JArray{ "WithCaller", "WithExceptionDetails" } }, + }; + SetNonString(serilogObj, "Serilog"); + } + public void ConfigureLogging() { - //pass explicit assemblies to the ConfigurationReaderOptions - //This is a workaround for the issue where serilog will try to load all - //Assemblies starting with "serilog" from the app folder, but it will fail - //if those assemblies are unreferenced. - //This was a problem when migrating from the ZipFile sink to the File sink. - //Upgrading users would still have the ZipFile sink dll in the program - //folder and serilog would try to load it, unsuccessfully. - //https://github.com/serilog/serilog-settings-configuration/issues/406 + ValidateSerilogConfiguration(); + + // Pass explicit assemblies to ConfigurationReaderOptions. + // Workaround: Serilog otherwise loads all "Serilog*" assemblies from the app folder and fails + // on unreferenced leftovers (e.g. ZipFile sink after migration). + // https://github.com/serilog/serilog-settings-configuration/issues/406 var readerOptions = new ConfigurationReaderOptions( typeof(ILogger).Assembly, // Serilog typeof(LoggerCallerEnrichmentConfiguration).Assembly, // Dinah.Core @@ -33,9 +103,9 @@ public partial class Configuration typeof(ConsoleLoggerConfigurationExtensions).Assembly, // Serilog.Sinks.Console typeof(FileLoggerConfigurationExtensions).Assembly); // Serilog.Sinks.File - configuration = new ConfigurationBuilder() - .AddJsonFile(Instance.LibationFiles.SettingsFilePath, optional: false, reloadOnChange: true) - .Build(); + // Build from the in-memory settings store so ZipFile->File migration (and CLI ephemeral + // settings) are what Serilog sees, not a stale disk copy. + configuration = CreateLoggingConfigurationRoot(); Log.Logger = new LoggerConfiguration() .ReadFrom.Configuration(configuration, readerOptions) .Destructure.ByTransforming(lp => lp.Path) @@ -44,6 +114,152 @@ public partial class Configuration SerilogInitialized = true; } + private IConfigurationRoot CreateLoggingConfigurationRoot() + { + var json = Settings.GetJObject().ToString(Formatting.None); + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(json)); + return new ConfigurationBuilder() + .AddJsonStream(stream) + .Build(); + } + + /// + /// Fail fast on structurally broken Serilog config (missing/empty WriteTo, bad MinimumLevel). + /// Hand-edited custom sink names are allowed; legacy ZipFile is migrated by . + /// Call after ZipFile->File migration in scaffolding. + /// + public void ValidateSerilogConfiguration() + { + var settingsPath = LibationFiles.SettingsFilePath; + if (GetObject("Serilog") is not JObject serilog) + { + throw InvalidConfigurationValueException.ForPath( + "Serilog", + null, + $"Settings.json ({settingsPath}) is missing a Serilog section. " + + "Add a Serilog configuration with at least one WriteTo sink (Libation defaults to File)."); + } + + ValidateSerilogMinimumLevel(serilog, settingsPath); + ValidateSerilogWriteTo(serilog, settingsPath); + } + + private static void ValidateSerilogMinimumLevel(JObject serilog, string settingsPath) + { + var minLevelToken = serilog["MinimumLevel"]; + if (minLevelToken is null) + return; + + string? levelText = minLevelToken.Type switch + { + JTokenType.String => minLevelToken.Value(), + JTokenType.Object => minLevelToken["Default"]?.Value(), + _ => minLevelToken.ToString() + }; + + if (levelText is null) + return; + + if (!Enum.TryParse(levelText, ignoreCase: true, out _)) + { + var allowed = string.Join(", ", Enum.GetNames()); + throw InvalidConfigurationValueException.ForPath( + "Serilog.MinimumLevel", + levelText, + $"Invalid value for 'Serilog.MinimumLevel' in Settings.json ({settingsPath}): " + + $"{InvalidConfigurationValueException.FormatValue(levelText)}. Expected one of: {allowed}."); + } + } + + private static void ValidateSerilogWriteTo(JObject serilog, string settingsPath) + { + var writeToToken = serilog["WriteTo"]; + if (writeToToken is null) + { + throw InvalidConfigurationValueException.ForPath( + "Serilog.WriteTo", + null, + $"Settings.json ({settingsPath}) Serilog section has no WriteTo sinks. " + + "Add at least one WriteTo sink (Libation defaults to File)."); + } + + if (writeToToken is not JArray writeTo) + { + throw InvalidConfigurationValueException.ForPath( + "Serilog.WriteTo", + writeToToken.Type.ToString(), + $"Settings.json ({settingsPath}) 'Serilog.WriteTo' must be a JSON array of sink objects."); + } + + if (writeTo.Count == 0) + { + throw InvalidConfigurationValueException.ForPath( + "Serilog.WriteTo", + "[]", + $"Settings.json ({settingsPath}) Serilog.WriteTo is empty. " + + "Add at least one WriteTo sink (Libation defaults to File)."); + } + + for (var i = 0; i < writeTo.Count; i++) + { + var path = $"Serilog.WriteTo[{i}]"; + if (writeTo[i] is not JObject sink) + { + throw InvalidConfigurationValueException.ForPath( + path, + writeTo[i]?.Type.ToString(), + $"Settings.json ({settingsPath}) '{path}' must be a JSON object with a Name property."); + } + + var name = sink["Name"]?.Value(); + var namePath = $"{path}.Name"; + if (string.IsNullOrWhiteSpace(name)) + { + throw InvalidConfigurationValueException.ForPath( + namePath, + name, + $"Settings.json ({settingsPath}) '{namePath}' is missing or empty."); + } + } + } + + /// + /// Force-read enum-backed settings so invalid values fail at startup instead of later. + /// + public void ValidateEnumSettings() + { + try + { + _ = ThemeVariant; + _ = MaxSampleRate; + _ = LameEncoderQuality; + _ = ClipsBookmarksFileFormat; + _ = TokenStorageMethod; + _ = SpatialAudioCodec; + _ = FileDownloadQuality; + _ = CreationTime; + _ = LastWriteTime; + _ = BadBook; + } + catch (InvalidConfigurationValueException ex) + { + throw EnhanceWithSettingsPath(ex); + } + } + + private InvalidConfigurationValueException EnhanceWithSettingsPath(InvalidConfigurationValueException ex) + { + var path = LibationFiles.SettingsFilePath; + if (ex.Message.Contains(path, StringComparison.OrdinalIgnoreCase)) + return ex; + + return new InvalidConfigurationValueException( + ex.PropertyPath, + ex.InvalidValue, + ex.ExpectedType, + $"Settings.json ({path}): {ex.Message}"); + } + [Description("The importance of a log event")] public LogEventLevel LogLevel { @@ -62,7 +278,11 @@ public partial class Configuration return; } - configuration?.Reload(); + if (SerilogInitialized) + { + // Rebuild from current settings (in-memory or disk) so MinimumLevel applies. + ConfigureLogging(); + } OnPropertyChanged(nameof(LogLevel), value); diff --git a/Source/LibationFileManager/Configuration.PersistentSettings.cs b/Source/LibationFileManager/Configuration.PersistentSettings.cs index 9695e5ee..e9ba729d 100644 --- a/Source/LibationFileManager/Configuration.PersistentSettings.cs +++ b/Source/LibationFileManager/Configuration.PersistentSettings.cs @@ -37,7 +37,18 @@ public partial class Configuration [return: NotNullIfNotNull(nameof(defaultValue))] public T? GetNonString(T defaultValue, [CallerMemberName] string propertyName = "") - => Settings is null ? default : Settings.GetNonString(propertyName, defaultValue); + { + if (Settings is null) + return default; + try + { + return Settings.GetNonString(propertyName, defaultValue); + } + catch (InvalidConfigurationValueException ex) + { + throw EnhanceWithSettingsPath(ex); + } + } [return: NotNullIfNotNull(nameof(defaultValue))] diff --git a/Source/LibationFileManager/EphemeralDictionary.cs b/Source/LibationFileManager/EphemeralDictionary.cs index 2a08b474..fbc6074a 100644 --- a/Source/LibationFileManager/EphemeralDictionary.cs +++ b/Source/LibationFileManager/EphemeralDictionary.cs @@ -22,7 +22,7 @@ internal class EphemeralDictionary : IJsonBackedDictionary public string? GetString(string propertyName, string? defaultValue = null) => JsonObject[propertyName]?.Value() ?? defaultValue; public T? GetNonString(string propertyName, T? defaultValue = default) - => GetObject(propertyName) is object obj ? IJsonBackedDictionary.UpCast(obj) : defaultValue; + => GetObject(propertyName) is object obj ? IJsonBackedDictionary.UpCast(obj, propertyName) : defaultValue; public object? GetObject(string propertyName) => JsonObject[propertyName]?.Value(); public void SetString(string propertyName, string? newValue) diff --git a/Source/LibationFileManager/LibationFileManager.csproj b/Source/LibationFileManager/LibationFileManager.csproj index 05f4bd8c..7298d5e6 100644 --- a/Source/LibationFileManager/LibationFileManager.csproj +++ b/Source/LibationFileManager/LibationFileManager.csproj @@ -6,7 +6,7 @@ - + diff --git a/Source/LibationFileManager/StartupAssemblyBootstrap.cs b/Source/LibationFileManager/StartupAssemblyBootstrap.cs index c46f759f..ca8d01e0 100644 --- a/Source/LibationFileManager/StartupAssemblyBootstrap.cs +++ b/Source/LibationFileManager/StartupAssemblyBootstrap.cs @@ -1,3 +1,4 @@ +using FileManager; using System; using System.IO; using System.Linq; @@ -158,6 +159,16 @@ public static class StartupAssemblyBootstrap public static FatalStartupMessage? GetStartupFailureMessage(Exception ex) { + if (TryFindInvalidConfigurationValue(ex, out var configEx) && configEx is not null) + { + return new FatalStartupMessage( + "Invalid Settings.json", + configEx.Message + + Environment.NewLine + + Environment.NewLine + + "Edit Settings.json to use a valid value, then restart Libation."); + } + if (IsApplicationControlBlockedAssembly(ex)) { return new FatalStartupMessage( @@ -182,6 +193,30 @@ public static class StartupAssemblyBootstrap return null; } + public static bool TryFindInvalidConfigurationValue(Exception? ex, out InvalidConfigurationValueException? configEx) + { + configEx = null; + for (var current = ex; current is not null; current = current.InnerException) + { + if (current is InvalidConfigurationValueException found) + { + configEx = found; + return true; + } + + if (current is AggregateException aggregate) + { + foreach (var inner in aggregate.InnerExceptions) + { + if (TryFindInvalidConfigurationValue(inner, out configEx)) + return true; + } + } + } + + return false; + } + /// /// Resolves a user-facing title and body for a fatal startup or crash, including emergency rollback when needed. /// diff --git a/Source/_Tests/LibationFileManager.Tests/SerilogConfigurationTests.cs b/Source/_Tests/LibationFileManager.Tests/SerilogConfigurationTests.cs new file mode 100644 index 00000000..94ce9204 --- /dev/null +++ b/Source/_Tests/LibationFileManager.Tests/SerilogConfigurationTests.cs @@ -0,0 +1,190 @@ +using AssertionHelper; +using FileManager; +using LibationFileManager; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Newtonsoft.Json.Linq; +using System.IO; +using System.Linq; + +namespace SerilogConfigurationTests; + +[TestClass] +[DoNotParallelize] +public class SerilogConfigurationTests +{ + [TestCleanup] + public void Cleanup() + { + Configuration.RestoreSingletonInstance(); + } + + [TestMethod] + public void Validate_accepts_File_sink() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString(CreateSerilog("File"), "Serilog"); + + config.ValidateSerilogConfiguration(); + } + + [TestMethod] + public void Validate_accepts_Console_sink() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString(CreateSerilog("Console"), "Serilog"); + + config.ValidateSerilogConfiguration(); + } + + [TestMethod] + public void Validate_accepts_hand_edited_custom_sink_name() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString(CreateSerilog("Seq"), "Serilog"); + + config.ValidateSerilogConfiguration(); + } + + [TestMethod] + public void EnsureSerilogConfig_migrates_ZipFile_to_File() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString(CreateSerilog("ZipFile"), "Serilog"); + + config.EnsureSerilogConfig(); + + var serilog = (JObject)config.GetObject("Serilog")!; + var name = serilog.SelectToken("$.WriteTo[0].Name")?.Value(); + Assert.AreEqual("File", name); + + config.ValidateSerilogConfiguration(); + } + + [TestMethod] + public void EnsureSerilogConfig_migrates_all_ZipFile_sinks() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString(new JObject + { + ["MinimumLevel"] = "Information", + ["WriteTo"] = new JArray + { + new JObject { ["Name"] = "ZipFile", ["Args"] = new JObject { ["path"] = "a.log" } }, + new JObject { ["Name"] = "Console" }, + new JObject { ["Name"] = "ZipFile", ["Args"] = new JObject { ["path"] = "b.log" } }, + } + }, "Serilog"); + + config.EnsureSerilogConfig(); + + var serilog = (JObject)config.GetObject("Serilog")!; + var names = serilog.SelectTokens("$.WriteTo[*].Name").Select(t => t.Value()).ToList(); + CollectionAssert.AreEqual(new[] { "File", "Console", "File" }, names); + + config.ValidateSerilogConfiguration(); + } + + [TestMethod] + public void Validate_rejects_invalid_MinimumLevel() + { + var config = Configuration.CreateMockInstance(); + var serilog = CreateSerilog("File"); + serilog["MinimumLevel"] = "Loud"; + config.SetNonString(serilog, "Serilog"); + + var ex = Assert.ThrowsExactly(config.ValidateSerilogConfiguration); + StringAssert.Contains(ex.Message, "MinimumLevel"); + StringAssert.Contains(ex.Message, "Loud"); + } + + [TestMethod] + public void Validate_rejects_malformed_WriteTo() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString(new JObject + { + ["MinimumLevel"] = "Information", + ["WriteTo"] = "File" + }, "Serilog"); + + var ex = Assert.ThrowsExactly(config.ValidateSerilogConfiguration); + StringAssert.Contains(ex.Message, "WriteTo"); + } + + [TestMethod] + public void Validate_rejects_empty_WriteTo() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString(new JObject + { + ["MinimumLevel"] = "Information", + ["WriteTo"] = new JArray() + }, "Serilog"); + + var ex = Assert.ThrowsExactly(config.ValidateSerilogConfiguration); + StringAssert.Contains(ex.Message, "empty"); + } + + [TestMethod] + public void Validate_rejects_missing_sink_Name() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString(new JObject + { + ["MinimumLevel"] = "Information", + ["WriteTo"] = new JArray { new JObject { ["Args"] = new JObject() } } + }, "Serilog"); + + var ex = Assert.ThrowsExactly(config.ValidateSerilogConfiguration); + StringAssert.Contains(ex.Message, "Name"); + } + + [TestMethod] + public void Ephemeral_ZipFile_migration_is_visible_to_Validate_without_disk_write() + { + // Mirrors CLI ephemeral startup: migrate in memory, then validate from the same store ConfigureLogging reads. + var config = Configuration.CreateMockInstance(); + config.IsEphemeralInstance.Should().BeTrue(); + config.SetNonString(CreateSerilog("ZipFile"), "Serilog"); + + config.EnsureSerilogConfig(); + config.ValidateSerilogConfiguration(); + + var serilog = (JObject)config.GetObject("Serilog")!; + Assert.AreEqual("File", serilog.SelectToken("$.WriteTo[0].Name")?.Value()); + } + + [TestMethod] + public void Fatal_startup_message_uses_invalid_configuration_body() + { + var ex = InvalidConfigurationValueException.ForEnum( + "TokenStorageMethod", + "Nope", + typeof(AudibleApi.Authorization.TokenStorageMethod)); + + var message = StartupAssemblyBootstrap.GetStartupFailureMessage(ex); + Assert.IsNotNull(message); + Assert.AreEqual("Invalid Settings.json", message!.Title); + StringAssert.Contains(message.Body, "TokenStorageMethod"); + StringAssert.Contains(message.Body, "Nope"); + } + + private static JObject CreateSerilog(string sinkName) => new() + { + ["MinimumLevel"] = "Information", + ["WriteTo"] = new JArray + { + new JObject + { + ["Name"] = sinkName, + ["Args"] = new JObject + { + ["path"] = Path.Combine(Path.GetTempPath(), "LibationTestLog.log"), + ["rollingInterval"] = "Month" + } + } + }, + ["Using"] = new JArray { "Dinah.Core", "Serilog.Exceptions" }, + ["Enrich"] = new JArray { "WithCaller", "WithExceptionDetails" } + }; +} diff --git a/Source/_Tests/LibationFileManager.Tests/TokenStorageMethodConfigurationTests.cs b/Source/_Tests/LibationFileManager.Tests/TokenStorageMethodConfigurationTests.cs index 1201d9cb..d40c368a 100644 --- a/Source/_Tests/LibationFileManager.Tests/TokenStorageMethodConfigurationTests.cs +++ b/Source/_Tests/LibationFileManager.Tests/TokenStorageMethodConfigurationTests.cs @@ -41,13 +41,56 @@ public class TokenStorageMethodConfigurationTests } [TestMethod] - public void Unknown_enum_value_resolves_to_Encrypted_not_Plaintext() + public void Unknown_enum_value_throws_InvalidConfigurationValueException() { - // Same UpCast path used by Settings.json deserialization for enum settings. - var parsed = IJsonBackedDictionary.UpCast(new JValue("NotARealMethod")); + var ex = Assert.ThrowsExactly( + () => IJsonBackedDictionary.UpCast(new JValue("NotARealMethod"), nameof(Configuration.TokenStorageMethod))); - Assert.AreEqual(TokenStorageMethod.Encrypted, parsed); - Assert.AreNotEqual(TokenStorageMethod.Plaintext, parsed); + StringAssert.Contains(ex.Message, "TokenStorageMethod"); + StringAssert.Contains(ex.Message, "NotARealMethod"); + StringAssert.Contains(ex.Message, "Plaintext"); + StringAssert.Contains(ex.Message, "Encrypted"); + } + + [TestMethod] + public void PlainText_casing_resolves_to_Plaintext() + { + Assert.AreEqual( + TokenStorageMethod.Plaintext, + IJsonBackedDictionary.UpCast(new JValue("PlainText"))); + Assert.AreEqual( + TokenStorageMethod.Plaintext, + IJsonBackedDictionary.UpCast(new JValue("plaintext"))); + } + + [TestMethod] + public void Undefined_numeric_enum_value_throws() + { + var ex = Assert.ThrowsExactly( + () => IJsonBackedDictionary.UpCast(new JValue(99), nameof(Configuration.TokenStorageMethod))); + + StringAssert.Contains(ex.Message, "TokenStorageMethod"); + StringAssert.Contains(ex.Message, "99"); + } + + [TestMethod] + public void Config_property_throws_for_invalid_TokenStorageMethod() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString("PlainTextTypo", nameof(Configuration.TokenStorageMethod)); + + var ex = Assert.ThrowsExactly(() => _ = config.TokenStorageMethod); + StringAssert.Contains(ex.Message, "TokenStorageMethod"); + StringAssert.Contains(ex.Message, "PlainTextTypo"); + } + + [TestMethod] + public void ValidateEnumSettings_throws_for_invalid_TokenStorageMethod() + { + var config = Configuration.CreateMockInstance(); + config.SetNonString("NotARealMethod", nameof(Configuration.TokenStorageMethod)); + + Assert.ThrowsExactly(config.ValidateEnumSettings); } [TestMethod] diff --git a/docs/advanced/troubleshoot.md b/docs/advanced/troubleshoot.md index f10fae1d..fe392588 100644 --- a/docs/advanced/troubleshoot.md +++ b/docs/advanced/troubleshoot.md @@ -1,5 +1,16 @@ # Troubleshooting Common Libation Errors +## Invalid Settings.json value (TokenStorageMethod, Serilog, etc.) + +**Symptoms:** Libation or LibationCli refuses to start with **Invalid Settings.json** (GUI) or **Invalid configuration** (CLI). The message names the setting, the bad value, and (for enums) the allowed values. + +**Common causes:** +- `TokenStorageMethod` mistyped (canonical values are `Encrypted` and `Plaintext` - casing variants like `PlainText` are accepted, but unknown spellings are not) +- `Serilog.WriteTo` missing, empty, or malformed (not an array of objects with `Name`). Hand-edited custom sink names are allowed; legacy `ZipFile` is migrated to `File` automatically +- `Serilog.MinimumLevel` set to a value that is not a Serilog level + +**Fix:** Edit `Settings.json` in your Libation Files directory to a valid value and restart. Do not delete the whole file unless it is corrupt JSON. + ## Invalid filenames or mangled paths (NTFS / Windows) NTFS filesystems (Windows, and NTFS-formatted external drives on Linux or Mac) do not allow colons (`:`) in filenames. Libation chooses filename replacement rules from the **OS it is running on**, not from the filesystem where books are saved. On Linux or in Docker, that often means colons are left in names even when `LIBATION_BOOKS_DIR` points at an NTFS volume, which can produce invalid paths, failed moves, or mangled folder names.