Audit follow-ups: keyboard, wording, performance, transfer, sync, retry and relay fixes #22

Merged
jaap-jan merged 21 commits from claude/audit-followups into main 2026-10-02 12:59:56 +00:00
6 changed files with 154 additions and 5 deletions
Showing only changes of commit 37e9d74938 - Show all commits

No files matched your search

+4
View File
@@ -48,6 +48,10 @@ internal sealed partial class DodoSshApp : Application
{ {
if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop) 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(); ListenForOpenedLinks();
ShowSomethingWhileComposing(desktop); ShowSomethingWhileComposing(desktop);
} }
@@ -0,0 +1,85 @@
using DodoSSH.Client.Session;
namespace DodoSSH.Client.App.Platform;
/// <summary>
/// Writes down why the application died, in the profile, before it does.
/// </summary>
/// <remarks>
/// <para>
/// 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.
/// </para>
/// <para>
/// 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.
/// </para>
/// </remarks>
internal static class CrashLog
{
/// <summary>Past this, the next entry starts a fresh file.</summary>
private const long MaxBytes = 1024 * 1024;
private static readonly Lock Gate = new();
private static string? file;
/// <summary>Starts recording, for the process's two last-chance hooks.</summary>
/// <remarks>
/// The UI thread's own hook is added by the application once Avalonia is running — see
/// <see cref="RecordFromTheUiThread"/> — because the dispatcher must not be touched before the platform
/// has set it up.
/// </remarks>
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();
};
}
/// <summary>The UI thread's hook: records the exception and lets it take the process as before.</summary>
internal static void RecordFromTheUiThread(Exception exception) => Write("UI thread", exception);
/// <summary>Appends one entry; never throws.</summary>
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.
}
}
}
}
+3
View File
@@ -75,6 +75,9 @@ internal static class Program
LaunchLink = link; LaunchLink = link;
// After the claim, so a second launch that only hands its link over writes nothing.
CrashLog.Install(paths);
BuildAvaloniaApp().StartWithClassicDesktopLifetime(args); BuildAvaloniaApp().StartWithClassicDesktopLifetime(args);
} }
@@ -122,6 +122,9 @@ public sealed record ClientPaths(string DataDirectory)
/// </remarks> /// </remarks>
public string InstanceLockFile => Path.Combine(DataDirectory, "instance.lock"); public string InstanceLockFile => Path.Combine(DataDirectory, "instance.lock");
/// <summary>Where the application writes down why it died, when something nothing caught ends it.</summary>
public string CrashLogFile => Path.Combine(DataDirectory, "crash.log");
/// <summary>Creates the profile directory if it is not there yet.</summary> /// <summary>Creates the profile directory if it is not there yet.</summary>
/// <remarks> /// <remarks>
/// Separate from resolving the path, because resolving must never have a side effect: it is read /// Separate from resolving the path, because resolving must never have a side effect: it is read
@@ -2133,12 +2133,25 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
return; return;
} }
await preferences.SetHiddenAsync(row.VaultId, hiding, CancellationToken.None) try
.ConfigureAwait(true); {
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 // The lists first, then the switches: rebuilding the switches is what redraws the menu, and doing
// second means the menu and the screen behind it never disagree, even for a frame. // it second means the menu and the screen behind it never disagree, even for a frame.
await open.RefreshVaultsAsync(CancellationToken.None).ConfigureAwait(true); 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(); RebuildVaultToggles();
@@ -0,0 +1,41 @@
using DodoSSH.Client.App.Platform;
using DodoSSH.Client.Session;
namespace DodoSSH.Client.App.Tests;
/// <summary>
/// That the application leaves a record of why it died.
/// </summary>
/// <remarks>
/// 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.
/// </remarks>
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));
}
}