Merge pull request #897 from jbellister-slac/plugin_error_handling

ENH: Better error handling for data plugin loading

pydm/data_plugins/__init__.py

@@ -101,7 +101,7 @@ def plugin_for_address(address: str) -> Optional[PyDMPlugin]:
     return None
 
 
-def add_plugin(plugin: Type[PyDMPlugin]) -> PyDMPlugin:
+def add_plugin(plugin: Type[PyDMPlugin]) -> Optional[PyDMPlugin]:
     """
     Add a PyDM plugin to the global registry of protocol vs. plugins
 
@@ -112,8 +112,8 @@ def add_plugin(plugin: Type[PyDMPlugin]) -> PyDMPlugin:
 
     Returns
     -------
-    plugin : PyDMPlugin
-        The instantiated PyDMPlugin.
+    plugin : PyDMPlugin, optional
+        The instantiated PyDMPlugin. If instantiation failed, will return None.
     """
     # Warn users if we are overwriting a protocol which already has a plugin
     if plugin.protocol in plugin_modules:
@@ -123,7 +123,12 @@ def add_plugin(plugin: Type[PyDMPlugin]) -> PyDMPlugin:
             plugin_modules[plugin.protocol],
             plugin.protocol,
         )
-    instance = plugin()
+    try:
+        instance = plugin()
+    except Exception:
+        logger.exception(f"Data plugin: {plugin} failed to load and will not be available for use!")
+        return None
+
     plugin_modules[plugin.protocol] = instance
     return instance
 
@@ -254,7 +259,9 @@ def load_plugins_from_entrypoints(
                 plugin,
             )
             continue
-        added_plugins[plugin.protocol] = add_plugin(plugin)
+        added_plugin = add_plugin(plugin)
+        if added_plugin is not None:
+            added_plugins[plugin.protocol] = added_plugin
     return added_plugins
 
 
@@ -290,7 +297,9 @@ def load_plugins_from_path(
                 )
                 continue
 
-            added_plugins[plugin.protocol] = add_plugin(plugin)
+            added_plugin = add_plugin(plugin)
+            if added_plugin is not None:
+                added_plugins[plugin.protocol] = added_plugin
 
     return added_plugins
 

pydm/tests/test_data_plugins_import.py

@@ -16,18 +16,25 @@ def test_data_plugin_add(qapp, test_plugin):
     assert isinstance(qapp.plugins['tst'], test_plugin)
 
 
-def test_plugin_directory_loading(qapp):
+def test_plugin_directory_loading(qapp, caplog):
     # Create a fake file
     cur_dir = os.getcwd()
     with open(os.path.join(cur_dir, 'plugin_foo.py'), 'w+') as handle:
         handle.write(fake_file)
         handle.flush()
 
     try:
-        # Load plugins
+        # Load plugins. One of them will raise an exception during initialization. This will
+        # prevent it from being added to the available plugins, but the others should still load.
         load_plugins_from_path([cur_dir], 'foo.py')
         assert 'tst1' in plugin_modules
         assert 'tst2' in plugin_modules
+        assert 'fail' not in plugin_modules
+
+        # Check for the error logged when a plugin fails to load. Confirms that the plugin that failed to load
+        # was the one expected to fail, and that there was only one failure total.
+        assert "FailingTestPlugin'> failed to load and will not be available for use" in caplog.text
+        assert caplog.text.count("failed to load and will not be available for use") == 1
     finally:
         os.remove(os.path.join(cur_dir, 'plugin_foo.py'))
 
@@ -51,6 +58,12 @@ class TestPlugin1(PyDMPlugin):
     protocol = 'tst1'
 
 
+class FailingTestPlugin(PyDMPlugin):
+    protocol = 'fail'
+
+    def __init__(self, *args, **kwargs):
+        raise Exception  # Purposefully fail to initialize for testing purposes
+
 class TestPlugin2(PyDMPlugin):
     protocol = 'tst2'
 """