nvaccess/nvda

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

Closed this issue · 1 comments

josephsl commented

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:

  • gui.addonGui.installAddon -> addonObj = systemUtils.ExecAndPump[addonHandler.Addon](addonHandler.installAddonBundle, bundle).funcRes
  • addonStore.install.installAddon -> addonObj = systemUtils.ExecAndPump[addonHandler.Addon](addonHandler.installAddonBundle, bundle).funcRes

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

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:

  • Add-on handler side (addonHandler.installAddonBundle): while installing add-ons, an overall try block is executed that will unpack and run onInstal function. An inner try block is used to actually execute the onInstall function with possible exceptions raised (after being converted into an add-on error exception). After this, the outer try block continues to finaly cluase that returns the ad-on bundle so other callers can perform cleanup actions. However, because finally clause includes a return statement, the newly created "add-on error" exception is lost i.e. not seen by systemUtils.ExecuteAndPump.run method.
  • GUI side: possible exceptions during add-on install routine is not handled, with NVDA proceeding to remove previous add-on versions after obtaining the add-on object.

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

Thanks.