diff --git a/src/DodoSSH.Client.App/App.axaml.cs b/src/DodoSSH.Client.App/App.axaml.cs index b49c046..0849859 100644 --- a/src/DodoSSH.Client.App/App.axaml.cs +++ b/src/DodoSSH.Client.App/App.axaml.cs @@ -48,6 +48,10 @@ internal sealed partial class DodoSshApp : Application { if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop) { + // Recorded, not handled: see CrashLog for why the process still goes. + Avalonia.Threading.Dispatcher.UIThread.UnhandledException += + (_, e) => CrashLog.RecordFromTheUiThread(e.Exception); + ListenForOpenedLinks(); ShowSomethingWhileComposing(desktop); } diff --git a/src/DodoSSH.Client.App/Platform/CrashLog.cs b/src/DodoSSH.Client.App/Platform/CrashLog.cs new file mode 100644 index 0000000..4168e4f --- /dev/null +++ b/src/DodoSSH.Client.App/Platform/CrashLog.cs @@ -0,0 +1,85 @@ +using DodoSSH.Client.Session; + +namespace DodoSSH.Client.App.Platform; + +/// +/// Writes down why the application died, in the profile, before it does. +/// +/// +/// +/// An exception nothing caught used to close the window and leave nothing behind — no file, no event, no +/// sentence — so the only report anybody could make was "it closed". This does not keep the process alive: +/// an exception that reached the top is one no code was written to recover from, and carrying on in whatever +/// state it left would be a guess. What it adds is the record. +/// +/// +/// Bounded, and appended to rather than rewritten, so a run of crashes keeps its history without the file +/// growing for ever. Every write is best-effort: a profile that cannot be written to is a reason to lose the +/// log, never a reason to fail harder. +/// +/// +internal static class CrashLog +{ + /// Past this, the next entry starts a fresh file. + private const long MaxBytes = 1024 * 1024; + + private static readonly Lock Gate = new(); + + private static string? file; + + /// Starts recording, for the process's two last-chance hooks. + /// + /// The UI thread's own hook is added by the application once Avalonia is running — see + /// — because the dispatcher must not be touched before the platform + /// has set it up. + /// + internal static void Install(ClientPaths paths) + { + ArgumentNullException.ThrowIfNull(paths); + + file = paths.CrashLogFile; + + AppDomain.CurrentDomain.UnhandledException += (_, e) => Write("Unhandled", e.ExceptionObject as Exception); + + // A fire-and-forget task that faulted. It does not end the process, which is exactly why it needs + // writing down: the work it was doing simply never happened, and nothing else would ever say so. + TaskScheduler.UnobservedTaskException += (_, e) => + { + Write("Unobserved task", e.Exception); + e.SetObserved(); + }; + } + + /// The UI thread's hook: records the exception and lets it take the process as before. + internal static void RecordFromTheUiThread(Exception exception) => Write("UI thread", exception); + + /// Appends one entry; never throws. + internal static void Write(string where, Exception? exception) + { + if (file is not { } path) + { + return; + } + + var entry = $"{TimeProvider.System.GetUtcNow():O} {where}{Environment.NewLine}{exception}{Environment.NewLine}{Environment.NewLine}"; + + lock (Gate) + { + try + { + Directory.CreateDirectory(Path.GetDirectoryName(path)!); + + if (File.Exists(path) && new FileInfo(path).Length > MaxBytes) + { + File.Delete(path); + } + + File.AppendAllText(path, entry); + } + catch (Exception failure) when (failure is IOException or UnauthorizedAccessException) + { + // Best-effort, by design: see the type's remarks. + } + } + } +} diff --git a/src/DodoSSH.Client.App/Program.cs b/src/DodoSSH.Client.App/Program.cs index 54c4d15..2d26c22 100644 --- a/src/DodoSSH.Client.App/Program.cs +++ b/src/DodoSSH.Client.App/Program.cs @@ -75,6 +75,9 @@ internal static class Program LaunchLink = link; + // After the claim, so a second launch that only hands its link over writes nothing. + CrashLog.Install(paths); + BuildAvaloniaApp().StartWithClassicDesktopLifetime(args); } diff --git a/src/DodoSSH.Client.Session/ClientPaths.cs b/src/DodoSSH.Client.Session/ClientPaths.cs index e441592..fff2503 100644 --- a/src/DodoSSH.Client.Session/ClientPaths.cs +++ b/src/DodoSSH.Client.Session/ClientPaths.cs @@ -122,6 +122,9 @@ public sealed record ClientPaths(string DataDirectory) /// public string InstanceLockFile => Path.Combine(DataDirectory, "instance.lock"); + /// Where the application writes down why it died, when something nothing caught ends it. + public string CrashLogFile => Path.Combine(DataDirectory, "crash.log"); + /// Creates the profile directory if it is not there yet. /// /// Separate from resolving the path, because resolving must never have a side effect: it is read diff --git a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs index 05f8f9a..fca3614 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs @@ -2133,12 +2133,25 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp return; } - await preferences.SetHiddenAsync(row.VaultId, hiding, CancellationToken.None) - .ConfigureAwait(true); + try + { + await preferences.SetHiddenAsync(row.VaultId, hiding, CancellationToken.None) + .ConfigureAwait(true); - // The lists first, then the switches: rebuilding the switches is what redraws the menu, and doing it - // second means the menu and the screen behind it never disagree, even for a frame. - await open.RefreshVaultsAsync(CancellationToken.None).ConfigureAwait(true); + // The lists first, then the switches: rebuilding the switches is what redraws the menu, and doing + // it second means the menu and the screen behind it never disagree, even for a frame. + await open.RefreshVaultsAsync(CancellationToken.None).ConfigureAwait(true); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + // A command's exception is rethrown on the UI thread, where nothing catches it — so a cache that + // could not be written to, or a vault locked halfway through, used to close the application. The + // switches are redrawn from what is actually stored, so the menu does not claim the change. + RebuildVaultToggles(); + StatusMessage = $"Could not change which vaults are shown: {exception.Message}"; + + return; + } RebuildVaultToggles(); diff --git a/tests/DodoSSH.Client.App.Tests/CrashLogTests.cs b/tests/DodoSSH.Client.App.Tests/CrashLogTests.cs new file mode 100644 index 0000000..23ac65f --- /dev/null +++ b/tests/DodoSSH.Client.App.Tests/CrashLogTests.cs @@ -0,0 +1,41 @@ +using DodoSSH.Client.App.Platform; +using DodoSSH.Client.Session; + +namespace DodoSSH.Client.App.Tests; + +/// +/// That the application leaves a record of why it died. +/// +/// +/// An exception nothing caught used to close the window and leave nothing at all behind. Installing the hooks +/// here also installs them in the test process, which is harmless: they only write to this test's own +/// directory, and only when something reaches the top. +/// +public sealed class CrashLogTests : IDisposable +{ + private readonly string directory = + Path.Combine(Path.GetTempPath(), $"dodossh-crash-{Guid.CreateVersion7():N}"); + + public void Dispose() + { + if (Directory.Exists(directory)) + { + Directory.Delete(directory, recursive: true); + } + } + + [Fact] + public void AnExceptionThatReachesTheTop_IsWrittenToTheProfile() + { + var paths = new ClientPaths(directory); + + CrashLog.Install(paths); + CrashLog.Write("UI thread", new InvalidOperationException("the thing nobody caught")); + + var written = File.ReadAllText(paths.CrashLogFile); + + written.ShouldContain("UI thread"); + written.ShouldContain("the thing nobody caught"); + written.ShouldContain(nameof(InvalidOperationException)); + } +}