-
-
Notifications
You must be signed in to change notification settings - Fork 448
Handle plugin download exception from wrong url #3629
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
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 exception handling during plugin downloads by replacing a ContinueWith-based continuation with an inline try-catch block, providing more granular error reporting.
- Replaces delegate syntax with a lambda expression and wraps the async download in a try-catch block
- Differentiates HttpRequestException from general exceptions with tailored error messages and logging
Comments suppressed due to low confidence (1)
Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs:376
- Verify that using distinct translation keys for installation errors here versus download errors in the previous block is intentional, to ensure consistent user-facing error messaging.
catch (Exception e)
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
🥷 Code experts: onesounds Jack251970, onesounds have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThis update refactors error handling and task structure in the plugin update workflow. It introduces explicit try-catch blocks for exceptions during asynchronous plugin updates, improves error messaging, and makes minor formatting adjustments. No changes are made to public API signatures or exported entities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PluginsManager
participant PluginDownloader
participant Logger
participant UI
User->>PluginsManager: RequestUpdateAsync()
PluginsManager->>PluginsManager: Task.Run(async lambda)
PluginsManager->>PluginDownloader: Download plugin (if not local)
PluginDownloader-->>PluginsManager: Download result
PluginsManager->>PluginsManager: Check cancellation
alt Not cancelled
PluginsManager->>PluginsManager: Update plugin
PluginsManager->>UI: Show success message
alt AutoRestartAfterChanging
PluginsManager->>UI: Show restart message
end
else Cancelled
PluginsManager->>UI: Show cancellation message
end
PluginsManager->>Logger: Log exceptions (on error)
PluginsManager->>UI: Show error message (on error)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (1)
Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (1)
199-199
: Consider using full word for clarity.The parameter name
prgBoxTitle
uses the abbreviation "prg" which triggers spelling warnings. Consider renaming toprogressBoxTitle
for better clarity and to avoid pipeline warnings.-private async Task DownloadFileAsync(string prgBoxTitle, string downloadUrl, string filePath, CancellationTokenSource cts, bool deleteFile = true, bool showProgress = true) +private async Task DownloadFileAsync(string progressBoxTitle, string downloadUrl, string filePath, CancellationTokenSource cts, bool deleteFile = true, bool showProgress = true)🧰 Tools
🪛 GitHub Check: Check Spelling
[warning] 199-199:
prg
is not a recognized word. (unrecognized-spelling)🪛 GitHub Actions: Check Spelling
[warning] 199-199:
prg
is not a recognized word. (unrecognized-spelling)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher/PublicAPIInstance.cs
(1 hunks)Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (1)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (6)
GetTranslation
(141-141)ShowMsg
(119-119)ShowMsg
(128-128)RestartApp
(34-34)ShowMsgError
(85-85)LogException
(281-281)
🪛 GitHub Actions: Check Spelling
Flow.Launcher/PublicAPIInstance.cs
[warning] 52-52: Ioc
is not a recognized word. (unrecognized-spelling)
[warning] 150-174: VSTHRD
is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs
[warning] 199-199: prg
is not a recognized word. (unrecognized-spelling)
[warning] 207-207: prg
is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (3)
Flow.Launcher/PublicAPIInstance.cs (1)
253-254
: LGTM! Good formatting fix.The whitespace addition after the lambda arrow follows standard C# formatting conventions.
Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (2)
319-387
: Excellent fix for handling unobserved task exceptions!The refactoring properly addresses the issue by:
- Wrapping the entire async operation in try-catch blocks
- Explicitly handling
HttpRequestException
for download failures with user-friendly error messages- Providing general exception handling as a fallback
- Ensuring proper logging of exceptions for debugging
Moving the
CancellationTokenSource
instantiation inside the try block is also a good practice.
453-458
: Good consistency improvement for error messaging.Using
ShowMsgError
instead ofShowMsg
in the exception handler ensures proper error indication to users with the appropriate error icon.
Handle exception in task. Issue from #3617.
Resolve exception message like: