plugins: ensure to log the offending plugin on instantiation failure

beets/plugins.py

@@ -23,6 +23,7 @@
 import traceback
 from collections import defaultdict
 from functools import wraps
+from importlib import import_module
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -46,7 +47,7 @@
 
 
 if TYPE_CHECKING:
-    from collections.abc import Iterable
+    from collections.abc import Iterable, Iterator
 
     from confuse import ConfigView
 
@@ -341,7 +342,7 @@ def helper(func: TFunc[Item]) -> TFunc[Item]:
         return helper
 
 
-_classes: set[type[BeetsPlugin]] = set()
+_instances: dict[type[BeetsPlugin], BeetsPlugin] = {}
 
 
 def load_plugins(names: Sequence[str] = ()) -> None:
@@ -350,57 +351,55 @@ def load_plugins(names: Sequence[str] = ()) -> None:
     package in sys.path; the module indicated should contain the
     BeetsPlugin subclasses desired.
     """
+    classes = set()
+
     for name in names:
-        modname = f"{PLUGIN_NAMESPACE}.{name}"
         try:
-            try:
-                namespace = __import__(modname, None, None)
-            except ImportError as exc:
-                # Again, this is hacky:
-                if exc.args[0].endswith(" " + name):
-                    log.warning("** plugin {0} not found", name)
-                else:
-                    raise
-            else:
-                for obj in getattr(namespace, name).__dict__.values():
-                    if (
-                        isinstance(obj, type)
-                        and issubclass(obj, BeetsPlugin)
-                        and obj != BeetsPlugin
-                        and obj != MetadataSourcePlugin
-                        and obj not in _classes
-                    ):
-                        _classes.add(obj)
-
-        except Exception:
+            mod = import_module(f".{name}", package=PLUGIN_NAMESPACE)
+        except ModuleNotFoundError:
+            log.warning("** plugin {} not found", name)
+            continue
+        except ImportError:
             log.warning(
                 "** error loading plugin {}:\n{}",
                 name,
                 traceback.format_exc(),
             )
+            continue
 
+        for _name, cls in inspect.getmembers(mod, inspect.isclass):
+            if (
+                issubclass(cls, BeetsPlugin)
+                and cls != BeetsPlugin
+                and cls != MetadataSourcePlugin
+                and cls not in _instances
+            ):
+                classes.add(cls)
 
-_instances: dict[type[BeetsPlugin], BeetsPlugin] = {}
+    for cls in classes:
+        _register_plugin(cls)
+
+
+def _register_plugin(cls: type[BeetsPlugin]) -> None:
+    if cls in _instances:
+        return
+
+    try:
+        _instances[cls] = cls()
+    except Exception:
+        log.error(
+            "failed to initialize plugin class {}.{}",
+            cls.__module__,
+            cls.__qualname__,
+        )
+        raise
 
 
-def find_plugins() -> list[BeetsPlugin]:
-    """Returns a list of BeetsPlugin subclass instances from all
-    currently loaded beets plugins. Loads the default plugin set
-    first.
+def find_plugins() -> Iterator[BeetsPlugin]:
+    """Returns an iterator of BeetsPlugin subclass instances from all
+    currently loaded beets plugins.
     """
-    if _instances:
-        # After the first call, use cached instances for performance reasons.
-        # See https://github.com/beetbox/beets/pull/3810
-        return list(_instances.values())
-
-    load_plugins()
-    plugins = []
-    for cls in _classes:
-        # Only instantiate each plugin class once.
-        if cls not in _instances:
-            _instances[cls] = cls()
-        plugins.append(_instances[cls])
-    return plugins
+    yield from _instances.values()
 
 
 # Communication with plugins.

beets/test/helper.py

@@ -487,7 +487,6 @@ def unload_plugins(self) -> None:
         for plugin_class in beets.plugins._instances:
             plugin_class.listeners = None
         self.config["plugins"] = []
-        beets.plugins._classes = set()
         beets.plugins._instances = {}
         Item._types = self.original_item_types
         Album._types = self.original_album_types

test/test_plugins.py

@@ -37,30 +37,22 @@
 
 
 class PluginLoaderTestCase(BasePluginTestCase):
-    def setup_plugin_loader(self):
-        # FIXME the mocking code is horrific, but this is the lowest and
-        # earliest level of the plugin mechanism we can hook into.
-        self._plugin_loader_patch = patch("beets.plugins.load_plugins")
-        self._plugin_classes = set()
-        load_plugins = self._plugin_loader_patch.start()
+    """A test case that adds an additional plugin to beets' plugin method
+    resolution mechanism.
 
-        def myload(names=()):
-            plugins._classes.update(self._plugin_classes)
-
-        load_plugins.side_effect = myload
-
-    def teardown_plugin_loader(self):
-        self._plugin_loader_patch.stop()
+    It does _not_ go through the full `plugins.load_plugins` code, which would
+    require that the plugin_class is part of an importable module.
+    """
 
     def register_plugin(self, plugin_class):
-        self._plugin_classes.add(plugin_class)
+        plugins._register_plugin(plugin_class)
 
     def setUp(self):
-        self.setup_plugin_loader()
         super().setUp()
+        self._prev_plugin_instances = plugins._instances.copy()
 
     def tearDown(self):
-        self.teardown_plugin_loader()
+        plugins._instances = self._prev_plugin_instances
         super().tearDown()
 
 

test/test_ui.py

@@ -1059,7 +1059,7 @@ def test_cli_config_file_loads_plugin_commands(self):
             file.write("plugins: test")
 
         self.run_command("--config", self.cli_config_path, "plugin", lib=None)
-        assert plugins.find_plugins()[0].is_test_plugin
+        assert next(iter(plugins.find_plugins())).is_test_plugin
         self.unload_plugins()
 
     def test_beetsdir_config(self):