diff --git a/README.md b/README.md index 08a464e..0abeaf1 100644 --- a/README.md +++ b/README.md @@ -238,6 +238,8 @@ These are the parts that look like they could be simplified away and cannot. | Behaviour | What breaks without it | |-----------|------------------------| | Catching **all** exceptions out of the Avalonia run call, and using `HasStarted` to tell "the tray ran and quit" from "the tray never came up" | Avalonia's `DBusTrayIconImpl.WatchAsync` is `async void` with an exception filter that stops applying once the icon is disposed, so an ordinary Linux shutdown throws `TaskCanceledException` *after* the tray has done its job. Without the flag, a clean quit restarts in the console. | +| Tracking `ToolStarted` separately from `HasStarted`, and never falling back to the console once it is set | Start-up can still throw after the start action has run (a status getter, a missing icon resource). Falling back then starts the tool a second time and never stops the first start. | +| Re-seeding a failed preference restore after a successful start action | `TrayMenu.Run` clears `LastError` on success, so the restore failure would be wiped before the menu was ever painted. | | Falling back to the console when the windowing stack refuses after `DesktopSession` said yes | A `DISPLAY` pointing at nothing, a missing libX11, or no status-notifier host kills a process someone started to hold a resource. | | Constructing menu items eagerly and the `TrayIcon` lazily | `TrayIcon`'s constructor reaches for a platform handle that does not exist until Avalonia has started. | | Keeping a failed action's message in a field rather than writing it into the menu item | The refresh that runs immediately after every action overwrites anything written directly. | diff --git a/TrayApp.Test/TrayApplicationTests.cs b/TrayApp.Test/TrayApplicationTests.cs index bfe0e62..056bf29 100644 --- a/TrayApp.Test/TrayApplicationTests.cs +++ b/TrayApp.Test/TrayApplicationTests.cs @@ -5,6 +5,7 @@ namespace ktsu.TrayApp.Test; using System.Linq; using Avalonia.Controls; using ktsu.TrayApp; +using ktsu.TrayApp.Cli; using ktsu.TrayApp.Menu; using ktsu.TrayApp.Tray; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -237,4 +238,194 @@ public void TrayIconClick_WithNoToggleAtAll_DoesNothing() // of all. Assert.AreEqual(0, commandCalls); } + + [TestMethod] + public void StartTool_WhenTheStartActionSucceeds_KeepsTheSeededRestoreError() + { + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Status(() => "idle") + .Toggle("One", () => false, _ => { }) + .OnStart(() => { }) + .Build(); + + using TrayApplication host = Host(app, out TrayMenu menu); + menu.SeedError("no inhibitor"); + + host.StartTool(); + + // The remembered setting was not applied, and a successful start does not change that. Clearing the + // message here would put a healthy status line in front of the user before the menu is ever painted. + Assert.AreEqual("no inhibitor", menu.LastError); + Assert.AreEqual("Demo failed: no inhibitor", menu.StatusItem!.Header); + } + + [TestMethod] + public void StartTool_WhenTheStartActionFails_ReportsItsOwnError() + { + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Status(() => "idle") + .Toggle("One", () => false, _ => { }) + .OnStart(() => throw new InvalidOperationException("device busy")) + .Build(); + + using TrayApplication host = Host(app, out TrayMenu menu); + menu.SeedError("no inhibitor"); + + host.StartTool(); + + // The start refusal is the newer and more pressing failure, so it wins the one status line. + Assert.AreEqual("device busy", menu.LastError); + } + + [TestMethod] + public void Dispose_AfterTheToolStartedButBeforeTheTrayFinishedStarting_StopsTheTool() + { + int stops = 0; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Toggle("One", () => false, _ => { }) + .OnStart(() => { }) + .OnStop(() => stops++) + .Build(); + + TrayApplication host = Host(app, out _); + host.StartTool(); + host.Dispose(); + + // Whatever threw between the start action and the end of start-up, the tool is holding what it took, + // and nothing else will release it. + Assert.IsFalse(host.HasStarted); + Assert.AreEqual(1, stops); + } + + [TestMethod] + public void RecoverFromTrayFailure_AfterTheToolStarted_DoesNotStartItAgain() + { + int starts = 0; + int stops = 0; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Toggle("One", () => false, _ => { }) + .OnStart(() => starts++) + .OnStop(() => stops++) + .Build(); + + using TrayApplication host = Host(app, out _); + host.StartTool(); + + // What a missing icon resource or a throwing status getter looks like from RunTray: start-up threw + // after the start action had run, so HasStarted is still false. + int exitCode = TrayAppBuilder.RecoverFromTrayFailure( + app, + new CommandLineOptions { Duration = TimeSpan.Zero }, + host, + new InvalidOperationException("The tray icon 'nope-idle.png' is missing")); + + Assert.AreEqual(1, starts); + Assert.AreEqual(1, stops); + Assert.AreEqual(1, exitCode); + } + + [TestMethod] + public void RecoverFromTrayFailure_BeforeTheToolStarted_FallsBackToTheConsole() + { + int starts = 0; + int stops = 0; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Toggle("One", () => false, _ => { }) + .OnStart(() => starts++) + .OnStop(() => stops++) + .Build(); + + using TrayApplication host = Host(app, out _); + + int exitCode = TrayAppBuilder.RecoverFromTrayFailure( + app, + new CommandLineOptions { Duration = TimeSpan.Zero }, + host, + new InvalidOperationException("XOpenDisplay failed")); + + // The windowing stack refused before anything was started, so the terminal run is the only start. + Assert.AreEqual(1, starts); + Assert.AreEqual(1, stops); + Assert.AreEqual(0, exitCode); + } + + [TestMethod] + public void StartTool_WithNoStartAction_KeepsTheSeededErrorAndRecordsTheStart() + { + using TrayApplication host = Host(Definition(), out TrayMenu menu); + menu.SeedError("no inhibitor"); + + host.StartTool(); + + // There is no start action to succeed, so nothing clears the restore error; the tool still counts as + // started, so a later failure stops it rather than starting it again in the terminal. + Assert.IsTrue(host.ToolStarted); + Assert.AreEqual("no inhibitor", menu.LastError); + } + + [TestMethod] + public void RecoverFromTrayFailure_WithNoHost_FallsBackToTheConsole() + { + int starts = 0; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Toggle("One", () => false, _ => { }) + .OnStart(() => starts++) + .Build(); + + // Avalonia can refuse before it ever asks for the application, leaving no host to dispose. + int exitCode = TrayAppBuilder.RecoverFromTrayFailure( + app, + new CommandLineOptions { Duration = TimeSpan.Zero }, + host: null, + new InvalidOperationException("XOpenDisplay failed")); + + Assert.AreEqual(1, starts); + Assert.AreEqual(0, exitCode); + } + + [TestMethod] + public void RecoverFromTrayFailure_WithDebugOutputOn_StillStopsTheToolOnce() + { + int stops = 0; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Toggle("One", () => false, _ => { }) + .OnStop(() => stops++) + .Build(); + + using TrayApplication host = Host(app, out _); + host.StartTool(); + + string? previous = Environment.GetEnvironmentVariable("TRAYAPP_DEBUG"); + + try + { + Environment.SetEnvironmentVariable("TRAYAPP_DEBUG", "1"); + + int exitCode = TrayAppBuilder.RecoverFromTrayFailure( + app, + new CommandLineOptions { Duration = TimeSpan.Zero }, + host, + new InvalidOperationException("status getter threw")); + + // Printing the full exception is diagnostic only; it must not change what the run ends as. + Assert.AreEqual(1, exitCode); + Assert.AreEqual(1, stops); + } + finally + { + Environment.SetEnvironmentVariable("TRAYAPP_DEBUG", previous); + } + } } diff --git a/TrayApp/Tray/TrayApplication.cs b/TrayApp/Tray/TrayApplication.cs index e0b315d..f40f6ee 100644 --- a/TrayApp/Tray/TrayApplication.cs +++ b/TrayApp/Tray/TrayApplication.cs @@ -81,6 +81,16 @@ internal TrayApplication(TrayAppDefinition app, TrayMenu menu, TimeSpan? duratio /// internal bool HasStarted { get; private set; } + /// + /// Gets a value indicating whether the tool's start action has run. + /// + /// + /// Set before , and not the same thing: start-up can still throw after the start + /// action - a status getter, a subscription, a missing icon resource - and by then the tool is holding + /// whatever it took. stops it on this, and the caller must not start it again. + /// + internal bool ToolStarted { get; private set; } + /// public override void Initialize() => Name = app.DisplayName; @@ -102,10 +112,7 @@ public override void OnFrameworkInitializationCompleted() // Started here rather than before the run so that a tool whose start action throws still gets a tray // icon, with the refusal in its status line, instead of a process that exits before the user sees // anything. Run() puts the message where RefreshMenu will pick it up. - if (app.OnStart is not null) - { - menu.Run(app.OnStart); - } + StartTool(); // The tool's own state can change with nobody having clicked anything - a timer expiring, a device // going away - and the menu only ever reads through getters, so all it needs is a nudge. @@ -124,6 +131,34 @@ public override void OnFrameworkInitializationCompleted() base.OnFrameworkInitializationCompleted(); } + /// + /// Runs the tool's start action, putting a refusal in the status line. + /// + /// + /// Internal so that the start sequence can be driven without a platform: everything here reads and + /// writes menu state, and none of it needs the tray icon. + /// + internal void StartTool() + { + ToolStarted = true; + + if (app.OnStart is null) + { + return; + } + + // A failed preference restore was seeded before the menu existed, and a successful start does not + // make the remembered setting any more applied. Run() clears the error on success, so put it back; + // a failed start reports its own error instead, which is the more pressing of the two. + string? seeded = menu.LastError; + + if (menu.Run(app.OnStart) && seeded is not null) + { + menu.SeedError(seeded); + menu.Refresh(); + } + } + /// /// Stops the tray icon and runs the tool's stop action. /// @@ -139,7 +174,7 @@ public void Dispose() expiryTimer?.Stop(); menu.QuitRequested -= OnQuitRequested; - if (HasStarted) + if (ToolStarted) { app.OnStop?.Invoke(); } diff --git a/TrayApp/TrayAppBuilder.cs b/TrayApp/TrayAppBuilder.cs index 0fa5684..85299cf 100644 --- a/TrayApp/TrayAppBuilder.cs +++ b/TrayApp/TrayAppBuilder.cs @@ -43,6 +43,7 @@ namespace ktsu.TrayApp; public sealed class TrayAppBuilder { private const int ExitSuccess = 0; + private const int ExitFailure = 1; private const int ExitUsage = 2; /// Set to 1 to print the full exception behind a tray failure. @@ -568,30 +569,57 @@ void Remember(TrayToggleItem toggle) } catch (Exception ex) { - if (Environment.GetEnvironmentVariable(DebugEnvironmentVariable) == "1") - { - Console.Error.WriteLine(ex.ToString()); - } + return RecoverFromTrayFailure(app, options, host, ex); + } + } - // The tray already ran and quit; this is Avalonia's teardown throwing after the fact (see - // TrayApplication.HasStarted). Restarting in the terminal here would leave a tool the user just - // quit still holding whatever it holds. - if (host?.HasStarted == true) - { - host.Dispose(); - return ExitSuccess; - } + /// + /// Decides what a tray run that threw ends as. + /// + /// The tool being run. + /// The options the run was started with. + /// The tray host, or if Avalonia never constructed it. + /// What the run threw. + /// The process exit code. + internal static int RecoverFromTrayFailure( + TrayAppDefinition app, + CommandLineOptions options, + TrayApplication? host, + Exception ex) + { + if (Environment.GetEnvironmentVariable(DebugEnvironmentVariable) == "1") + { + Console.Error.WriteLine(ex.ToString()); + } - // Otherwise the windowing stack refused after the session check passed - a DISPLAY pointing at - // nothing, a missing libX11, no status-notifier host on the bus - and it reports those as plain - // exceptions of whatever type the backend felt like (X11 throws Exception("XOpenDisplay failed")). - // There is no useful list to filter on, and every one of them means the same thing here: no tray, - // so use the terminal. Letting it escape instead would kill a process the user asked to keep - // running. - host?.Dispose(); - Console.Error.WriteLine($"{app.Name}: could not show a tray icon ({ex.Message}). Staying in the terminal instead."); - return ConsoleRunner.Run(app, options); + // The tray already ran and quit; this is Avalonia's teardown throwing after the fact (see + // TrayApplication.HasStarted). Restarting in the terminal here would leave a tool the user just + // quit still holding whatever it holds. + if (host?.HasStarted == true) + { + host.Dispose(); + return ExitSuccess; } + + // The icon came up and the tool was started, then something later in start-up threw - a status + // getter, a subscription, a missing icon resource. Disposing stops the tool; falling back to the + // terminal would start it a second time, and say the icon could not be shown when it was. + if (host?.ToolStarted == true) + { + host.Dispose(); + Console.Error.WriteLine($"{app.Name}: the tray failed after starting ({ex.Message})."); + return ExitFailure; + } + + // Otherwise the windowing stack refused after the session check passed - a DISPLAY pointing at + // nothing, a missing libX11, no status-notifier host on the bus - and it reports those as plain + // exceptions of whatever type the backend felt like (X11 throws Exception("XOpenDisplay failed")). + // There is no useful list to filter on, and every one of them means the same thing here: no tray, + // so use the terminal. Letting it escape instead would kill a process the user asked to keep + // running. + host?.Dispose(); + Console.Error.WriteLine($"{app.Name}: could not show a tray icon ({ex.Message}). Staying in the terminal instead."); + return ConsoleRunner.Run(app, options); } ///