Report completed updates, tune read failures and crash locations accurately

src/Wisp.App/ApplicationUpdateLauncher.cs

@@ -287,7 +287,8 @@ internal static bool TryConsumeResult(out ApplicationUpdateResultStatus result)
 
     internal static bool TryConsumeResult(
         string localApplicationDataPath,
-        out ApplicationUpdateResultStatus result)
+        out ApplicationUpdateResultStatus result,
+        Func<string?>? registeredVersion = null)
     {
         result = default;
         var resultPath = ResultPath(localApplicationDataPath);
@@ -322,6 +323,19 @@ internal static bool TryConsumeResult(
                 return true;
             }
 
+            if (record.State == "failed" && record.TargetVersion is { } completedVersion &&
+                completedVersion == ApplicationVersionInfo.MachineVersion &&
+                (registeredVersion ?? RegisteredVersion)() == completedVersion)
+            {
+                // The helper judged the installer by its exit code alone. When the target build is the one
+                // running and Setup registered it, the files were installed and the update completed, even
+                // though the installer process itself ended with a failure code.
+                result = new ApplicationUpdateResultStatus(
+                    $"Wisp {completedVersion} was installed successfully.",
+                    "Check again", "success", SourceVersion: record.SourceVersion, TargetVersion: completedVersion);
+                return true;
+            }
+
             if (record.State == "failed")
             {
                 // Do not render text from the helper record. A fixed local message
@@ -387,6 +401,22 @@ record = fresh;
         return new(incident, HelperExitStatus(reason, permanent), permanent);
     }
 
+    // The version Setup registered for this user's installation, or null when it cannot be read.
+    private static string? RegisteredVersion()
+    {
+        try
+        {
+            using var key = Microsoft.Win32.Registry.CurrentUser.OpenSubKey(UpdateApplyContract.UninstallRegistryKey, writable: false);
+            return key?.GetValue("DisplayVersion") is string text && SemanticVersion.TryParse(text, out var version)
+                ? version.ToString()
+                : null;
+        }
+        catch (Exception exception) when (exception is System.Security.SecurityException or IOException or UnauthorizedAccessException)
+        {
+            return null;
+        }
+    }
+
     private static string ResultPath(string localApplicationDataPath) =>
         Path.Combine(localApplicationDataPath, "Wisp", ResultFileName);
 

src/Wisp.App/CrashDiagnostics/CrashReport.cs

@@ -32,7 +32,18 @@ internal sealed record CrashReport(Guid Id, DateTimeOffset TimeUtc, string WispV
     // A short local-only remark from the operation that recovered, such as which process held a locked file.
     public string? Note { get; init; }
 
-    internal static CrashReport Capture(Exception? exception, CrashOrigin origin, bool terminating, DateTimeOffset now)
+    internal static CrashReport Capture(Exception? exception, CrashOrigin origin, bool terminating, DateTimeOffset now) =>
+        new(Guid.NewGuid(), now.ToUniversalTime(), ApplicationVersionInfo.MachineVersion,
+            ApplicationVersionInfo.DiagnosticBuildId, Environment.Version.ToString(), origin, terminating, DescribeExceptions(exception))
+        {
+            ModuleVersionId = typeof(App).Assembly.ManifestModule.ModuleVersionId,
+            WindowsVersion = Environment.OSVersion.Version.ToString(),
+            ProcessArchitecture = RuntimeInformation.ProcessArchitecture
+        };
+
+    // The exception chain as types, HRESULTs and the methods on each stack; operation failures that are
+    // not crashes use the same projection so their reports can name where they failed.
+    internal static CrashExceptionInfo[] DescribeExceptions(Exception? exception)
     {
         var errors = new List<CrashExceptionInfo>();
         var pending = new Queue<Exception>();
@@ -52,13 +63,7 @@ internal static CrashReport Capture(Exception? exception, CrashOrigin origin, bo
             }
             else if (error.InnerException is { } inner) pending.Enqueue(inner);
         }
-        return new(Guid.NewGuid(), now.ToUniversalTime(), ApplicationVersionInfo.MachineVersion,
-            ApplicationVersionInfo.DiagnosticBuildId, Environment.Version.ToString(), origin, terminating, errors.ToArray())
-        {
-            ModuleVersionId = typeof(App).Assembly.ManifestModule.ModuleVersionId,
-            WindowsVersion = Environment.OSVersion.Version.ToString(),
-            ProcessArchitecture = RuntimeInformation.ProcessArchitecture
-        };
+        return errors.ToArray();
     }
 
     // Revalidate disk content before showing or copying it, even though only safe fields are written.

src/Wisp.App/Supplementary/SupplementaryIncident.cs

@@ -182,7 +182,14 @@ internal static string Signature(SupplementaryIncident value)
                 text.Append(component.Component).Append('\n').Append(component.Reason).Append('\n')
                     .Append(component.Code.ToString(CultureInfo.InvariantCulture)).Append('\n');
         if (value.SchemaVersion == 3 && value.Crash is { } crash)
+        {
             text.Append(crash.Origin).Append('\n').Append(crash.Module ?? "").Append('\n');
+            // Windows names the process module (wisp.exe, which carries the .NET runtime) for most faults;
+            // the module Wisp captured at the fault separates a renderer, driver or hook fault from a
+            // runtime one. Its offset changes with every build, so it stays out. Wisp Tools accepts the
+            // signature with or without this line, so reports from earlier versions keep their grouping.
+            if (crash.FaultModule is { Length: > 0 } faultModule) text.Append(faultModule).Append('\n');
+        }
         if (value.SchemaVersion == 3 && value.Update is { } update)
             text.Append(update.InstallerExitCode?.ToString(CultureInfo.InvariantCulture) ?? "").Append('\n');
         if (value.SchemaVersion == 3 && value.Recorder?.Resolution is { } resolution)

src/Wisp.App/Tunes/TuneCaptureService.cs

@@ -288,6 +288,7 @@ System.ComponentModel.Win32Exception or DllNotFoundException or EntryPointNotFou
         }
     }
 
+    // The methods on the failing stack say which of the reader's checks failed; the message stays local.
     internal static SupplementaryIncident TuneIncident(TuneCaptureStage stage, string reason, Exception? error = null) =>
         SupplementaryIncidentSchema.Create("tune", reason, stage switch
         {
@@ -298,7 +299,7 @@ internal static SupplementaryIncident TuneIncident(TuneCaptureStage stage, strin
             _ => "tune-current"
         }, error is System.ComponentModel.Win32Exception native ? unchecked((uint)native.NativeErrorCode) :
             error is not null ? unchecked((uint)error.HResult) : null,
-        error is null ? null : [new(CrashDiagnostics.CrashReportSymbols.ExceptionType(error.GetType()), unchecked((uint)error.HResult), [])],
+        error is null ? null : SupplementaryIncidentCapture.Exceptions(CrashDiagnostics.CrashReport.DescribeExceptions(error)),
         SupplementaryIncidentCapture.Context(HealthContextRecorder.Current.Snapshot(), DateTimeOffset.UtcNow));
 
     private TuneCaptureResult Fail(TuneCaptureStatus status, string message, long generation, string details, SupplementaryIncident? incident = null)

src/Wisp.Update/UpdateModels.cs

@@ -77,6 +77,11 @@ public static class UpdateApplyContract
     // The same request fails the same way again, so the app stops automatic attempts for the session.
     public const int InvalidRequestExitCode = 2;
 
+    // The installer's registration under the current user, named by its AppId in installer/Wisp.iss.
+    // Setup writes DisplayVersion after it has installed the files.
+    public const string UninstallRegistryKey =
+        @"Software\Microsoft\Windows\CurrentVersion\Uninstall\{A8FC0D58-11E3-4B25-B78D-3B98E9855473}_is1";
+
     public static string CreateReadyEventName(string token)
     {
         if (!IsValidReadyToken(token))

src/Wisp.Updater/InstalledApplicationValidator.cs

@@ -16,8 +16,7 @@ internal interface IInstallationRegistrationReader
 
 internal sealed class WindowsInstallationRegistrationReader : IInstallationRegistrationReader
 {
-    private const string UninstallKey =
-        @"Software\Microsoft\Windows\CurrentVersion\Uninstall\{A8FC0D58-11E3-4B25-B78D-3B98E9855473}_is1";
+    private const string UninstallKey = Wisp.Update.UpdateApplyContract.UninstallRegistryKey;
 
     public string? ReadInstallLocation()
     {

tests/Wisp.App.Tests/ApplicationUpdateResultTests.cs

@@ -120,6 +120,35 @@ public void MatchingRunningBuildConfirmsInstalledOutcome()
         Assert.False(ApplicationUpdateLauncher.TryConsumeResult(layout.Root, out _));
     }
 
+    [Fact]
+    public void FailedInstallerExitForTheRunningRegisteredTargetIsAnInstalledUpdate()
+    {
+        var target = ApplicationVersionInfo.MachineVersion;
+        string Record(string recorded) =>
+            $$"""{"schemaVersion":1,"state":"failed","sourceVersion":"1.0.0","targetVersion":"{{recorded}}","errorCode":"UPDATE_INSTALLER_EXIT_FAILED","recoveryState":"restarted","installerExitCode":1073807364}""";
+
+        // The installer ended with a failure code after Setup installed and registered the target.
+        using (var layout = new ResultLayout(Record(target)))
+        {
+            Assert.True(ApplicationUpdateLauncher.TryConsumeResult(layout.Root, out var result, () => target));
+            Assert.Equal("success", result.Outcome);
+            Assert.Null(result.Incident);
+            Assert.Equal(target, result.TargetVersion);
+            Assert.Equal($"Wisp {target} was installed successfully.", result.Status);
+            Assert.False(File.Exists(layout.ResultPath));
+        }
+
+        // Setup did not register the target, or the target is not the running build: the update did not complete.
+        foreach (var (registered, recorded) in new[] { ("1.0.0", target), ((string?)null, target), (target, "9.9.9") })
+        {
+            using var layout = new ResultLayout(Record(recorded));
+            Assert.True(ApplicationUpdateLauncher.TryConsumeResult(layout.Root, out var result, () => registered));
+            Assert.Equal("failure", result.Outcome);
+            Assert.Equal("update-installer-exit-failed", result.Incident?.Reason);
+            Assert.Equal(1073807364u, result.Incident?.Update?.InstallerExitCode);
+        }
+    }
+
     [Fact]
     public void InvalidResultIsConsumedWithoutBeingDisplayed()
     {

tests/Wisp.App.Tests/SupplementaryIncidentTests.cs

@@ -15,6 +15,19 @@ public void CrashSignatureIncludesOriginAndModuleButNotLookupOrChangingContext()
         Assert.True(SupplementaryIncidentSchema.Valid(incident));
         Assert.Equal(incident.Signature, SupplementaryIncidentSchema.Signature(incident with { Crash = incident.Crash! with { Lookup = "timeout" } }));
         Assert.NotEqual(incident.Signature, SupplementaryIncidentSchema.Signature(incident with { Crash = incident.Crash! with { Module = "dxgi.dll" } }));
+        // The module Wisp captured at the fault splits faults Windows attributes to the same module; its offset does not.
+        var native = incident with { Crash = incident.Crash! with { Module = "wisp.exe", FaultModule = "dxgi.dll", FaultOffset = 0x15f2 } };
+        var renderer = native with { Crash = native.Crash! with { FaultModule = "wisp.nativerenderer.dll" } };
+        Assert.NotEqual(SupplementaryIncidentSchema.Signature(native), SupplementaryIncidentSchema.Signature(renderer));
+        Assert.Equal(SupplementaryIncidentSchema.Signature(native),
+            SupplementaryIncidentSchema.Signature(native with { Crash = native.Crash! with { FaultOffset = 0x450f } }));
+        // Without a captured fault module the signature is the one earlier versions sent; with one, the module
+        // is one more line. Wisp Tools' incident signature test uses the same text.
+        static string Hash(string text) => Convert.ToHexString(System.Security.Cryptography.SHA256.HashData(System.Text.Encoding.UTF8.GetBytes(text))).ToLowerInvariant();
+        Assert.Equal(3, incident.SchemaVersion);
+        Assert.Equal(Hash("wisp-tools-incident-v3\nwindows-fault\nwindows-application-fault\ncrash-windows\n\nwindows\nntdll.dll\n"), incident.Signature);
+        Assert.Equal(Hash("wisp-tools-incident-v3\nwindows-fault\nwindows-application-fault\ncrash-windows\n\nwindows\nwisp.exe\ndxgi.dll\n"),
+            SupplementaryIncidentSchema.Signature(native));
         Assert.False(SupplementaryIncidentSchema.Valid(incident with { SchemaVersion = 1 }));
         Assert.False(SupplementaryIncidentSchema.Valid(SupplementaryIncidentSchema.Create("windows-fault", crash: new("ui"))));
         Assert.False(SupplementaryIncidentSchema.Valid(SupplementaryIncidentSchema.Create("windows-fault", crash: new("windows", Module: "C:\\PRIVATE\\ntdll.dll"))));

tests/Wisp.App.Tests/TuneCaptureServiceIncidentTests.cs

@@ -0,0 +1,38 @@
+using System.IO;
+using Wisp.App.Supplementary;
+using Wisp.App.Tunes;
+using Xunit;
+
+namespace Wisp.App.Tests;
+
+public sealed class TuneCaptureServiceIncidentTests
+{
+    [Fact]
+    public void TuneReadFailureNamesTheReaderMethodThatFailedWithoutItsMessage()
+    {
+        var read = new NativeTuneRead(new Memory(), CancellationToken.None, maximumBytes: 4);
+        _ = read.Bytes(0x10000, 4);
+        var error = Assert.Throws<InvalidDataException>(() => read.Bytes(0x10000, 1));
+
+        var incident = TuneCaptureService.TuneIncident(TuneCaptureStage.ReadTune, "tune-read-failed", error);
+
+        var exception = Assert.Single(incident.Exceptions!.Value);
+        Assert.Equal("InvalidDataException", exception.Type);
+        Assert.Contains(SupplementaryIncidentSchema.MethodId("Wisp.App.Tunes.NativeTuneRead.Bytes"), exception.Methods);
+        Assert.True(SupplementaryIncidentSchema.Valid(incident));
+        Assert.DoesNotContain("exceeded its bounds", System.Text.Json.JsonSerializer.Serialize(incident), StringComparison.Ordinal);
+
+        // Different checks are different problems: the failing method is part of the signature.
+        var elsewhere = TuneCaptureService.TuneIncident(TuneCaptureStage.ReadTune, "tune-read-failed", new InvalidDataException("other"));
+        Assert.NotEqual(incident.Signature, elsewhere.Signature);
+    }
+
+    private sealed class Memory : IReadOnlyProcessMemory
+    {
+        public bool TryReadByte(ulong address, out byte value) { value = 0; return false; }
+        public bool TryReadUInt32(ulong address, out uint value) { value = 0; return false; }
+        public bool TryReadUInt64(ulong address, out ulong value) { value = 0; return false; }
+        public bool TryReadSingle(ulong address, out float value) { value = 0; return false; }
+        public bool TryReadBytes(ulong address, Span<byte> destination) { destination.Fill(1); return true; }
+    }
+}

tests/Wisp.Updater.Tests/InstalledApplicationValidatorTests.cs

@@ -2,6 +2,22 @@
 
 namespace Wisp.Updater.Tests;
 
+public sealed class InstallationRegistrationKeyTests
+{
+    [Fact]
+    public void RegistrationKeyIsTheInstallersAppId()
+    {
+        var directory = new DirectoryInfo(AppContext.BaseDirectory);
+        while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "Wisp.sln"))) directory = directory.Parent;
+        Assert.NotNull(directory);
+        var script = File.ReadAllText(Path.Combine(directory.FullName, "installer", "Wisp.iss"));
+        var appId = System.Text.RegularExpressions.Regex.Match(script, @"^AppId=\{\{([0-9A-F-]{36})\}", System.Text.RegularExpressions.RegexOptions.Multiline);
+        Assert.True(appId.Success);
+        Assert.Equal(Wisp.Update.UpdateApplyContract.UninstallRegistryKey,
+            $@"Software\Microsoft\Windows\CurrentVersion\Uninstall\{{{appId.Groups[1].Value}}}_is1");
+    }
+}
+
 public sealed class InstalledApplicationValidatorTests
 {
     [Fact]