diff --git a/TrayApp.Test/FatalErrorTests.cs b/TrayApp.Test/FatalErrorTests.cs new file mode 100644 index 0000000..e7d1bc3 --- /dev/null +++ b/TrayApp.Test/FatalErrorTests.cs @@ -0,0 +1,67 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TrayApp.Test; + +using System; +using System.Diagnostics.CodeAnalysis; +using ktsu.TrayApp; +using ktsu.TrayApp.Menu; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests for the line between a failure the library absorbs and one it must not. +/// +[TestClass] +public class FatalErrorTests +{ + [TestMethod] + public void IsNotFatal_ForAnOrdinaryRefusal_IsTrue() + { + Assert.IsTrue(FatalError.IsNotFatal(new InvalidOperationException("no inhibitor here"))); + Assert.IsTrue(FatalError.IsNotFatal(new PlatformNotSupportedException())); + Assert.IsTrue(FatalError.IsNotFatal(new ObjectDisposedException("blocker"))); + } + + [TestMethod] + [SuppressMessage( + "Usage", + "CA2201:Do not raise reserved exception types", + Justification = "These are the reserved types under test; constructing one is how the filter that excludes them is checked, and none of them is raised.")] + public void IsNotFatal_ForAProcessThatIsAlreadyLost_IsFalse() + { + Assert.IsFalse(FatalError.IsNotFatal(new OutOfMemoryException())); + Assert.IsFalse(FatalError.IsNotFatal(new AccessViolationException())); + } + + [TestMethod] + public void IsNotFatal_ForARefusedAllocation_IsTrue() + { + // InsufficientMemoryException derives from OutOfMemoryException but means the opposite: one + // allocation was refused before being attempted, and the process is fine. + Assert.IsTrue(FatalError.IsNotFatal(new InsufficientMemoryException())); + } + + [TestMethod] + [SuppressMessage( + "Usage", + "CA2201:Do not raise reserved exception types", + Justification = "Throwing the reserved type is the behaviour under test: the menu must let it through rather than report it as a refusal.")] + public void Run_DoesNotSwallowAFatalException() + { + TrayMenu menu = new("Demo", () => "idle", []); + + // The same setter reached from a click and from a preference restore must behave the same way, so + // the filter sits on both paths rather than only the one a review flagged. + Assert.ThrowsExactly(() => menu.Run(() => throw new OutOfMemoryException())); + Assert.IsNull(menu.LastError); + } + + [TestMethod] + public void Run_StillAbsorbsAnOrdinaryRefusal() + { + TrayMenu menu = new("Demo", () => "idle", []); + + Assert.IsFalse(menu.Run(() => throw new InvalidOperationException("nope"))); + Assert.AreEqual("nope", menu.LastError); + } +} diff --git a/TrayApp.Test/TrayAppBuilderTests.cs b/TrayApp.Test/TrayAppBuilderTests.cs index c813a53..d563747 100644 --- a/TrayApp.Test/TrayAppBuilderTests.cs +++ b/TrayApp.Test/TrayAppBuilderTests.cs @@ -109,7 +109,7 @@ public async Task LoadPreferences_RestoresOnlyThePersistedToggles() .Build(); using DebouncedPreferenceStore store = new(provider, "tray", Debounce); - TrayAppBuilder.LoadPreferences(app, store); + _ = TrayAppBuilder.LoadPreferences(app, store); Assert.IsTrue(remembered); Assert.IsFalse(forgotten); @@ -137,7 +137,7 @@ public async Task LoadPreferences_ThenOptions_LetsAnExplicitSwitchWin() TrayAppDefinition app = builder.Build(); using DebouncedPreferenceStore store = new(provider, "tray", Debounce); - TrayAppBuilder.LoadPreferences(app, store); + _ = TrayAppBuilder.LoadPreferences(app, store); Assert.IsTrue(running, "the remembered value should be restored first"); Assert.IsTrue(CommandLineParser.TryParse(["--off"], app.Options, out CommandLineOptions options, out _)); @@ -147,6 +147,65 @@ public async Task LoadPreferences_ThenOptions_LetsAnExplicitSwitchWin() Assert.IsFalse(running); } + [TestMethod] + public async Task LoadPreferences_WhenTheToolRefuses_KeepsGoingAndHandsBackTheMessage() + { + InMemoryPersistenceProvider provider = new(); + + using (DebouncedPreferenceStore seed = new(provider, "tray", Debounce)) + { + await seed.LoadAsync().ConfigureAwait(false); + seed.Set("awake", true); + seed.Set("display", true); + await seed.FlushAsync().ConfigureAwait(false); + } + + bool display = false; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .Toggle("Keep awake", () => false, _ => throw new InvalidOperationException("no inhibitor here"), persistAs: "awake") + .Toggle("Keep display awake", () => display, value => display = value, persistAs: "display") + .Preferences(provider) + .Build(); + + using DebouncedPreferenceStore store = new(provider, "tray", Debounce); + + // Restoring "on" asks the tool to take an inhibitor, and the machine may have none to give. That + // refusal happens before there is a menu to fail into; letting it escape would end the process + // before the tray appeared. + string? error = TrayAppBuilder.LoadPreferences(app, store); + + Assert.AreEqual("no inhibitor here", error); + + // The toggle after the one that refused is still restored. + Assert.IsTrue(display); + } + + [TestMethod] + public async Task LoadPreferences_WhenNothingRefuses_HandsBackNoMessage() + { + InMemoryPersistenceProvider provider = new(); + + using (DebouncedPreferenceStore seed = new(provider, "tray", Debounce)) + { + await seed.LoadAsync().ConfigureAwait(false); + seed.Set("awake", true); + await seed.FlushAsync().ConfigureAwait(false); + } + + bool awake = false; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .Toggle("Keep awake", () => awake, value => awake = value, persistAs: "awake") + .Preferences(provider) + .Build(); + + using DebouncedPreferenceStore store = new(provider, "tray", Debounce); + + Assert.IsNull(TrayAppBuilder.LoadPreferences(app, store)); + Assert.IsTrue(awake); + } + [TestMethod] public void TryApplyOptions_WhenAHandlerRejectsTheValue_ReportsItAsAUsageError() { diff --git a/TrayApp.Test/TrayMenuTests.cs b/TrayApp.Test/TrayMenuTests.cs index 40ef800..446f965 100644 --- a/TrayApp.Test/TrayMenuTests.cs +++ b/TrayApp.Test/TrayMenuTests.cs @@ -122,6 +122,23 @@ public void Activate_WhenTheToolRefuses_KeepsTheMessageAndSurvivesTheRefresh() Assert.AreEqual("idle", menu.StatusItem.Header); } + [TestMethod] + public void SeedError_ShowsInTheStatusLineAndClearsOnTheNextSuccess() + { + TrayMenu menu = BuildMenu(() => "idle"); + + // A refusal while restoring a remembered toggle happens before the menu exists, so the host seeds it. + menu.SeedError("no inhibitor here"); + menu.Refresh(); + + Assert.AreEqual("no inhibitor here", menu.LastError); + Assert.AreEqual("Demo failed: no inhibitor here", menu.StatusItem!.Header); + + Assert.IsTrue(menu.Run(() => { })); + Assert.IsNull(menu.LastError); + Assert.AreEqual("idle", menu.StatusItem.Header); + } + [TestMethod] public void Activate_OnTheQuitItem_RaisesQuitRequested() { diff --git a/TrayApp/ConsoleRunner.cs b/TrayApp/ConsoleRunner.cs index 2c814df..e70facc 100644 --- a/TrayApp/ConsoleRunner.cs +++ b/TrayApp/ConsoleRunner.cs @@ -31,7 +31,7 @@ internal static class ConsoleRunner [SuppressMessage( "Design", "CA1031:Do not catch general exception types", - Justification = "The start action belongs to the consuming tool, so there is no exception type to filter on; reporting the refusal and exiting 1 is the contract the exit codes in --help describe.")] + Justification = "The start action belongs to the consuming tool, so there is no exception type to filter on beyond the fatal ones FatalError excludes; reporting the refusal and exiting 1 is the contract the exit codes in --help describe.")] internal static int Run(TrayAppDefinition app, CommandLineOptions options) { Ensure.NotNull(app); @@ -41,7 +41,7 @@ internal static int Run(TrayAppDefinition app, CommandLineOptions options) { app.OnStart?.Invoke(); } - catch (Exception ex) + catch (Exception ex) when (FatalError.IsNotFatal(ex)) { Console.Error.WriteLine($"{app.Name}: {ex.Message}"); return ExitFailure; diff --git a/TrayApp/FatalError.cs b/TrayApp/FatalError.cs new file mode 100644 index 0000000..6569d4e --- /dev/null +++ b/TrayApp/FatalError.cs @@ -0,0 +1,38 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TrayApp; + +using System; + +/// +/// Tells a failure the library is meant to absorb from one it must not. +/// +/// +/// +/// The library catches broadly on purpose wherever it runs the consuming tool's own code: a toggle setter, +/// a command, a start action. There is no exception type to filter on there - the tool's platform layer +/// throws whatever it throws - and the whole point of this library is that a refusal ends in a message +/// rather than in a dead process. +/// +/// +/// That reasoning stops at an exception which says the process is already lost. Reporting one in a menu and +/// carrying on presents a tray that cannot work, and buries the real cause under whatever fails next. +/// is absent because .NET does not let managed code catch it, and +/// is deliberately not fatal: it means one allocation was refused +/// before being attempted, not that the process is out of memory. +/// +/// +internal static class FatalError +{ + /// + /// Determines whether an exception is one the library may absorb. + /// + /// The exception the tool's code threw. + /// when it may be reported and survived. + internal static bool IsNotFatal(Exception exception) => exception switch + { + InsufficientMemoryException => true, + OutOfMemoryException or AccessViolationException => false, + _ => true, + }; +} diff --git a/TrayApp/Menu/TrayMenu.cs b/TrayApp/Menu/TrayMenu.cs index bf27ed1..023cea3 100644 --- a/TrayApp/Menu/TrayMenu.cs +++ b/TrayApp/Menu/TrayMenu.cs @@ -86,6 +86,16 @@ public TrayMenu(string displayName, Func? status, IEnumerable public string? LastError { get; private set; } + /// + /// Records a failure that happened before the menu existed. + /// + /// The message, or to clear it. + /// + /// Restoring a remembered toggle runs the tool's setter before there is a menu to fail into, and the + /// next successful action clears this the same way it clears a failed click. + /// + internal void SeedError(string? message) => LastError = message; + /// /// Re-reads every item's state from the tool. /// @@ -144,7 +154,7 @@ public bool Activate(TrayMenuItem item) [SuppressMessage( "Design", "CA1031:Do not catch general exception types", - Justification = "The action belongs to the consuming tool, so there is no exception type to filter on, and letting one escape a tray click handler takes down a process the user asked to keep running.")] + Justification = "The action belongs to the consuming tool, so there is no exception type to filter on beyond the fatal ones FatalError excludes, and letting one escape a tray click handler takes down a process the user asked to keep running.")] public bool Run(Action action) { Ensure.NotNull(action); @@ -156,7 +166,7 @@ public bool Run(Action action) action(); LastError = null; } - catch (Exception ex) + catch (Exception ex) when (FatalError.IsNotFatal(ex)) { LastError = ex.Message; Console.Error.WriteLine($"{displayName}: {ex.Message}"); diff --git a/TrayApp/TrayAppBuilder.cs b/TrayApp/TrayAppBuilder.cs index 2949473..0fa5684 100644 --- a/TrayApp/TrayAppBuilder.cs +++ b/TrayApp/TrayAppBuilder.cs @@ -383,6 +383,7 @@ public Task RunAsync(string[] args) bool useTray = UseTray(options); DebouncedPreferenceStore? store = null; + string? restoreError = null; try { @@ -391,7 +392,7 @@ public Task RunAsync(string[] args) if (useTray && app.Persistence is not null) { store = new DebouncedPreferenceStore(app.Persistence, app.PreferenceKey, app.PreferenceDebounce); - LoadPreferences(app, store); + restoreError = LoadPreferences(app, store); } if (!TryApplyOptions(options, out string optionError)) @@ -405,7 +406,7 @@ public Task RunAsync(string[] args) return Task.FromResult(ExitSuccess); } - int exitCode = useTray ? RunTray(app, options, store) : ConsoleRunner.Run(app, options); + int exitCode = useTray ? RunTray(app, options, store, restoreError) : ConsoleRunner.Run(app, options); // FinishAsync owns the store from here: it flushes whatever the run queued and disposes it. Task finishing = FinishAsync(exitCode, store); @@ -456,19 +457,40 @@ private static async Task FinishAsync(int exitCode, DebouncedPreferenceStor return exitCode; } + /// + /// Reads the remembered state and applies it to the toggles that asked to be remembered. + /// + /// The tool being run. + /// The store to read from. + /// The first refusal's message, for the menu's status line, or . /// + /// /// Blocking here is deliberate. The load has to finish before the menu is built, and it has to finish on /// the thread that goes on to start Avalonia - see the remarks on . There is no /// synchronization context in a console application, so there is nothing for this to deadlock against. + /// + /// + /// A setter can refuse here exactly as it can on a click - restoring "keep awake: on" asks the tool to + /// take an inhibitor, and the machine may have none to give. This runs before there is a menu to fail + /// into, so the refusal is caught, reported, and handed back for the status line. Letting it escape would + /// end the process before the tray appeared, which is the one thing this library exists to prevent: the + /// remaining toggles are still restored, and the tray still comes up saying why. + /// /// [SuppressMessage( "Reliability", "CA2007:Consider calling ConfigureAwait on the awaited task", Justification = "Not an await; the result is taken synchronously so that the tray still starts on the process's main thread.")] - internal static void LoadPreferences(TrayAppDefinition app, DebouncedPreferenceStore store) + [SuppressMessage( + "Design", + "CA1031:Do not catch general exception types", + Justification = "The setter belongs to the consuming tool, so there is no exception type to filter on beyond the fatal ones FatalError excludes, and a refusal here must not stop the tray from coming up.")] + internal static string? LoadPreferences(TrayAppDefinition app, DebouncedPreferenceStore store) { store.LoadAsync().GetAwaiter().GetResult(); + string? firstError = null; + foreach (TrayToggleItem toggle in app.Toggles) { if (toggle.PreferenceKey is null || store.Get(toggle.PreferenceKey) is not bool remembered) @@ -476,8 +498,18 @@ internal static void LoadPreferences(TrayAppDefinition app, DebouncedPreferenceS continue; } - toggle.Set(remembered); + try + { + toggle.Set(remembered); + } + catch (Exception ex) when (FatalError.IsNotFatal(ex)) + { + firstError ??= ex.Message; + Console.Error.WriteLine($"{app.Name}: {ex.Message}"); + } } + + return firstError; } /// @@ -507,9 +539,15 @@ internal static bool TryApplyOptions(CommandLineOptions options, out string erro "Design", "CA1031:Do not catch general exception types", Justification = "Any failure to bring up a tray icon is recoverable by falling back to the console, and the windowing backends do not report those failures through a common exception type.")] - private static int RunTray(TrayAppDefinition app, CommandLineOptions options, DebouncedPreferenceStore? store) + private static int RunTray( + TrayAppDefinition app, + CommandLineOptions options, + DebouncedPreferenceStore? store, + string? restoreError) { TrayMenu menu = new(app.DisplayName, app.Status, app.Items); + menu.SeedError(restoreError); + TrayApplication? host = null; void Remember(TrayToggleItem toggle)