From 3c75781b9d68b7b1958082a0f75a2fc5db774418 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Fri, 29 Dec 2017 18:42:20 -0500 Subject: 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. --- buildstream/_exceptions.py | 76 +++++++++++++++++++++++++++++++++++-------- buildstream/_ostree.py | 5 +-- buildstream/_scheduler/job.py | 25 +++++++++++++- buildstream/element.py | 5 +-- buildstream/source.py | 5 +-- buildstream/utils.py | 8 +++-- 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(): -- cgit v1.2.1