Exceptions refactoring

Outline of changes to exceptions:

  o Now BstError base class has a `domain` and `reason` member,
    reason members are optional.

  o All derived error classes now specify their `domain`

  o For LoadError, LoadErrorReason defines the error's `reason`

  o Now the scheduler `job` class takes care of sending the error
    reason and domain back to the main process where the last
    exception which caused a child task to fail can be discretely stored.

6 files changed, 100 insertions, 24 deletions

diff --git a/buildstream/_exceptions.py b/buildstream/_exceptions.py
index ffcd992be..8dd57b617 100644
--- a/buildstream/_exceptions.py
+++ b/buildstream/_exceptions.py
@@ -23,6 +23,8 @@ from enum import Enum
 
 # The last raised exception, this is used in test cases only
 _last_exception = None
+_last_task_error_domain = None
+_last_task_error_reason = None
 
 
 def _get_last_exception():
@@ -33,6 +35,41 @@ def _get_last_exception():
     return le
 
 
+# Called from regression test fixtures
+def _get_last_task_error():
+    global _last_task_error_domain
+    global _last_task_error_reason
+
+    d = _last_task_error_domain
+    r = _last_task_error_reason
+    _last_task_error_domain = _last_task_error_reason = None
+    return (d, r)
+
+
+# Called from the scheduler when an error is encountered
+def _set_last_task_error(domain, reason):
+    global _last_task_error_domain
+    global _last_task_error_reason
+
+    _last_task_error_domain = domain
+    _last_task_error_reason = reason
+
+
+class ErrorDomain(Enum):
+    PLUGIN = 1
+    LOAD = 2
+    IMPL = 3
+    PLATFORM = 4
+    SANDBOX = 5
+    ARTIFACT = 6
+    PIPELINE = 7
+    OSTREE = 8
+    UTIL = 9
+    PROG_NOT_FOUND = 12
+    SOURCE = 10
+    ELEMENT = 11
+
+
 # BstError is an internal base exception class for BuildSream
 # exceptions.
 #
@@ -42,16 +79,21 @@ def _get_last_exception():
 #
 class BstError(Exception):
 
-    def __init__(self, message):
+    def __init__(self, message, *, domain=None, reason=None):
         global _last_exception
 
-        super(BstError, self).__init__(message)
+        super().__init__(message)
 
         # The build sandbox in which the error occurred, if the
         # error occurred at element assembly time.
         #
         self.sandbox = None
 
+        # Error domain and reason
+        #
+        self.domain = domain
+        self.reason = reason
+
         # Hold on to the last raised exception for testing purposes
         _last_exception = self
 
@@ -64,7 +106,8 @@ class BstError(Exception):
 # or by the base :class:`.Plugin` element itself.
 #
 class PluginError(BstError):
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.PLUGIN, reason=reason)
 
 
 # LoadErrorReason
@@ -113,16 +156,16 @@ class LoadErrorReason(Enum):
 #
 # Raised while loading some YAML.
 #
+# Args:
+#    reason (LoadErrorReason): machine readable error reason
+#    message (str): human readable error explanation
+#
 # This exception is raised when loading or parsing YAML, or when
 # interpreting project YAML
 #
 class LoadError(BstError):
     def __init__(self, reason, message):
-        super(LoadError, self).__init__(message)
-
-        # The :class:`.LoadErrorReason` for which this exception was raised
-        #
-        self.reason = reason
+        super().__init__(message, domain=ErrorDomain.LOAD, reason=reason)
 
 
 # ImplError
@@ -131,14 +174,16 @@ class LoadError(BstError):
 # implement a mandatory method
 #
 class ImplError(BstError):
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.IMPL, reason=reason)
 
 
 # PlatformError
 #
 # Raised if the current platform is not supported.
 class PlatformError(BstError):
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.PLATFORM, reason=reason)
 
 
 # SandboxError
@@ -146,7 +191,8 @@ class PlatformError(BstError):
 # Raised when errors are encountered by the sandbox implementation
 #
 class SandboxError(BstError):
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.SANDBOX, reason=reason)
 
 
 # ArtifactError
@@ -154,7 +200,8 @@ class SandboxError(BstError):
 # Raised when errors are encountered in the artifact caches
 #
 class ArtifactError(BstError):
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.ARTIFACT, reason=reason)
 
 
 # PipelineError
@@ -163,11 +210,12 @@ class ArtifactError(BstError):
 #
 class PipelineError(BstError):
 
-    def __init__(self, message=None):
+    def __init__(self, message=None, reason=None):
 
         # The empty string should never appear to a user,
         # this only allows us to treat this internal error as
         # a BstError from the frontend.
         if message is None:
             message = ""
-        super(PipelineError, self).__init__(message)
+
+        super().__init__(message, domain=ErrorDomain.PIPELINE, reason=reason)
diff --git a/buildstream/_ostree.py b/buildstream/_ostree.py
index 5e533e2dd..c9f0389d1 100644
--- a/buildstream/_ostree.py
+++ b/buildstream/_ostree.py
@@ -23,7 +23,7 @@
 # Code based on Jürg's artifact cache and Andrew's ostree plugin
 #
 import os
-from ._exceptions import BstError
+from ._exceptions import BstError, ErrorDomain
 
 import gi
 gi.require_version('OSTree', '1.0')
@@ -33,7 +33,8 @@ from gi.repository.GLib import Variant, VariantDict  # nopep8
 
 # For users of this file, they must expect (except) it.
 class OSTreeError(BstError):
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.UTIL, reason=reason)
 
 
 # ensure()
diff --git a/buildstream/_scheduler/job.py b/buildstream/_scheduler/job.py
index 43b6b7c78..310bb8c70 100644
--- a/buildstream/_scheduler/job.py
+++ b/buildstream/_scheduler/job.py
@@ -30,7 +30,7 @@ import multiprocessing
 from ruamel import yaml
 
 # BuildStream toplevel imports
-from .._exceptions import BstError
+from .._exceptions import BstError, _set_last_task_error
 from .._message import Message, MessageType, unconditional_messages
 from ..plugin import _plugin_lookup
 from .. import _yaml, _signals, utils
@@ -278,9 +278,12 @@ class Job():
                                  elapsed=elapsed, detail=str(e),
                                  logfile=filename, sandbox=e.sandbox)
 
+                # Report the exception to the parent (for internal testing purposes)
+                self.child_send_error(e)
                 self.child_shutdown(1)
 
             except Exception as e:
+
                 # If an unhandled (not normalized to BstError) occurs, that's a bug,
                 # send the traceback and formatted exception back to the frontend
                 # and print it to the log file.
@@ -301,6 +304,20 @@ class Job():
         # is already busy in sys.exit()
         self.child_shutdown(0)
 
+    def child_send_error(self, e):
+        domain = None
+        reason = None
+
+        if isinstance(e, BstError):
+            domain = e.domain
+            reason = e.reason
+
+        envelope = Envelope('error', {
+            'domain': domain,
+            'reason': reason
+        })
+        self.queue.put(envelope)
+
     def child_complete(self, pid, returncode, element):
         if returncode != 0 and self.tries <= self.max_retries:
             self.shutdown()
@@ -381,6 +398,12 @@ class Job():
             # Propagate received messages from children
             # back through the context.
             self.scheduler.context._message(envelope.message)
+        elif envelope.message_type == 'error':
+            # For regression tests only, save the last error domain / reason
+            # reported from a child task in the main process, this global state
+            # is currently managed in _exceptions.py
+            _set_last_task_error(envelope.message['domain'],
+                                 envelope.message['reason'])
         elif envelope.message_type == 'result':
             assert(self.result is None)
             self.result = envelope.message
diff --git a/buildstream/element.py b/buildstream/element.py
index 7e7037ea5..ee056f39c 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -35,7 +35,7 @@ import shutil
 
 from . import _yaml
 from ._variables import Variables
-from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError
+from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError, ErrorDomain
 from . import Plugin, Consistency
 from ._project import BST_ARTIFACT_VERSION as BST_CORE_ARTIFACT_VERSION
 from . import SandboxFlags
@@ -84,7 +84,8 @@ class ElementError(BstError):
 
     This exception is raised when an :class:`.Element` encounters an error.
     """
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.ELEMENT, reason=reason)
 
 
 class Element(Plugin):
diff --git a/buildstream/source.py b/buildstream/source.py
index f610e90af..63d8ffafd 100644
--- a/buildstream/source.py
+++ b/buildstream/source.py
@@ -27,7 +27,7 @@ from contextlib import contextmanager
 
 from . import Plugin
 from . import _yaml, utils
-from ._exceptions import BstError, ImplError, LoadError, LoadErrorReason
+from ._exceptions import BstError, ImplError, LoadError, LoadErrorReason, ErrorDomain
 
 
 class Consistency():
@@ -58,7 +58,8 @@ class SourceError(BstError):
 
     This exception is raised when a :class:`.Source` encounters an error.
     """
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.SOURCE, reason=reason)
 
 
 class Source(Plugin):
diff --git a/buildstream/utils.py b/buildstream/utils.py
index 7e84dbbe0..89c4cc016 100644
--- a/buildstream/utils.py
+++ b/buildstream/utils.py
@@ -41,7 +41,7 @@ import psutil
 
 from . import _signals
 from . import _yaml
-from ._exceptions import BstError
+from ._exceptions import BstError, ErrorDomain
 
 
 class UtilError(BstError):
@@ -52,7 +52,8 @@ class UtilError(BstError):
     or either of the :class:`.ElementError` or :class:`.SourceError`
     exceptions should be raised from this error.
     """
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.UTIL, reason=reason)
 
 
 class ProgramNotFoundError(BstError):
@@ -60,7 +61,8 @@ class ProgramNotFoundError(BstError):
 
     It is normally unneeded to handle this exception from plugin code.
     """
-    pass
+    def __init__(self, message, reason=None):
+        super().__init__(message, domain=ErrorDomain.PROG_NOT_FOUND, reason=reason)
 
 
 class FileListResult():