Fix temp files deleting issue & Validate the cache directory before loading all bookmarks #3535
Conversation
Jack251970
added
Dev branch only
This comment has been minimized.
This comment has been minimized.
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 Walkthrough## Walkthrough
The error handling logic in the `LoadFaviconsFromDb` method was updated to improve cleanup procedures when copying the favicon database fails. The new implementation checks for the existence of the temporary file before attempting deletion and logs any exceptions that occur during this cleanup step. Additionally, the favicon cache directory validation was moved from the `Init` method to the start of `LoadBookmarksIfEnabled`, ensuring validation occurs only when bookmarks are loaded and enabled.
## Changes
| File(s) | Change Summary |
|------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------|
| Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs | Enhanced error handling in `LoadFaviconsFromDb` by adding existence check and nested try-catch for temp file deletion; added logging for deletion failures. |
| Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs | Moved favicon cache directory validation from `Init` to the start of `LoadBookmarksIfEnabled` to validate only when bookmarks are loaded and enabled. |
## Poem
> In the world of bookmarks, neat and bright,
> A bunny checks errors, making things right.
> If temp files linger, don’t fret or frown—
> Now logs will catch them, when cleanup goes down.
> With careful paws and a coder’s delight,
> Bugs hop away, far out of sight!Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
✨ 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.
Pull Request Overview
This PR addresses an exception triggered when deleting temporary favicon DB files during the copying process by adding a nested try-catch block to safely handle file deletion failures.
- Adds an inner try-catch block to delete the temporary file if it exists following a failed copy
- Logs an exception if the deletion also fails, preventing unhandled exceptions during cleanup
Comments suppressed due to low confidence (1)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs:141
- [nitpick] Consider renaming the exception variable 'ex1' to a more descriptive name (e.g., 'deleteEx') to clarify that it relates to the deletion failure.
catch (Exception ex1)
@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, Yusyuriv 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 |
|
@jjw24 Tested and I think it is good to go. |
Follow on with #3361.
1. Fix temp files deleting issue
Fix issue:
2. Validate the cache directory before loading all bookmarks
Validate the cache directory before loading all bookmarks because Flow needs this directory to storage favicons.
Or loading will fail when the cache directory is missing.