nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
Other
2.03k stars 625 forks source link

Add-on bundle installer: add-on is removed without installation if add-on installTasks.onInstall function fails on update #16704

Closed josephsl closed 1 week ago

josephsl commented 1 month ago

Hi,

A more general version of #16702:

Steps to reproduce:

Create an add-on (or modify an existing add-on from its source code) with the following installTasks.py module (it works better if installTasks.py already exists in the source code):

# Whatever copyright header you wish to use

def onInstall() -> None: raise RuntimeError(whateverExceptionText)

Package the modified add-on, then try installing it. To fully reproduce the problem, you must create an add-on that is newer than an installed add-on (works best if you are add-on authors and ask an add-on to "volunteer" i.e. edit one of your add-ons with modified installTasks.py and reverting it afterwards). For a demo, I have modified a dev build of Windows App Essentials.

Actual behavior:

After installation prompts are shown, the progress tones are heard (if NVDA was told to do so) along with error tone (if alpha build is in use), then NVDA asks for a restart.

previous add-on version is removed along with json files, without the new add-on being installed

Expected behavior:

NVDA presents add-on installation error dialog.

NVDA logs, crash dumps and other attachments:

ERROR - addonHandler.installAddonBundle (09:58:54.692) - systemUtils.ExecAndPump(<function installAddonBundle at 0x03515438>) (8444): task 'onInstall' on addon 'wintenApps' failed Traceback (most recent call last): File "addonHandler__init.pyc", line 439, in installAddonBundle File "addonHandler\init__.pyc", line 754, in runInstallTask File "C:\Users\User\AppData\Roaming\nvda\addons\wintenApps.pendingInstall\installTasks.py", line 14, in onInstall raise RuntimeError("Testing add-on install exception handling") RuntimeError: Testing add-on install exception handling

While not shown in the traceback, this path is called from gui.addonGui.py. This will be important (see technical information).

System configuration

NVDA installed/portable/running from source:

Installed

NVDA version:

alpha-32397,ad9ed733

Windows version:

Windows 11 Version 24H2 preview (build 26100.863, AMD64)

Name and version of other software in use when reproducing the issue:

None

Other information about your system:

Used as a development workstation

Other questions

Does the issue still occur after restarting your computer?

Yes

Have you tried any other versions of NVDA? If so, please report their behaviors.

Reproducible in 2024.1 and 2024.2 beta 3 (see #16702).

If NVDA add-ons are disabled, is your problem still occurring?

Yes

Does the issue still occur after you run the COM Registration Fixing Tool in NVDA's tools menu?

Not applicable

Technical information:

Possible cause: not catching exceptions while retrieving add-on object while calling systemUtils.ExecAndPump during add-on installs. The following code fragments are affected:

systemUtils.execAndPump runs addonHandler.installAddonBundle on a separate thread, with the latter function catching exceptions coming from an add-on's installTasks.onInstall function (two try/except block layers are used). However, right after asking systemUtils.ExecAndPump to run add-on installation routine, NVDA proceeds to remove older add-on releases along with json file(s). This makes the subsequent "add-on install error" dialog dead code (not invoked despite the condition being right to display this dialog).

A possible solution is catching exceptions coming from systemUtils.ExecAndPump through inter-thread communication, with the call to this function housed in its own try/except block so results of possible exceptions will not affect previous add-on instlals. This may also require rethinking the json file handling strategy - previous add-on version json file should not be removed until add-on installation is successful.

Thanks.

josephsl commented 2 weeks ago

Hi,

Upon further investigation, it turns out the error may have to do with a combination of install routines from GUI side and in add-on handler:

Of these, I think fixing the add-on handler issue may offer a resolution path.

Thanks.