From 83bf760c4a087bbd3a12d6c26746ff27810e70a1 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Wed, 19 Mar 2025 16:27:23 +0000 Subject: [PATCH] plugins: better handle exceptions in __init__ When importing a plugin, if it raised an exception in its `__init__` file, we ignored it, and still loaded the plugin, in a potentially half-broken state. This is because maybe_load_plugin_init_method only calls exec_module_from_spec if the plugin is not already in sys.modules, but exec_module_from_spec will put the plugin into sys.modules even if it errors. Consider this patch to test with, enable the "labels" plugin: ```patch diff --git a/electrum/plugins/labels/__init__.py b/electrum/plugins/labels/__init__.py index b68127df8e..0d6d95abce 100644 --- a/electrum/plugins/labels/__init__.py +++ b/electrum/plugins/labels/__init__.py @@ -21,3 +21,5 @@ async def pull(self: 'Commands', plugin: 'LabelsPlugin' = None, wallet=None, for arg:bool:force:pull all labels """ return await plugin.pull_thread(wallet, force=force) + +raise Exception("heyheyhey") ``` I would expect we don't load the labels plugin due to the error, but we do: ``` >>> plugins.get_plugin("labels") ``` Log: ``` $ ./run_electrum -v --testnet -o 0.75 | I | simple_config.SimpleConfig | electrum directory /home/user/.electrum/testnet 0.75 | E | p/plugin.Plugins | cannot initialize plugin labels: Error pre-loading electrum.plugins.labels: Exception('heyheyhey') Traceback (most recent call last): File "/home/user/wspace/electrum/electrum/plugin.py", line 148, in exec_module_from_spec spec.loader.exec_module(module) File "", line 883, in exec_module File "", line 241, in _call_with_frames_removed File "/home/user/wspace/electrum/electrum/plugins/labels/__init__.py", line 25, in raise Exception("heyheyhey") Exception: heyheyhey The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/home/user/wspace/electrum/electrum/plugin.py", line 167, in load_plugins self.maybe_load_plugin_init_method(name) File "/home/user/wspace/electrum/electrum/plugin.py", line 293, in maybe_load_plugin_init_method module = self.exec_module_from_spec(init_spec, base_name) File "/home/user/wspace/electrum/electrum/plugin.py", line 150, in exec_module_from_spec raise Exception(f"Error pre-loading {path}: {repr(e)}") from e Exception: Error pre-loading electrum.plugins.labels: Exception('heyheyhey') 0.75 | D | util.profiler | Plugins.__init__ 0.0030 sec 0.84 | I | simple_config.SimpleConfig | electrum directory /home/user/.electrum/testnet 0.89 | I | __main__ | get_default_language: detected default as lang='en_UK' 0.89 | I | i18n | setting language to 'en_UK' 0.89 | I | logging | Electrum version: 4.5.8 - https://electrum.org - https://github.com/spesmilo/electrum 0.89 | I | logging | Python version: 3.10.12 (main, Feb 4 2025, 14:57:36) [GCC 11.4.0]. On platform: Linux-6.8.0-52-generic-x86_64-with-glibc2.35 0.89 | I | logging | Logging to file: /home/user/.electrum/testnet/logs/electrum_log_20250319T161247Z_6605.log 0.89 | I | logging | Log filters: verbosity '*', verbosity_shortcuts '' 0.89 | I | exchange_rate.FxThread | using exchange CoinGecko 0.90 | D | util.profiler | Daemon.__init__ 0.0047 sec 0.90 | I | daemon.Daemon | starting taskgroup. 0.90 | I | daemon.CommandsServer | now running and listening. socktype=unix, addr=/home/user/.electrum/testnet/daemon_rpc_socket 0.90 | I | p/plugin.Plugins | registering hardware bitbox02: ['hardware', 'bitbox02', 'BitBox02'] 0.90 | I | p/plugin.Plugins | registering hardware coldcard: ['hardware', 'coldcard', 'Coldcard Wallet'] 0.90 | I | p/plugin.Plugins | registering hardware digitalbitbox: ['hardware', 'digitalbitbox', 'Digital Bitbox wallet'] 0.90 | I | p/plugin.Plugins | could not find manifest.json of plugin hw_wallet, skipping... 0.90 | I | p/plugin.Plugins | registering hardware jade: ['hardware', 'jade', 'Jade wallet'] 0.90 | I | p/plugin.Plugins | registering hardware keepkey: ['hardware', 'keepkey', 'KeepKey wallet'] 0.90 | I | p/plugin.Plugins | registering hardware ledger: ['hardware', 'ledger', 'Ledger wallet'] 0.90 | I | p/plugin.Plugins | registering hardware safe_t: ['hardware', 'safe_t', 'Safe-T mini wallet'] 0.90 | I | p/plugin.Plugins | registering hardware trezor: ['hardware', 'trezor', 'Trezor wallet'] 0.90 | I | p/plugin.Plugins | registering wallet type ('2fa', 'trustedcoin') 1.01 | I | p/plugin.Plugins | loaded plugin 'labels'. (from thread: 'GUI') 1.01 | D | util.profiler | Plugins.__init__ 0.1183 sec ``` --- electrum/plugin.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/electrum/plugin.py b/electrum/plugin.py index 3ef989614..928ec79a3 100644 --- a/electrum/plugin.py +++ b/electrum/plugin.py @@ -61,6 +61,7 @@ plugin_loaders = {} hook_names = set() hooks = {} _root_permission_cache = {} +_exec_module_failure = {} # type: Dict[str, Exception] class Plugins(DaemonThread): @@ -139,7 +140,9 @@ class Plugins(DaemonThread): self.external_plugin_metadata[name] = d @staticmethod - def exec_module_from_spec(spec, path): + def exec_module_from_spec(spec, path: str): + if prev_fail := _exec_module_failure.get(path): + raise Exception(f"exec_module already failed once before, with: {prev_fail!r}") try: module = importlib.util.module_from_spec(spec) # sys.modules needs to be modified for relative imports to work @@ -147,6 +150,13 @@ class Plugins(DaemonThread): sys.modules[path] = module spec.loader.exec_module(module) except Exception as e: + # We can't undo all side-effects, but we at least rm the module from sys.modules, + # so the import system knows it failed. If called again for the same plugin, we do not + # retry due to potential interactions with not-undone side-effects (e.g. plugin + # might have defined commands). + _exec_module_failure[path] = e + if path in sys.modules: + sys.modules.pop(path, None) raise Exception(f"Error pre-loading {path}: {repr(e)}") from e return module