Handle recoverable DWM composition exceptions gracefully #4113
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) composition COMExceptions and skips showing the ReportWindow for those cases; introduces a new helper to identify such exceptions and updates ErrorReporting to use it. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ErrorReporting
participant ExceptionHelper
participant ReportWindow
App->>ErrorReporting: Exception occurs
Note over ErrorReporting: Log fatal exception
ErrorReporting->>ExceptionHelper: IsRecoverableDwmCompositionException(e)?
alt Recoverable DWM Exception
ExceptionHelper-->>ErrorReporting: true
Note over ErrorReporting: Return early — skip ReportWindow
else Non-recoverable Exception
ExceptionHelper-->>ErrorReporting: false
ErrorReporting->>ReportWindow: Create & Show error report UI
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Possibly related issues
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 (2)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-07-06T12:21:37.947ZApplied to files:
📚 Learning: 2025-06-29T08:31:07.816ZApplied to files:
📚 Learning: 2025-06-08T14:12:21.348ZApplied to files:
🧬 Code graph analysis (1)Flow.Launcher/Helper/ErrorReporting.cs (1)
🪛 GitHub Check: Check SpellingFlow.Launcher/Helper/ExceptionHelper.cs[warning] 42-42: [warning] 22-22: [warning] 42-42: [warning] 22-22: Flow.Launcher/Helper/ErrorReporting.cs[warning] 25-25: ⏰ 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). (3)
🔇 Additional comments (4)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
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 <[email protected]>
Co-authored-by: Copilot <[email protected]>
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