-
-
Notifications
You must be signed in to change notification settings - Fork 457
Handle recoverable DWM composition exceptions gracefully #4113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Updated the `ErrorReporting` class to log recoverable DWM composition exceptions instead of displaying the "Report problem UI," addressing issues with crashes in `PresentationFramework.dll`. Introduced a new `ExceptionHelper` class to encapsulate logic for identifying recoverable DWM composition exceptions based on specific HRESULT values and stack trace patterns. Added detailed comments for context and referenced a related GitHub issue. Improved modularity and maintainability by isolating exception handling logic in the `ExceptionHelper` class.
|
🥷 Code experts: Yusyuriv 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. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds detection for recoverable DWM (Desktop Window Manager) COMExceptions and skips showing the ReportWindow for those cases by introducing an internal helper and using it in ErrorReporting to return early for identified recoverable DWM exceptions. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ErrorReporting
participant ExceptionHelper
participant ReportWindow
App->>ErrorReporting: Exception thrown
Note over ErrorReporting: Log fatal exception
ErrorReporting->>ExceptionHelper: IsRecoverableDwmCompositionException(e)?
alt Recoverable DWM exception (true)
ExceptionHelper-->>ErrorReporting: true
Note over ErrorReporting: Return early — skip ReportWindow
else Non-recoverable (false)
ExceptionHelper-->>ErrorReporting: false
ErrorReporting->>ReportWindow: Create & show report UI
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (7)📓 Common learnings📚 Learning: 2025-07-06T12:21:37.947ZApplied to files:
📚 Learning: 2025-07-21T09:19:49.684ZApplied to files:
📚 Learning: 2025-07-21T09:19:19.012ZApplied to files:
📚 Learning: 2025-08-13T06:12:43.382ZApplied to files:
📚 Learning: 2025-09-06T05:32:51.575ZApplied to files:
📚 Learning: 2025-07-20T07:28:28.092ZApplied to files:
🪛 GitHub Check: Check SpellingFlow.Launcher/Helper/ExceptionHelper.cs[warning] 40-40: [warning] 20-20: [warning] 40-40: [warning] 20-20: ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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.
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 (2)
Flow.Launcher/Helper/ErrorReporting.cs (1)
20-25: DWM guard behavior looks right; consider simplifying the helper call.The new guard correctly keeps logging while suppressing the report UI for recoverable DWM composition COMExceptions, which aligns with the PR goal. One small readability tweak:
ExceptionHelper.IsRecoverableDwmCompositionExceptionalready handles non‑COM exceptions internally via pattern matching, so the explicitas COMExceptioncast is redundant and slightly narrows future flexibility (e.g., if you later want to inspect wrapper exceptions inside the helper).You could simplify to:
- if (ExceptionHelper.IsRecoverableDwmCompositionException(e as System.Runtime.InteropServices.COMException)) return; + if (ExceptionHelper.IsRecoverableDwmCompositionException(e)) + { + return; + }This preserves behavior today and keeps the helper usage straightforward.
Flow.Launcher/Helper/ExceptionHelper.cs (1)
1-44: Helper logic matches the described DWM COMException patterns; consider tests and possible inner‑exception handling.The detection logic for recoverable DWM composition exceptions looks solid: you constrain on COMException, check the two specific HRESULTs, gate one on
Source == "PresentationFramework", and fall back to theDwmCompositionChangedstack‑trace pattern. That matches the linked issue and the intent to avoid raising the report UI for these crashes.Two small, non‑blocking improvements you might want to consider:
Unit tests for the helper branches
Creating targeted tests for:
DWM_E_COMPOSITIONDISABLEDSTATUS_MESSAGE_LOST_HR+ PresentationFramework source- Stack‑trace pattern only
- Non‑matching COMException and non‑COM exceptions
would help prevent regressions if more patterns or HRESULTs are added later.
Future‑proofing for wrapped exceptions (optional)
Right now, only top‑level COMExceptions are treated as recoverable. If WPF or other infrastructure ever starts wrapping these in another exception type (e.g.,
XamlParseExceptionwith an inner COMException), you may want to extend this helper to optionally walkInnerException/AggregateExceptiontrees. Not necessary for the current bug, but something to keep in mind.Overall, the implementation is clear and localized, which should make future tuning straightforward.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher/Helper/ErrorReporting.cs(1 hunks)Flow.Launcher/Helper/ExceptionHelper.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3572
File: Flow.Launcher/Helper/PluginInstallationHelper.cs:251-252
Timestamp: 2025-06-29T08:31:07.816Z
Learning: In Flow Launcher's PluginInstallationHelper, when the progress box encounters an exception during download (indicated by reportProgress becoming null), the retry logic intentionally downloads without a progress handler to avoid repeating the same progress reporting failure. This prevents cascading exceptions in the progress reporting mechanism.
📚 Learning: 2025-07-06T12:21:37.947Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Applied to files:
Flow.Launcher/Helper/ExceptionHelper.csFlow.Launcher/Helper/ErrorReporting.cs
📚 Learning: 2025-06-29T08:31:07.816Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3572
File: Flow.Launcher/Helper/PluginInstallationHelper.cs:251-252
Timestamp: 2025-06-29T08:31:07.816Z
Learning: In Flow Launcher's PluginInstallationHelper, when the progress box encounters an exception during download (indicated by reportProgress becoming null), the retry logic intentionally downloads without a progress handler to avoid repeating the same progress reporting failure. This prevents cascading exceptions in the progress reporting mechanism.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
🧬 Code graph analysis (1)
Flow.Launcher/Helper/ErrorReporting.cs (1)
Flow.Launcher/Helper/ExceptionHelper.cs (2)
ExceptionHelper(10-44)IsRecoverableDwmCompositionException(22-43)
🪛 GitHub Check: Check Spelling
Flow.Launcher/Helper/ExceptionHelper.cs
[warning] 42-42:
Dwm is not a recognized word. (unrecognized-spelling)
[warning] 22-22:
Dwm is not a recognized word. (unrecognized-spelling)
[warning] 42-42:
Dwm is not a recognized word. (unrecognized-spelling)
[warning] 22-22:
Dwm is not a recognized word. (unrecognized-spelling)
Flow.Launcher/Helper/ErrorReporting.cs
[warning] 25-25:
Dwm is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CodeQL analysis (csharp)
- GitHub Check: Agent
- GitHub Check: gitStream.cm
- GitHub Check: build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves error handling for WPF DWM composition exceptions by introducing graceful recovery instead of displaying error UI. The changes isolate exception detection logic in a new ExceptionHelper class and update ErrorReporting to silently log recoverable DWM composition exceptions.
- Introduced
ExceptionHelperclass with logic to identify recoverable DWM composition exceptions based on HRESULT values (0x80263001, 0xD0000701) and stack trace patterns - Modified
ErrorReporting.Report()to check for recoverable DWM exceptions before displaying error UI - Added detailed comments explaining the issue context and linking to the WPF source code where these exceptions originate
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Flow.Launcher/Helper/ExceptionHelper.cs | New class providing static method to identify recoverable DWM composition COMExceptions by checking HRESULT codes and stack traces |
| Flow.Launcher/Helper/ErrorReporting.cs | Updated to check for recoverable DWM exceptions and silently log them instead of showing error dialog, preventing UI hangs during WPF composition changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/Helper/ErrorReporting.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
📚 Learning: 2025-07-06T12:21:37.947Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
📚 Learning: 2025-06-29T08:31:07.816Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3572
File: Flow.Launcher/Helper/PluginInstallationHelper.cs:251-252
Timestamp: 2025-06-29T08:31:07.816Z
Learning: In Flow Launcher's PluginInstallationHelper, when the progress box encounters an exception during download (indicated by reportProgress becoming null), the retry logic intentionally downloads without a progress handler to avoid repeating the same progress reporting failure. This prevents cascading exceptions in the progress reporting mechanism.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
📚 Learning: 2025-09-05T11:56:27.267Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/HotkeyControlDialog.xaml:6-6
Timestamp: 2025-09-05T11:56:27.267Z
Learning: In Flow.Launcher's migration to iNKORE.UI.WPF.Modern UI framework, dialog resource keys like PopuBGColor, PopupButtonAreaBGColor, PopupButtonAreaBorderColor, and PopupTextColor are provided by the iNKORE.UI.WPF.Modern framework itself, not defined locally in the codebase theme files.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
📚 Learning: 2025-09-28T03:59:59.693Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Plugins/Flow.Launcher.Plugin.Explorer/Views/RenameFile.xaml.cs:75-80
Timestamp: 2025-09-28T03:59:59.693Z
Learning: In Flow.Launcher's Explorer plugin rename dialog (RenameFile.xaml.cs), the dialog should close unconditionally after calling RenameThing.Rename(), even on validation failures, because RenameThing.Rename() displays error messages via popup notifications. This is the intended UX design.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
📚 Learning: 2025-08-13T06:12:43.382Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3897
File: Flow.Launcher/ViewModel/PluginViewModel.cs:46-51
Timestamp: 2025-08-13T06:12:43.382Z
Learning: In Flow Launcher's PluginViewModel.cs, the LoadIconAsync method does not require additional try-catch error handling according to maintainer Jack251970, as the existing error handling approach is considered sufficient for the image loading operations.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Applied to files:
Flow.Launcher/Helper/ErrorReporting.cs
🧬 Code graph analysis (1)
Flow.Launcher/Helper/ErrorReporting.cs (1)
Flow.Launcher/Helper/ExceptionHelper.cs (2)
ExceptionHelper(10-44)IsRecoverableDwmCompositionException(22-43)
🪛 GitHub Check: Check Spelling
Flow.Launcher/Helper/ErrorReporting.cs
[warning] 26-26:
Dwm is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher/Helper/ErrorReporting.cs (1)
26-26: LGTM! Clean approach to handling recoverable DWM composition exceptions.The implementation correctly:
- Logs the exception first (line 17) to ensure all errors are captured for diagnostics
- Detects recoverable DWM composition exceptions using the helper
- Skips the error report UI for these transient issues, preventing the reported UI hangs
This addresses issue #4016 effectively while maintaining proper error logging.
Updated the
ErrorReportingclass to log recoverable DWM composition exceptions instead of displaying the "Report problem UI," addressing issues with crashes inPresentationFramework.dll. Introduced a newExceptionHelperclass to encapsulate logic for identifying recoverable DWM composition exceptions based on specific HRESULT values and stack trace patterns. Added detailed comments for context and referenced a related GitHub issue. Improved modularity and maintainability by isolating exception handling logic in theExceptionHelperclass.From: microsoft/PowerToys#42588.
Resolve: #4016