Improve error handling at plugin load time
Historically, I added PluginError near the beginning of the project, to test for all the odd cases where we have trouble loading malformed or broken plugin.
Since then we have improved a lot in the domain of error reporting, but the existing PluginError does something incorrect, as it:
- Inherits from
BstError, and as such is treated as a simple informative message to the user. - Is actually used to indicate a bug in a plugin - these errors are aimed at making life easier for plugin authors.
Recommended course of action
First, we should make PluginError stop inheriting from BstError, as it is more helpful to a plugin author to see a full stack trace in the case there is an error loading their plugin.
Further, since we have identified an opportunity (as a result of discussion about issue #317 (closed)) to report a generic error to the user in the specific case that a python plugin tries to import a dependency that is not a hard dependency of BuildStream, we should raise a LoadError for that specific case instead of a PluginError, with a more informative message; such as:
"The
fooplugin failed to import python modulebar, please install it with pip3 or using your distribution package manager"
Plugins which can behave better
deb
Currently, we have the new deb source plugin which imports arpy at the toplevel (as discovered in #317 (closed)), I think with a generic error about not being able to find python dependencies, such as the suggested LoadError above - we should not really need anything custom in the deb source for this.
Without the above suggested improvement in the core, it currently says something a little bit to cryptic to report to an innocent user:
Error loading pipeline: Failed to load Source plugin 'deb': No module named 'arpy'
ostree
The ostree plugin on the other can be installed on platforms where ostree is not a hard dependency of BuildStream. In this case, it would not be acceptable to just report an error about a failure to import the gi introspection module.
In this case, it could be interesting to do something like this at the toplevel:
try:
from buildstream import _ostree
except ImportError as e:
raise SourceError("Informative message about installing ostree, " +
"with a hint that it may not be available on the given platform") from e