From b61ecf1afd2393144282fcff219710310338b9e5 Mon Sep 17 00:00:00 2001 From: Views2k <292889387+Views2k@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:58:17 +0000 Subject: [PATCH 1/2] Report completed updates, tune read failures and crash locations accurately An installer that ends with a failure code after Setup installed and registered the target version no longer turns a completed update into a failure: when the recorded target is the running build and the version Setup registered, the update is reported as installed. Tune read failures carry the methods on the failing stack, like crash reports, so each of the reader's checks is its own problem instead of one bare InvalidDataException. A crash signature includes the module Wisp captured at the fault when there is one, so a renderer, driver or hook fault no longer shares a signature with every fault Windows attributes to wisp.exe. Wisp Tools accepts the signature with or without that line. --- src/Wisp.App/ApplicationUpdateLauncher.cs | 32 +++++++++++++++- src/Wisp.App/CrashDiagnostics/CrashReport.cs | 21 ++++++---- .../Supplementary/SupplementaryIncident.cs | 7 ++++ src/Wisp.App/Tunes/TuneCaptureService.cs | 3 +- src/Wisp.Update/UpdateModels.cs | 5 +++ .../InstalledApplicationValidator.cs | 3 +- .../ApplicationUpdateResultTests.cs | 29 ++++++++++++++ .../SupplementaryIncidentTests.cs | 13 +++++++ .../TuneCaptureServiceIncidentTests.cs | 38 +++++++++++++++++++ .../InstalledApplicationValidatorTests.cs | 16 ++++++++ 10 files changed, 155 insertions(+), 12 deletions(-) create mode 100644 tests/Wisp.App.Tests/TuneCaptureServiceIncidentTests.cs diff --git a/src/Wisp.App/ApplicationUpdateLauncher.cs b/src/Wisp.App/ApplicationUpdateLauncher.cs index 230064ed..ad31cea1 100644 --- a/src/Wisp.App/ApplicationUpdateLauncher.cs +++ b/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? 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); diff --git a/src/Wisp.App/CrashDiagnostics/CrashReport.cs b/src/Wisp.App/CrashDiagnostics/CrashReport.cs index a8f7c09b..ce83aca2 100644 --- a/src/Wisp.App/CrashDiagnostics/CrashReport.cs +++ b/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(); var pending = new Queue(); @@ -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. diff --git a/src/Wisp.App/Supplementary/SupplementaryIncident.cs b/src/Wisp.App/Supplementary/SupplementaryIncident.cs index 2fa3121a..73eeb26d 100644 --- a/src/Wisp.App/Supplementary/SupplementaryIncident.cs +++ b/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) diff --git a/src/Wisp.App/Tunes/TuneCaptureService.cs b/src/Wisp.App/Tunes/TuneCaptureService.cs index f31dd56c..e8691625 100644 --- a/src/Wisp.App/Tunes/TuneCaptureService.cs +++ b/src/Wisp.App/Tunes/TuneCaptureService.cs @@ -298,7 +298,8 @@ 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), [])], + // The methods on the failing stack say which of the reader's checks failed; the message stays local. + 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) diff --git a/src/Wisp.Update/UpdateModels.cs b/src/Wisp.Update/UpdateModels.cs index 078089e3..1ade06e7 100644 --- a/src/Wisp.Update/UpdateModels.cs +++ b/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)) diff --git a/src/Wisp.Updater/InstalledApplicationValidator.cs b/src/Wisp.Updater/InstalledApplicationValidator.cs index 12341f67..7a51dd17 100644 --- a/src/Wisp.Updater/InstalledApplicationValidator.cs +++ b/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() { diff --git a/tests/Wisp.App.Tests/ApplicationUpdateResultTests.cs b/tests/Wisp.App.Tests/ApplicationUpdateResultTests.cs index ccc5776f..8caa4c1b 100644 --- a/tests/Wisp.App.Tests/ApplicationUpdateResultTests.cs +++ b/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() { diff --git a/tests/Wisp.App.Tests/SupplementaryIncidentTests.cs b/tests/Wisp.App.Tests/SupplementaryIncidentTests.cs index f25a50fb..073f345a 100644 --- a/tests/Wisp.App.Tests/SupplementaryIncidentTests.cs +++ b/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")))); diff --git a/tests/Wisp.App.Tests/TuneCaptureServiceIncidentTests.cs b/tests/Wisp.App.Tests/TuneCaptureServiceIncidentTests.cs new file mode 100644 index 00000000..eebd86a2 --- /dev/null +++ b/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(() => 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 destination) { destination.Fill(1); return true; } + } +} diff --git a/tests/Wisp.Updater.Tests/InstalledApplicationValidatorTests.cs b/tests/Wisp.Updater.Tests/InstalledApplicationValidatorTests.cs index 0eca8bc7..877b83cb 100644 --- a/tests/Wisp.Updater.Tests/InstalledApplicationValidatorTests.cs +++ b/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] From 64fba8e3696e9586ceeadf2aca11001abcf7f1bf Mon Sep 17 00:00:00 2001 From: Views2k <292889387+Views2k@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:03:56 +0000 Subject: [PATCH 2/2] Keep the tune incident comment off the argument list --- src/Wisp.App/Tunes/TuneCaptureService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Wisp.App/Tunes/TuneCaptureService.cs b/src/Wisp.App/Tunes/TuneCaptureService.cs index e8691625..ef470127 100644 --- a/src/Wisp.App/Tunes/TuneCaptureService.cs +++ b/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,6 @@ 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, - // The methods on the failing stack say which of the reader's checks failed; the message stays local. error is null ? null : SupplementaryIncidentCapture.Exceptions(CrashDiagnostics.CrashReport.DescribeExceptions(error)), SupplementaryIncidentCapture.Context(HealthContextRecorder.Current.Snapshot(), DateTimeOffset.UtcNow));