Repository navigation
190 bug winexportlogps1 cannot export logs - #192
Conversation
👀 Heads Up — Manual Review RequiredHi @Magnetarman, However, the automated check detected that one or more sensitive WinToolkit files were modified, which affect the user interface, the main template or the project's static resources: Sensitive files modified
Why this notice?These files are not blocked for contributors, but they require additional verification by the maintainer before merging, because unexpected changes could:
Next stepsNo action is required from you. The maintainer will review the changes with extra care before merging. Automated notification from the WinToolkit CI/CD system — PR #192. |
👀 Heads Up — Manual Review RequiredHi @Magnetarman, However, the automated check detected that one or more sensitive WinToolkit files were modified, which affect the user interface, the main template or the project's static resources: Sensitive files modified
Why this notice?These files are not blocked for contributors, but they require additional verification by the maintainer before merging, because unexpected changes could:
Next stepsNo action is required from you. The maintainer will review the changes with extra care before merging. Automated notification from the WinToolkit CI/CD system — PR #192. |
Description
Fixes a regression introduced during the modularization refactor that broke every toolkit operation animated by a spinner, by throwing
Attempted to divide by zero.Root cause. The global frame set
$Global:Spinnerswas never initialized in production code. The linewas lost when the monolithic
WinToolkit-template.ps1was split intowintoolkit-modules/*. The only remaining assignment was inside.github/tests/WinToolkit.Tests.ps1, which is why CI stayed green while every real run failed.Every spinner loop indexed the frame set with
% $Global:Spinners.Length. In PowerShell$null.Lengthevaluates to0, so$spinnerIndex % 0threw aRuntimeExceptionon the first tick of the loop — i.e. immediately after$proc.Start().Impact. The blast radius was much wider than a single tool:
WinRepairToolkit— all 6 repair commands failed, retried 3 times identically, and were misreported asinterrotto per Timeoutbecause the syntheticExitCode = -1matched the timeout heuristic.chkdsk/sfc/DISMwere also left running unattended, since the exception escaped before the existing kill path.WinExportLog— the-Jobbranch ofInvoke-WithSpinnerswallowed the error and returned aHashtable, so-eq $truewas false and the tool always reportedErrore sconosciuto: il file ZIP non è stato creato. The compression job was killed by thefinally { Remove-Job }block, so the export silently produced nothing — the tool was unusable exactly when needed to ship diagnostics.Invoke-WithSpinner(-Command,-Timer,-Process,-Job) was equally affected, and the GUI was too: the crash sits before theGuiSessionActiveshort-circuit inWrite-ProgressUpdate.Changes.
$Global:Spinnersin the globals block ofwintoolkit-modules/10-Module.Localization.ps1(its historical position, immediately before$Global:MsgStyles), with a comment explaining why it must stay.Get-SpinnerChartowintoolkit-modules/20-Module.UI.ps1— a safe DRY accessor that degrades to''when the frame set is missing or empty, so this class of failure can never again abort a real operation. Applied it to all four unguarded call sites, following the pattern already used inUninstall-Office.ps1.catchofInvoke-ExternalCommandWithLog, preventing orphaned repair processes on any early failure.Also fixed — second, independent divide-by-zero in
tools/WinBackupDriver.ps1. The guard checked that backup files existed, not that they weighed anything: a payload below ~5 KB rounds[Math]::Round(x/1MB, 2)to0, making the compression-ratio division throw the exact same error and aborting the whole backup.Audit. All 15 scripts in
tools/were reviewed. The folder contains only 5 division operations; the other 4 are already safe (foreach/forguards or an existing-gt 0check), confirmed by reading each call site. No other unguarded division or$Global:Spinnersindexing remains intools/, the core modules or the GUI shim.Fixes #190
Type of Change
Files Modified
wintoolkit-modules/10-Module.Localization.ps1— restored the missing$Global:Spinnersframe-set initialization (root cause)wintoolkit-modules/20-Module.UI.ps1— addedGet-SpinnerChar, a null/empty-safe spinner accessor that degrades to''wintoolkit-modules/60-Module.Processes.ps1— routed the 4 unguarded spinner call sites throughGet-SpinnerChar; kill the child process in thecatchto avoid orphaned processestools/Uninstall-Office.ps1— replaced the inlineif ($Global:Spinners)fallback with the sharedGet-SpinnerCharhelpertools/WinBackupDriver.ps1— guarded the compression-ratio division against a rounded-to-zero$totalSizeMB(second, independent divide-by-zero)WinToolkit.ps1— regenerated bycompiler.ps1only (not hand-edited); CI rebuilds this artifact automaticallyTests & Verification
Commands run locally (PowerShell 7.6.6, Windows 11 24H2):
Functional verification against the compiled
WinToolkit.ps1:⠋Invoke-ExternalCommandWithLogSuccess=True, ExitCode=0, StdOut=[ok]Invoke-WithSpinner -Job(theWinExportLogpath)Boolean True→-eq $truenow trueInvoke-WithSpinner -Timer/-CommandSpinners = $nulland empty array'', command stillSuccess=TrueGet-SpinnerCharcycling on a 3-frame setABCAB, index advances correctlyWinBackupDriveredge case$totalSizeMB = 00, no exception (wasAttempted to divide by zero.)Before the fix, the same test reproduced the reported failure:
Log / Screenshot
The single remaining warning ("Encoding: Without BOM") is pre-existing and declared
acceptable by the test script. Its final
$env:GITHUB_OUTPUTerror only occurslocally, as that variable exists solely inside GitHub Actions.
Notes for the maintainer
Two items to handle before merging:
WinToolkit.ps1must not be committed. It was regenerated locally withcompiler.ps1for verification only; CI produces it automatically. Drop it fromthe commit (
git restore WinToolkit.ps1) so the checklist rule holds.190-bug-winexportlogps1-cannot-export-logs, which does not match any patternthat triggers the pipeline (
BUG/*,ENHANCEMENT/*,FEATURE/*,GUI/*,Changes/*). Renaming toBUG/divide-by-zero-spinnerwould enable thelightweight CI pipeline. Target branch is
Dev, as required.Checklist
Devunless this is an authorized maintainer release changeWinToolkit.ps1not modified manually (handled by CI automation)WinToolkit.ps1,start-core.ps1) are not edited manually.github/,start.ps1orstart-modules/include the required maintainer review