Fix ReportWindow crash when log directory is empty or missing#4338
Conversation
Co-authored-by: Jack251970 <53996452+Jack251970@users.noreply.github.com>
|
🥷 Code experts: no user but you matched threshold 10 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
There was a problem hiding this comment.
Pull request overview
Fixes a crash in ReportWindow when opening the appearance settings page and the version log directory is missing or contains no log files (resolving #4326).
Changes:
- Guard log discovery with directory-existence checks and
FirstOrDefault()to avoidSequence contains no elements. - Wrap log lookup in a
try/catchand only show the “upload log” prompt when a log file is found.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMade ReportWindow's log discovery and usage robust by adding try/catch around log retrieval, checking the logs directory exists, using FirstOrDefault instead of First, and guarding log usage with a null check. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 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)
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.
🧹 Nitpick comments (1)
Flow.Launcher/ReportWindow.xaml.cs (1)
51-53: Narrow the catch or broaden the comment.This currently swallows every exception type, while the comment says only IO errors are being ignored. Catch the expected filesystem exceptions here, or update the comment so it matches the behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/ReportWindow.xaml.cs` around lines 51 - 53, The catch block currently uses a broad catch (Exception) while the comment says only IO errors are intended to be ignored; narrow the catch to the filesystem-related exceptions (e.g., catch IOException and also consider UnauthorizedAccessException, PathTooLongException, DirectoryNotFoundException/FileNotFoundException) around the code that finds the log file in ReportWindow.xaml.cs, or alternatively update the comment to state that all exceptions are being swallowed — prefer narrowing the catch to the specific exceptions so only expected filesystem errors are ignored and other errors still surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Flow.Launcher/ReportWindow.xaml.cs`:
- Around line 51-53: The catch block currently uses a broad catch (Exception)
while the comment says only IO errors are intended to be ignored; narrow the
catch to the filesystem-related exceptions (e.g., catch IOException and also
consider UnauthorizedAccessException, PathTooLongException,
DirectoryNotFoundException/FileNotFoundException) around the code that finds the
log file in ReportWindow.xaml.cs, or alternatively update the comment to state
that all exceptions are being swallowed — prefer narrowing the catch to the
specific exceptions so only expected filesystem errors are ignored and other
errors still surface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35867006-a3ef-48b9-aa96-22902a4a52bd
📒 Files selected for processing (1)
Flow.Launcher/ReportWindow.xaml.cs
* Initial plan * Fix ReportWindow crash when log directory is empty or doesn't exist Co-authored-by: Jack251970 <53996452+Jack251970@users.noreply.github.com> * Improve code comments Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Resolve #4326
Summary by cubic
Summary of changes
Fixes a crash in ReportWindow when the log directory is missing, empty, or unreadable by safely handling IO errors and absent log files. The UI only prompts to upload a log when a log file exists.
Bug Fixes
SetExceptionnow verifies directory existence and usesFirstOrDefault()for the latest log.Written for commit b1d98f6. Summary will update on new commits.