fix: avoid startup crash when saved appdata path is unavailable#259
fix: avoid startup crash when saved appdata path is unavailable#259nickogl wants to merge 1 commit into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new appdata path validation layer is added to PathManager that normalizes persisted paths, filters default values, and validates directory accessibility through a helper method. A Windows-only test ensures unavailable network paths are properly rejected with error reporting. ChangesAppdata Path Validation with Accessibility Checking
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@WheelWizard.Test/Features/Settings/PathManagerTests.cs`:
- Around line 21-24: GetUnavailableWindowsPath currently returns a UNC host path
which can hang on DNS resolution; change it to return a deterministic unused
local drive-letter path so failures are immediate. Update the
GetUnavailableWindowsPath method to return a path like
$@"Z:\WheelWizardTests\{Guid.NewGuid():N}" (or better: pick an unused drive
letter by checking DriveInfo.GetDrives() and return the first free letter)
instead of the UNC \\nonexistent-host\... value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6d359d4-6869-4a86-99df-0ee15a9eff84
📒 Files selected for processing (2)
WheelWizard.Test/Features/Settings/PathManagerTests.csWheelWizard/Services/PathManager.cs
Purpose of this PR:
WheelWizard could crash on startup at logger initialization when a previously saved custom data folder points to an unavailable path (for example after external drive letter changes). This PR fixes the root cause by making appdata-path resolution resilient.
How to Test:
HKCU\Software\WheelWizard\AppDataLocationto a path on an unavailable drive (for exampleZ:\...whereZ:does not exist).What Has Been Changed:
PathManagernow ignores persisted custom appdata paths that cannot be created/accessed and falls back to default appdata path for startup.PathManagerAPI.Checklist before merging
Discussion points
Logger initialization failures did not surface anywhere but the Windows event logs (at least for me), which makes diagnosis harder for end users. A follow-up improvement could be either to add a fallback sink or improve the way this error surfaces to the end user.
Summary by CodeRabbit
Tests
Bug Fixes