Skip to content
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

Fix some add-on installation errors not being reported #2518

Merged
merged 1 commit into from
May 25, 2023

Conversation

abdnh
Copy link
Contributor

@abdnh abdnh commented May 24, 2023

A missing future.result() call was masking add-on install/update errors. You can reproduce the issue by installing my add-on InContext, then manually decreasing meta.json's mod value and checking for add-on updates to trigger an update. You should see that the download progress dialog closes silently with Anki showing the "No updates available." tooltip. Anki then will prompt to update the same add-on every time.

In the case of InContext, a PermissionError was being masked, which is caused by the add-on holding a DB connection to a file in the user_files folder indefinitely, preventing Anki from backing up the folder (only a problem on Windows AFAIK).

I guess the issue in my add-on can be solved by either (1) refactoring the add-on to hold a DB connection only when absolutely needed, (2) patching some methods or (3) making use of hooks to close the connection when the add-on is being deleted/updated. I prefer (3) but the addons_dialog_will_delete_addons hook for example is not called when updating. Will you accept a PR that adds a call to it in the update routine, or maybe adds a new hook?

@dae
Copy link
Member

dae commented May 25, 2023

Thanks Abdo!

3 does sound like the best option. A new hook is probably best, as the existing hook was originally added to show a message to the user, which presumably is not wanted on upgrade: #1232

@dae dae merged commit 8d1e5c3 into ankitects:main May 25, 2023
@abdnh abdnh deleted the fut-result branch May 26, 2023 01:45
abdnh added a commit to abdnh/anki-incontext that referenced this pull request May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants