Skip to content

Fix-up of PR 12845: Work around a crash when uninstalling add-ons by taking advantage of lazy evaluation of type hints.#13135

Merged
seanbudd merged 1 commit into
nvaccess:masterfrom
lukaszgo1:PR12845fix-up
Dec 5, 2021
Merged

Fix-up of PR 12845: Work around a crash when uninstalling add-ons by taking advantage of lazy evaluation of type hints.#13135
seanbudd merged 1 commit into
nvaccess:masterfrom
lukaszgo1:PR12845fix-up

Conversation

@lukaszgo1

Copy link
Copy Markdown
Contributor

Link to issue number:

Fix-up of PR #12845

Summary of the issue:

PR #12845 added additional type hints to methods on appModuleHandler.AppModule. Unfortunately presence of these annotations can cause a crash when uninstalling add-ons importing gui in their installTasks module. Log from the crash is as follows:

  File "userConfig\addons\reportPasswords\installTasks.py", line 7, in <module>
    import gui
  File "gui\__init__.py", line 18, in <module>
    import ui
  File "ui.py", line 21, in <module>
    import speech
  File "speech\__init__.py", line 7, in <module>
    from .speech import (
  File "speech\speech.py", line 16, in <module>
    import api
  File "api.py", line 11, in <module>
    import review
  File "review.py", line 10, in <module>
    from NVDAObjects import NVDAObject, NVDAObjectTextInfo
  File "NVDAObjects\__init__.py", line 20, in <module>
    import eventHandler
  File "eventHandler.py", line 15, in <module>
    import appModuleHandler
  File "appModuleHandler.py", line 306, in <module>
    class AppModule(baseObject.ScriptableObject):
  File "appModuleHandler.py", line 602, in AppModule
    def getStatusBarText(self, obj: NVDAObjects.NVDAObject) -> str:
AttributeError: module 'NVDAObjects' has no attribute 'NVDAObject'
CRITICAL - __main__ (16:28:23.982) - MainThread (688):
core failure
Traceback (most recent call last):
  File "nvda.pyw", line 389, in <module>
    core.main()
  File "core.py", line 464, in main
    import appModuleHandler
  File "appModuleHandler.py", line 31, in <module>
    import NVDAObjects #Catches errors before loading default appModule
  File "NVDAObjects\__init__.py", line 19, in <module>
    import review
  File "review.py", line 7, in <module>
    import api
  File "api.py", line 31, in <module>
    def getFocusObject() -> NVDAObjects.NVDAObject:
AttributeError: module 'NVDAObjects' has no attribute 'NVDAObject'

Description of how this pull request fixes the issue:

This PR takes advantage of Postponed Evaluation of Annotations introduced in PEP 563 to avoid type hints in appModuleHandler being evaluated at import time.

Testing strategy:

  • Uninstalled an add-on which imports gui in installTasks
  • Inspected type hints on (appModuleHandler.AppModule.getStatusBarText by executing typing.get_type_hints(appModuleHandler.AppModule.getStatusBarText) both before and after this change - made sure they has not changed.

Known issues with pull request:

None known

Change log entries:

None needed - unreleased regression.

Code Review Checklist:

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

@lukaszgo1 lukaszgo1 requested a review from a team as a code owner December 5, 2021 20:55
@lukaszgo1 lukaszgo1 requested a review from seanbudd December 5, 2021 20:55
@seanbudd seanbudd merged commit a674652 into nvaccess:master Dec 5, 2021
@nvaccessAuto nvaccessAuto added this to the 2022.1 milestone Dec 5, 2021
@lukaszgo1 lukaszgo1 deleted the PR12845fix-up branch December 5, 2021 23:40
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.

3 participants