Use ShowMsgError API Function & Handle E_ABORT COMException #3901
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves error handling and user experience in the Flow Launcher by replacing message boxes with error notifications and handling operation cancellation scenarios. The changes prevent unnecessary user interruptions while maintaining proper error reporting.
Key changes:
- Replace
ShowMsgBoxwithShowMsgErrorto reduce user disruption - Handle E_ABORT COMException (0x80004004) to gracefully manage cancelled folder operations
- Add comprehensive documentation explaining when and why E_ABORT occurs
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
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. 📝 WalkthroughWalkthroughIgnores COMException E_ABORT when opening directories and standardizes several error UI calls from ShowMsgBox to ShowMsgError in PublicAPIInstance; also replaces a ShowMsgBox call with ShowMsgError in PluginsLoader during .NET plugin load warning reporting. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant PublicAPI
participant Explorer
UI->>PublicAPI: OpenDirectory(path, selectItem)
PublicAPI->>Explorer: request open and select
alt Explorer succeeds or user cancels (COM E_ABORT)
Explorer-->>PublicAPI: success / E_ABORT
PublicAPI-->>UI: return (no error shown)
else Other exception
Explorer-->>PublicAPI: throws exception
PublicAPI-->>UI: ShowMsgError("folderOpenError", ex.Message)
end
sequenceDiagram
participant PluginsLoader
participant API
PluginsLoader->>API: detect .NET plugin load issue
PluginsLoader-->>API: ShowMsgError(concatenatedMessage)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Flow.Launcher/PublicAPIInstance.cs (3)
383-397: E_ABORT handling is correct; consider a named constant + minimal debug traceSwallowing COMException E_ABORT (0x80004004) aligns with Windows shell semantics and should eliminate the spurious popup. Two small refinements:
- Replace the magic number with a named constant for readability.
- Add a Debug-level trace so repeated occurrences can be diagnosed without bothering users.
Apply within this block:
-catch (COMException ex) when (ex.ErrorCode == unchecked((int)0x80004004)) +catch (COMException ex) when (ex.ErrorCode == E_ABORT) { /* * The COMException with HResult 0x80004004 is E_ABORT (operation aborted). * Shell APIs often return this when the operation is canceled or the shell cannot complete it cleanly. * It most likely comes from Win32Helper.OpenFolderAndSelectFile(targetPath). * Typical triggers: * The target file/folder was deleted/moved between computing targetPath and the shell call. * The folder is on an offline network/removable drive. * Explorer is restarting/busy and aborts the request. * A selection request to a new/closing Explorer window is canceled. * Because it is commonly user- or environment-driven and not actionable, * we should treat it as expected noise and ignore it to avoid bothering users. */ + LogDebug(ClassName, $"Ignoring E_ABORT while opening '{targetPath}': {ex.Message}"); }Add near the top of the class (e.g., after Line 43):
private const int E_ABORT = unchecked((int)0x80004004);
409-412: Consistency: standardize error notification parameter orderThis notification uses a detailed, formatted message as the title and a generic “errorTitle” as the subtitle. If your intended UX is headline first, then details, consider flipping the parameters to keep consistency across error paths.
-ShowMsgError( - string.Format(GetTranslation("folderOpenError"), ex.Message), - GetTranslation("errorTitle") -); +ShowMsgError( + GetTranslation("errorTitle"), + string.Format(GetTranslation("folderOpenError"), ex.Message) +);
440-442: Browser error notification: align title/subtitle conventionSame note as folder-open error: if the convention is headline first and details second, consider swapping to keep a uniform UX across notifications.
-ShowMsgError( - GetTranslation("browserOpenError"), - GetTranslation("errorTitle") -); +ShowMsgError( + GetTranslation("errorTitle"), + GetTranslation("browserOpenError") +);If “browserOpenError” is already the detailed message and you have a dedicated “browserOpenErrorTitle”, using it would be even clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/PublicAPIInstance.cs(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-06T12:21:37.947Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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/PublicAPIInstance.cs
🧬 Code Graph Analysis (1)
Flow.Launcher/PublicAPIInstance.cs (4)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (4)
LogError(305-305)ShowMsgError(85-85)GetTranslation(171-171)LogException(311-311)Flow.Launcher.Core/Plugin/JsonRPCV2Models/JsonRPCPublicAPI.cs (3)
LogError(158-161)ShowMsgError(62-65)GetTranslation(97-100)Flow.Launcher.Core/Resource/Internationalization.cs (1)
GetTranslation(333-345)Flow.Launcher.Infrastructure/Logger/Log.cs (1)
Exception(97-107)
⏰ 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/PublicAPIInstance.cs (1)
11-11: Import for COM interop is appropriateAdding System.Runtime.InteropServices is required for COMException usage here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Flow.Launcher.Core/Plugin/PluginsLoader.cs (1)
129-131: Nit: simplify multiline message assembly for readabilityMinor readability tweak: build the content with string.Join to avoid manual concatenation and duplicated separators.
- API.ShowMsgError($"{errorMessage}{Environment.NewLine}{Environment.NewLine}" + - $"{errorPluginString}{Environment.NewLine}{Environment.NewLine}" + - API.GetTranslation("referToLogs")); + API.ShowMsgError( + string.Join(Environment.NewLine + Environment.NewLine, new[] + { + errorMessage, + errorPluginString, + API.GetTranslation("referToLogs") + }));Note: If you adopt the two-argument overload per the other comment, apply string.Join to the message argument there instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
Flow.Launcher.Core/Plugin/PluginsLoader.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-13T06:12:43.361Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3897
File: Flow.Launcher/ViewModel/PluginViewModel.cs:46-51
Timestamp: 2025-08-13T06:12:43.361Z
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.Core/Plugin/PluginsLoader.cs
📚 Learning: 2025-07-06T12:21:37.947Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.Core/Plugin/PluginsLoader.cs
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.Core/Plugin/PluginsLoader.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.
Applied to files:
Flow.Launcher.Core/Plugin/PluginsLoader.cs
🧬 Code Graph Analysis (1)
Flow.Launcher.Core/Plugin/PluginsLoader.cs (1)
Flow.Launcher.Core/Resource/Internationalization.cs (1)
GetTranslation(333-345)
⏰ 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). (1)
- GitHub Check: build
Use ShowMsgError API Function & Handle E_ABORT COMException
Use ShowMsgError API Function
Use
ShowMsgErrorinstead ofShowMsgBoxso that Flow will not bother users a lot.Handle E_ABORT COMException
Handle E_ABORT COMException in
OpenDirectoryso that Flow will not try to open folder when users cancel the action.This is confirmed by fosterbarnes.
Resolve #3893.