-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix/multiple bug fixes #3935
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
base: main
Are you sure you want to change the base?
Fix/multiple bug fixes #3935
Conversation
…race condition - Use capability.id instead of title for validation checks in add_app_provider - Improve external_integration validation to handle actions without webhook - Trim chat and memory prompts before submission - Add low battery warning (<10%) on firmware update page to prevent bricking - Disable firmware update button when battery is too low - Fix race condition in WAL sync where _device could become null during async operations
…eck, limitless delete, URI parsing This commit addresses 6 open issues: 1. BasedHardware#3855: Failed to delete syncing temporal file - Added try-except handling in delete_syncing_temporal_file() to gracefully handle race conditions where files may have already been deleted 2. BasedHardware#3886: Confusing UI with similar icons in App Store - Changed error placeholder icons from Icons.apps to Icons.image_not_supported_outlined - Changed app options menu icon from Icons.apps to Icons.settings_outlined - Reduces visual confusion between error states and menu controls 3. BasedHardware#3783: Apps caching issue when editing - Improved cache invalidation logic to check both old and new visibility states - Ensures cache is properly cleared when app visibility changes in either direction 4. BasedHardware#3735: Don't check for discarded convo prompt if > 4 mins - Added duration check before calling should_discard_conversation() - Conversations longer than 4 minutes are no longer checked for discarding 5. BasedHardware#3684: delete_limitless_conversations deletes all convos - Re-enabled delete_conversations_by_source(uid, 'limitless') function - Correctly filters by source='limitless' to only delete Limitless imports 6. BasedHardware#3762: FormatException in URI scheme parsing - Added URL validation using Uri.tryParse()?.hasScheme == true - Prevents FormatException on invalid/empty URLs in app_detail.dart
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.
Code Review
This pull request contains multiple bug fixes across the frontend and backend. Key changes include adding validation for URLs before launching them, preventing firmware updates on low battery, fixing a race condition when accessing device info, and improving cache invalidation logic. I've found one high-severity issue related to overly broad exception handling in the storage utility, which could mask critical errors. My suggestion is to catch a more specific exception.
I am having trouble creating individual review comments. Click here to see my feedback.
backend/utils/other/storage.py (235-240)
Catching a generic Exception is too broad and can mask other important errors like permission issues or network failures. It's better to catch the specific NotFound exception that the Google Cloud Storage client throws when a blob does not exist.
To use NotFound, you'll need to add from google.cloud.exceptions import NotFound at the top of the file.
try:
blob.delete()
except NotFound:
# File may have already been deleted (race condition) or network retry caused duplicate deletion
# This is expected behavior - if the file is gone, the delete was successful
print(f"Note: Could not delete syncing file {file_path} as it was not found.")
|
which bugs # does this fix |
|
can u add before/after for visual changes pls and add pr description |
|
@aaravgarg sure will share them , and added the description |
Bug fixes:-
Issues Fixed
Test Plan