Skip to content

190 bug winexportlogps1 cannot export logs - #192

Merged
Magnetarman merged 6 commits into
Devfrom
190-bug-winexportlogps1-cannot-export-logs
Oct 3, 2026
Merged

Magnetarman merged 6 commits into
Devfrom
190-bug-winexportlogps1-cannot-export-logs

Conversation

@Magnetarman

Copy link
Copy Markdown
Owner

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:Spinners was never initialized in production code. The line

$Global:Spinners = '⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏'.ToCharArray()

was lost when the monolithic WinToolkit-template.ps1 was split into wintoolkit-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.Length evaluates to 0, so $spinnerIndex % 0 threw a RuntimeException on 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 as interrotto per Timeout because the synthetic ExitCode = -1 matched the timeout heuristic. chkdsk/sfc/DISM were also left running unattended, since the exception escaped before the existing kill path.
  • WinExportLog — the -Job branch of Invoke-WithSpinner swallowed the error and returned a Hashtable, so -eq $true was false and the tool always reported Errore sconosciuto: il file ZIP non è stato creato. The compression job was killed by the finally { Remove-Job } block, so the export silently produced nothing — the tool was unusable exactly when needed to ship diagnostics.
  • Every other tool calling Invoke-WithSpinner (-Command, -Timer, -Process, -Job) was equally affected, and the GUI was too: the crash sits before the GuiSessionActive short-circuit in Write-ProgressUpdate.

Changes.

  1. Restored $Global:Spinners in the globals block of wintoolkit-modules/10-Module.Localization.ps1 (its historical position, immediately before $Global:MsgStyles), with a comment explaining why it must stay.
  2. Added Get-SpinnerChar to wintoolkit-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 in Uninstall-Office.ps1.
  3. Terminate the child process in the catch of Invoke-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) to 0, 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/for guards or an existing -gt 0 check), confirmed by reading each call site. No other unguarded division or $Global:Spinners indexing remains in tools/, the core modules or the GUI shim.

Fixes #190


Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • ♻️ Refactoring (no functional change)
  • 🧹 Cleanup / Maintenance
  • 📖 Documentation

Files Modified

  • wintoolkit-modules/10-Module.Localization.ps1 — restored the missing $Global:Spinners frame-set initialization (root cause)
  • wintoolkit-modules/20-Module.UI.ps1 — added Get-SpinnerChar, a null/empty-safe spinner accessor that degrades to ''
  • wintoolkit-modules/60-Module.Processes.ps1 — routed the 4 unguarded spinner call sites through Get-SpinnerChar; kill the child process in the catch to avoid orphaned processes
  • tools/Uninstall-Office.ps1 — replaced the inline if ($Global:Spinners) fallback with the shared Get-SpinnerChar helper
  • tools/WinBackupDriver.ps1 — guarded the compression-ratio division against a rounded-to-zero $totalSizeMB (second, independent divide-by-zero)
  • WinToolkit.ps1 — regenerated by compiler.ps1 only (not hand-edited); CI rebuilds this artifact automatically

Tests & Verification

  • Verified locally via the relevant build/test command
  • If CI/CD changed, validated the affected workflow/action and updated the tracking document
  • Execution logs attached (snippet or screenshot)

Commands run locally (PowerShell 7.6.6, Windows 11 24H2):

& .\compiler.ps1                             -> 15 modules, 0 errors, exit code 0
& .\.github\scripts\Test-CompiledScript.ps1  -> ALL TESTS PASSED, 0 errors, 1 warning

Functional verification against the compiled WinToolkit.ps1:

# Test Result
1 Spinners initialized by the core 10 frames, first ⠋
2 External command via Invoke-ExternalCommandWithLog Success=True, ExitCode=0, StdOut=[ok]
3 Invoke-WithSpinner -Job (the WinExportLog path) Boolean True → -eq $true now true
4 Invoke-WithSpinner -Timer / -Command no exception
5 Regression: Spinners = $null and empty array returns '', command still Success=True
6 Get-SpinnerChar cycling on a 3-frame set ABCAB, index advances correctly
7 WinBackupDriver edge case $totalSizeMB = 0 ratio 0, no exception (was Attempted to divide by zero.)

Before the fix, the same test reproduced the reported failure:

[ERROR] Errore durante Compressione dei log in corso...: Attempted to divide by zero.
[ERROR] Errore sconosciuto: il file ZIP non è stato creato.
Log / Screenshot
# WinRepairToolkit_2026-10-03_15-16-31.log (BEFORE)
[15:16:31] [ERROR] Eccezione durante l'esecuzione del comando esterno | Context:
{"Command":"chkdsk","Exception":"Attempted to divide by zero.",
 "Stack":"in Invoke-ExternalCommandWithLog, <No file>: riga 779
          in Invoke-WithSpinner, <No file>: riga 865 ..."}
[15:16:31] [SUCCESS] Controllo disco NON completato (interrotto per Timeout).
[15:16:39] [WARNING] 5 errori rilevati. Nuovo tentativo.

# WinExportLog_2026-09-30_11-24-06.log (BEFORE)
[11:24:07] [ERROR] Errore durante Compressione dei log in corso...:
            Attempted to divide by zero.
[11:24:07] [ERROR] Errore sconosciuto: il file ZIP non è stato creato.

# AFTER (local verification on the compiled script)
=== Spinners definiti dal core: 10 | Primo: [⠋] ===
-Cmd  -> Success=True ExitCode=0 StdOut=[ok]
-Timer-> OK (nessuna eccezione)
-Job  -> Tipo=Boolean Valore=True ('-eq $true')=True
WinBackupDriver edge case: total=0 -> ratio=0  OK
Get-SpinnerChar: null -> [] | vuoto -> [] | ABC x5 -> ABCAB (indice=5)

compiler.ps1 -> Process: 15 | Error : 0 | code 0
Test-CompiledScript.ps1 -> ALL TESTS PASSED! | Errors: 0 | Warnings: 1

The single remaining warning ("Encoding: Without BOM") is pre-existing and declared
acceptable by the test script. Its final $env:GITHUB_OUTPUT error only occurs
locally, as that variable exists solely inside GitHub Actions.


Notes for the maintainer

Two items to handle before merging:

  1. WinToolkit.ps1 must not be committed. It was regenerated locally with
    compiler.ps1 for verification only; CI produces it automatically. Drop it from
    the commit (git restore WinToolkit.ps1) so the checklist rule holds.
  2. Branch name. The current branch is
    190-bug-winexportlogps1-cannot-export-logs, which does not match any pattern
    that triggers the pipeline (BUG/*, ENHANCEMENT/*, FEATURE/*, GUI/*,
    Changes/*). Renaming to BUG/divide-by-zero-spinner would enable the
    lightweight CI pipeline. Target branch is Dev, as required.

Checklist

Missing a checkbox or violating a rule will result in automatic PR rejection.

  • PR targets Dev unless this is an authorized maintainer release change
  • Atomic change: one issue or one feature per PR
  • WinToolkit.ps1 not modified manually (handled by CI automation)
  • Generated artifacts (WinToolkit.ps1, start-core.ps1) are not edited manually
  • Changes to .github/, start.ps1 or start-modules/ include the required maintainer review
  • Existing code style respected, no debug code left behind
  • Clear commits in Italian, max 72 characters per line

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

👀 Heads Up — Manual Review Required

Hi @Magnetarman,
thanks for your contribution! This PR was accepted and can proceed normally through the review flow.

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

  • wintoolkit-modules/10-Module.Localization.ps1
  • wintoolkit-modules/20-Module.UI.ps1
  • wintoolkit-modules/60-Module.Processes.ps1

Why this notice?

These files are not blocked for contributors, but they require additional verification by the maintainer before merging, because unexpected changes could:

  • Alter the interface behaviour (WinToolkit_GUI.ps1, start.ps1)
  • Impact compilation or the framework core functions (wintoolkit-modules/)
  • Introduce unverified resources into the distribution (assets/)

Next steps

No 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.

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

👀 Heads Up — Manual Review Required

Hi @Magnetarman,
thanks for your contribution! This PR was accepted and can proceed normally through the review flow.

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

  • wintoolkit-modules/10-Module.Localization.ps1
  • wintoolkit-modules/20-Module.UI.ps1
  • wintoolkit-modules/60-Module.Processes.ps1

Why this notice?

These files are not blocked for contributors, but they require additional verification by the maintainer before merging, because unexpected changes could:

  • Alter the interface behaviour (WinToolkit_GUI.ps1, start.ps1)
  • Impact compilation or the framework core functions (wintoolkit-modules/)
  • Introduce unverified resources into the distribution (assets/)

Next steps

No 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.

@Magnetarman
Magnetarman merged commit abef322 into Dev Oct 3, 2026
8 checks passed
@Magnetarman
Magnetarman deleted the 190-bug-winexportlogps1-cannot-export-logs branch October 3, 2026 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant