Skip to content

fix(checkup): Catch Get-MpPreference exception#6584

Open
Ninja3047 wants to merge 1 commit intoScoopInstaller:developfrom
Ninja3047:fix-mp-exception
Open

fix(checkup): Catch Get-MpPreference exception#6584
Ninja3047 wants to merge 1 commit intoScoopInstaller:developfrom
Ninja3047:fix-mp-exception

Conversation

@Ninja3047
Copy link
Copy Markdown

@Ninja3047 Ninja3047 commented Jan 13, 2026

Description

Catches Get-MpPreference error and returns gracefully

Motivation and Context

Closes: #6583

How Has This Been Tested?

Tested by running scoop checkup on my machine.

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error handling in Windows Defender diagnostic checks to improve reliability when retrieving security preferences and exclusion settings. The diagnostic tool now gracefully handles retrieval failures and provides enhanced debug information for troubleshooting, preventing false negatives due to temporary retrieval errors.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 13, 2026

Walkthrough

The change adds error handling to Get-MpPreference calls in diagnostic.ps1 to gracefully handle CimException errors when Windows Defender cannot be accessed (e.g., when another antivirus is running). Instead of throwing an unhandled exception, the code now catches the error, logs debug information, and returns a pass status.

Changes

Cohort / File(s) Summary
Error handling for Defender diagnostics
lib/diagnostic.ps1
Wrapped Get-MpPreference calls in try/catch block with explicit -ErrorAction Stop; added catch handler for Microsoft.Management.Infrastructure.CimException that logs debug info and returns $true; moved ExclusionPath retrieval inside try block for consistent error handling

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 When Defender won't speak, don't let errors leak,
A try-catch so neat makes the chaos less bleak,
Now CimExceptions rest, while diagnostics pass the test,
With grace and finesse, our checkup's the best! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(checkup): Catch Get-MpPreference exception' accurately describes the main change—adding exception handling for Get-MpPreference calls in the diagnostic module.
Linked Issues check ✅ Passed The PR successfully addresses issue #6583 by implementing try/catch blocks around Get-MpPreference calls to handle CimException, preventing unhandled exceptions during scoop checkup.
Out of Scope Changes check ✅ Passed All changes are focused on adding exception handling to lib/diagnostic.ps1 for Get-MpPreference calls, directly addressing the linked issue without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
lib/diagnostic.ps1 (1)

9-29: Consider caching Get-MpPreference result to avoid duplicate calls.

Get-MpPreference is called twice (lines 10 and 16). This is inefficient and could fail on the second call even if the first succeeded. Store the result in a variable.

♻️ Suggested refactor
     if (Test-CommandAvailable Get-MpPreference) {
         try {
-            if ((Get-MpPreference -Erroraction Stop).DisableRealtimeMonitoring) { return $true }
+            $mpPreference = Get-MpPreference -ErrorAction Stop
+            if ($mpPreference.DisableRealtimeMonitoring) { return $true }
             if ($defender -and $defender.Status) {
                 if ($defender.Status -eq [System.ServiceProcess.ServiceControllerStatus]::Running) {
                     $installPath = $scoopdir;
                     if ($global) { $installPath = $globaldir; }

-                    $exclusionPath = (Get-MpPreference -Erroraction Stop).ExclusionPath
+                    $exclusionPath = $mpPreference.ExclusionPath
                     if (!($exclusionPath -contains $installPath)) {
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd2e93d and 062f004.

📒 Files selected for processing (1)
  • lib/diagnostic.ps1
🔇 Additional comments (1)
lib/diagnostic.ps1 (1)

26-28: Good error handling for the reported issue.

The catch block correctly handles CimException (HRESULT 0x800106ba) that occurs when Windows Defender is unavailable. Returning $true is appropriate since the check cannot complete and should not block other diagnostics.

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