Updated chkdsk script for better logging and error handling TOOLING…#115
Updated chkdsk script for better logging and error handling TOOLING…#115anmocanu wants to merge 2 commits into
Conversation
…59932 Refactored the script to improve logging and error handling. Ensured that the return statement is outside the try/catch/finally blocks. https://dev.azure.com/Azure-VM-POD/Verticals/_workitems/edit/59932
.VERSION
v1.1: [May 2026] - Updated the script again (current)
- Fixed breaking exception when the Hyper-V module is not installed on the host.
- Added explicit checking via Get-Module before executing nested VM discovery.
- Included advanced Gen2 unlettered EFI fallback and dynamic drive-letter assignment.
v1.0: Initial commit. This was the version 1.0 of the script.
|
This PR also addresses #68 |
There was a problem hiding this comment.
Issues Found
🔴 Bug: else block misplaced — Hyper-V skip message outside the if check
if ($hyperVModuleAvailable -and (Get-Command -Name 'Get-VM' ...)) {
# ... VM handling ...
}
else {
Log-Info "Hyper-V module/cmdlets not available on this host -> skipping nested VM discovery"
}This else is paired with the Hyper-V check, but it's immediately followed by the partition enumeration code outside any block scope. The structure is technically correct — the else just logs and continues — but visually it looks like the partition logic might be inside the else. This is fine functionally, but adding a blank line or comment separator after the else block would improve readability.
Severity: Not a bug, just readability. Low.
🟡 Concern: No check for chkdsk exit code
$chkdskResults = chkdsk $letter /f 2>&1After running chkdsk, the script parses the output text for summary lines but doesn't check $LASTEXITCODE. chkdsk returns specific exit codes:
0= no errors found1= errors found and fixed2= disk cleanup performed3= could not check the disk
The script should check $LASTEXITCODE and set $script_final_status = $STATUS_ERROR if chkdsk reports unfixable corruption (exit code 3).
🟡 Concern: $letter colon handling
$letter = $partition.DriveLetter
if ($letter -notmatch ":") { $letter = "$letter" + ":" }Get-Disk-Partitions returns DriveLetter as a single character (e.g., C), so this always appends :. The -notmatch ":" check is defensive but may be unnecessary complexity. Consider just always formatting: $letter = "$($partition.DriveLetter):".
🟢 Minor: Summary regex may miss important lines
if ($str -match '(no problems|correcting|replacing|deleting|recovering|inserting|truncating|adjusting|resetting|Windows has|No further action|Cleaning up|could not fix|Errors detected|corrupt|found no)') {This regex covers many cases but could miss localized Windows installations where chkdsk output is in a different language. This is a known limitation and not unique to this PR, but worth documenting.
🟢 Minor: Add-Content for log vs Tee-Object pattern
The script uses Add-Content -Path $logFile for chkdsk output but the rest of the repo's scripts use Tee-Object -FilePath $logFile -Append. This inconsistency is understandable (separating stdout from log-only content is intentional) but could confuse future maintainers. Consider a brief inline comment explaining why Add-Content is used here instead of Tee-Object.
… 59932
Refactored the script to improve logging and error handling. Ensured that the return statement is outside the try/catch/finally blocks. https://dev.azure.com/Azure-VM-POD/Verticals/_workitems/edit/59932