Skip to content

Fix add-on store downloads after cancellation#20017

Merged
seanbudd merged 8 commits into
nvaccess:masterfrom
cary-rowen:fix/addonstore-cancelled-download
Apr 29, 2026
Merged

Fix add-on store downloads after cancellation#20017
seanbudd merged 8 commits into
nvaccess:masterfrom
cary-rowen:fix/addonstore-cancelled-download

Conversation

@cary-rowen

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #20015

Summary of the issue:

After cancelling an add-on download by closing the Add-on Store and choosing "Yes", the store can get into a bad state.
If the user opens it again and starts another download, NVDA can raise:

AttributeError: 'NoneType' object has no attribute 'submit'

Closing the store again after that can also raise:

AttributeError: 'NoneType' object has no attribute 'shutdown'

Description of user facing changes:

After cancelling a download, the Add-on Store can be used again normally.
Starting another download no longer throws those exceptions.
Closing the store again after that also works as expected.

Description of developer facing changes:

AddonFileDownloader now tracks state per download attempt.
This prevents an older cancelled worker from interfering with a later retry of the same add-on.

Description of development approach:

The fix updates the downloader so each download attempt has its own state and temporary file path.
After cancelAll(), the downloader can recreate its executor and be reused safely.
When a worker finishes, it now checks whether it still belongs to the active attempt before updating progress, cleaning up files, or completing the download.

Testing strategy:

Manual testing:

  • Start a download from Add-on Store.
  • Close the store and choose "Yes" to cancel.
  • Open Add-on Store again.
  • Start another download.
  • Close the store again and choose "Yes" if prompted.

Expected result:

  • The second download starts normally.
  • No submit or shutdown exception is logged.

Automated testing:

  • Added unit tests for reusing the downloader after cancelAll().
  • Added a regression test to cover cancel then retry while an older attempt is still finishing.

Known issues with pull request:

None known.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@cary-rowen cary-rowen force-pushed the fix/addonstore-cancelled-download branch from a3cdf81 to 42ceb6f Compare April 25, 2026 17:55
@cary-rowen cary-rowen force-pushed the fix/addonstore-cancelled-download branch from 42ceb6f to 76eac0a Compare April 26, 2026 08:28
@cary-rowen cary-rowen marked this pull request as ready for review April 27, 2026 04:11
@cary-rowen cary-rowen requested a review from a team as a code owner April 27, 2026 04:11
@cary-rowen cary-rowen requested a review from seanbudd April 27, 2026 04:11
Comment thread source/addonStore/network.py Outdated
Comment thread source/addonStore/network.py Outdated
Comment thread source/addonStore/network.py Outdated
Comment thread source/addonStore/network.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a bad Add-on Store downloader state after cancelling a download, so subsequent downloads and closing the store no longer raise NoneType executor errors.

Changes:

  • Refactors AddonFileDownloader to track per-attempt download state (including per-attempt temp file path) and ignore stale/completing workers from prior attempts.
  • Makes cancelAll() shut down and allow safe recreation of the executor/resources so the downloader instance can be reused.
  • Adds unit/regression tests covering cancel-then-retry and downloader reuse, plus a changelog entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
source/addonStore/network.py Introduces per-attempt state tracking and safer cancellation/retry behavior in AddonFileDownloader.
tests/unit/test_addonStoreNetwork.py Adds unit tests for cancel/retry scenarios and executor/directory recreation after cancellation.
user_docs/en/changes.md Adds a user-facing changelog entry for the fixed Add-on Store download retry behavior.

Comment thread source/addonStore/network.py
Comment thread source/addonStore/network.py Outdated
Comment thread source/addonStore/network.py Outdated
Comment thread source/addonStore/network.py Outdated
Comment thread source/addonStore/network.py Outdated
Comment thread source/addonStore/network.py Outdated
@seanbudd seanbudd marked this pull request as draft April 28, 2026 02:29
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Apr 28, 2026
@cary-rowen

Copy link
Copy Markdown
Contributor Author

Thanks @seanbudd, I addressed these in the latest commit. I replaced the one-field _DownloadProgress dataclass with a _TempDownloadPathT alias, renamed the attempt counter, and moved the add-on download directory checks/creation/removal under DOWNLOAD_LOCK. I also updated the exception logging to use log.exception, including the future exception path.

@cary-rowen cary-rowen marked this pull request as ready for review April 28, 2026 15:25

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cary-rowen

@seanbudd seanbudd merged commit ead66f8 into nvaccess:master Apr 29, 2026
33 of 37 checks passed
@github-actions github-actions Bot added this to the 2026.2 milestone Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Addon Store cannot start a new download after cancelling a previous download

3 participants