fix: explain why a bulk install failed instead of printing its exit code - #1711
Merged
Conversation
Closes #1642. Every claim in it re-verified against current source first: the bare exit code at BulkInstallerViewModel.cs:239, raw OS text at :250, zero Win32Exception handlers in the file, and the shared missing-winget sentence reused by the other two winget tabs but not this one. Three tabs run winget and had drifted into three levels of care. Uninstaller translated its exit codes into sentences; App Updates caught the missing-winget case and reused Uninstaller's message; Bulk Installer did neither, writing "Failed (exit 1618)" into the row — a number that tells the target persona nothing and reads like a crash — or the raw Windows error text. The same underlying failure was explained on two tabs and shown as a code on the third. Fixed as the issue recommended: promote both mechanisms into one helper rather than add a third private copy, so a fourth caller cannot reintroduce the drift. UninstallerViewModel's translator now delegates (its signature and tests unchanged), AppUpdatesViewModel's const forwards, and there is a test asserting the VM and the helper return identical strings for every mapped code — two copies that merely agree today would silently diverge on the next edit. The install and uninstall maps are deliberately SEPARATE, and a test pins that. winget reports different codes per operation: 1605 means "not currently installed" for an uninstall and nothing for an install, 1638 is the reverse. One shared map would produce confidently wrong sentences, which is worse than a number. Install codes were researched rather than copied: the MSI set (1602/1603/1618/1619/1620/1638), access denied, and winget's own results — the last of which arrive as large unsigned values and previously surfaced as a huge negative number. Also added the Win32Exception handler the sibling tabs already have, for both the install and search paths. Without it, a PC with no App Installer got raw "The system cannot find the file specified" text per row, where the other tabs explain that App Installer needs installing from the Store. Verified: 18 checks against the built assembly (each mapped code, winget's own cancelled result, the unmapped fallback staying diagnosable, the two maps not bleeding into each other, the VM delegating, and the shared sentence being actionable) — 18/18, plus 11 new xUnit tests. The harness prints the resulting strings so the wording was read, not assumed. This release also ships the bandwidth reload gate, which landed on main under a ci: title in #1709 and therefore never got a version — hence its CHANGELOG entry here. Both projects build with 0 warnings and 0 errors; leak scan across all 32 patterns: zero hits.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1642. Every claim in the issue re-verified against current source first: the bare exit code at
BulkInstallerViewModel.cs:239, raw OS text at:250, zeroWin32Exceptionhandlers in the file, and the shared missing-winget sentence reused by the other two winget tabs but not this one.Three tabs, three levels of care
Failed (exit 1618)into the row, or the raw Windows error text.So the same underlying failure was explained on two tabs and shown as a number on the third. "Failed (exit 1618)" tells the target persona nothing and reads like a crash; the sentence for it was already written 200 lines away.
Before → after:
Promoted, not copied
Fixed the way the issue recommended: both mechanisms move into one
WingetFailurehelper rather than adding a third private copy, so a fourth caller can't reintroduce the drift.UninstallerViewModel.DescribeUninstallFailurenow delegates — signature and existing tests unchanged.AppUpdatesViewModel.WingetUnavailableMessageforwards to the shared const.The two maps stay separate, and a test pins that
winget reports different codes per operation:
1605means "not currently installed" for an uninstall and nothing for an install;1638is the reverse. One shared map would produce confidently wrong sentences — worse than a number.Install codes were researched rather than copied from the uninstall side: the MSI set (
1602/1603/1618/1619/1620/1638), access denied, and winget's own results. That last group arrives as large unsigned values and previously surfaced as a huge negative number in the row.Also: the missing
Win32ExceptionhandlerAdded to both the install and search paths, matching what the sibling tabs already do. Without it, a PC with no App Installer got raw
The system cannot find the file specifiedper row, where the other two tabs explain that App Installer needs installing from the Store.Verification
18 checks against the built assembly, plus 11 new xUnit tests. The harness prints the resulting strings, so the wording was read rather than assumed:
Covered: every mapped code, winget's own cancelled result, the unmapped fallback staying diagnosable (readable sentence that still carries the number), the two maps not bleeding into each other, the VM delegating, and the shared sentence naming both what to install and where.
Why this release also carries the bandwidth gate
The reload gate landed on main in #1709 under a
ci:title, soauto-releasecorrectly computed no version bump and it never shipped. Its CHANGELOG entry is here so the fix actually reaches users.Both projects build with 0 warnings, 0 errors. Leak scan across all 32 patterns: zero hits.