From 8e3b2ab933c11ef37678da0b784b971c861afdf6 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 10:26:39 +0200 Subject: [PATCH] raise specific exceptions when creators fail to run their methods --- openpype/pipeline/create/context.py | 258 ++++++++++++++++++++++------ 1 file changed, 204 insertions(+), 54 deletions(-) diff --git a/openpype/pipeline/create/context.py b/openpype/pipeline/create/context.py index fb53b95a92..dfa9049601 100644 --- a/openpype/pipeline/create/context.py +++ b/openpype/pipeline/create/context.py @@ -1,6 +1,8 @@ import os +import sys import copy import logging +import traceback import collections import inspect from uuid import uuid4 @@ -22,6 +24,7 @@ from .creator_plugins import ( Creator, AutoCreator, discover_creator_plugins, + CreatorError, ) UpdateData = collections.namedtuple("UpdateData", ["instance", "changes"]) @@ -63,19 +66,76 @@ class HostMissRequiredMethod(Exception): class CreatorsOperationFailed(Exception): - pass + """Raised when a creator process crashes in 'CreateContext'. + + The exception contains information about the creator and error. The data + are prepared using 'prepare_failed_creator_operation_info' and can be + serialized using json. + + Usage is for UI purposes which may not have access to exceptions directly + and would not have ability to catch exceptions 'per creator'. + + Args: + msg (str): General error message. + failed_info (list[dict[str, Any]]): List of failed creators with + exception message and optionally formatted traceback. + """ + + def __init__(self, msg, failed_info): + super(CreatorsOperationFailed, self).__init__(msg) + self.failed_info = failed_info class CreatorsCollectionFailed(CreatorsOperationFailed): - pass + def __init__(self, failed_info): + msg = "Failed to collect instances" + super(CreatorsCollectionFailed, self).__init__( + msg, failed_info + ) class CreatorsSaveFailed(CreatorsOperationFailed): - pass + def __init__(self, failed_info): + msg = "Failed update instance changes" + super(CreatorsSaveFailed, self).__init__( + msg, failed_info + ) class CreatorsRemoveFailed(CreatorsOperationFailed): - pass + def __init__(self, failed_info): + msg = "Failed to remove instances" + super(CreatorsRemoveFailed, self).__init__( + msg, failed_info + ) + + +class CreatorsCreateFailed(CreatorsOperationFailed): + def __init__(self, failed_info): + msg = "Faled to create instances" + super(CreatorsCreateFailed, self).__init__( + msg, failed_info + ) + + +def prepare_failed_creator_operation_info( + identifier, label, exc_info, add_traceback=True +): + formatted_traceback = None + exc_type, exc_value, exc_traceback = exc_info + error_msg = str(exc_value) + + if add_traceback: + formatted_traceback = "".join(traceback.format_exception( + exc_type, exc_value, exc_traceback + )) + + return { + "creator_identifier": identifier, + "creator_label": label, + "message": error_msg, + "traceback": formatted_traceback + } class InstanceMember: @@ -1202,7 +1262,67 @@ class CreateContext: with self.bulk_instances_collection(): self._bulk_instances_to_process.append(instance) + def create(self, identifier, *args, **kwargs): + """Wrapper for creators to trigger created. + + Different types of creators may expect different arguments thus the + hints for args are blind. + + Args: + identifier (str): Creator's identifier. + *args (Tuple[Any]): Arguments for create method. + **kwargs (Dict[Any, Any]): Keyword argument for create method. + """ + + creator = self.creators.get(identifier) + label = getattr(creator, "label", None) + failed = False + add_traceback = False + try: + # Fake CreatorError (Could be maybe specific exception?) + if creator is None: + raise CreatorError( + "Creator {} was not found".format(identifier) + ) + + creator.create(*args, **kwargs) + + except CreatorError: + failed = True + exc_info = sys.exc_info() + + except: + failed = True + add_traceback = True + exc_info = sys.exc_info() + + if not failed: + return + + self.log.warning( + ( + "Failed to run Creator with identifier \"{}\"." + ).format(identifier), + exc_info=add_traceback + ) + + raise CreatorsCreateFailed([ + prepare_failed_creator_operation_info( + identifier, label, exc_info, add_traceback + ) + ]) + def creator_removed_instance(self, instance): + """When creator removes instance context should be acknowledged. + + If creator removes instance conext should know about it to avoid + possible issues in the session. + + Args: + instance (CreatedInstance): Object of instance which was removed + from scene metadata. + """ + self._instances_by_id.pop(instance.id, None) @contextmanager @@ -1237,42 +1357,72 @@ class CreateContext: self._instances_by_id = {} # Collect instances - failed_creators = [] + failed_info = [] for creator in self.creators.values(): + label = creator.label try: creator.collect_instances() except: - failed_creators.append(creator) + exc_info = sys.exc_info() + identifier = creator.identifier self.log.warning( - "Collection of instances for creator {} ({}) failed".format( - creator.label, creator.identifier), + ( + "Collection of instances for creator {} failed" + ).format(identifier), exc_info=True ) + failed_info.append( + prepare_failed_creator_operation_info( + identifier, label, exc_info + ) + ) - if failed_creators: - joined_creators = ", ".join( - [creator.label for creator in failed_creators] - ) - - raise CreatorsCollectionFailed( - "Failed to collect instances of creators {}".format(joined_creators) - ) + if failed_info: + raise CreatorsCollectionFailed(failed_info) def execute_autocreators(self): """Execute discovered AutoCreator plugins. Reset instances if any autocreator executed properly. """ + + failed_info = [] for identifier, creator in self.autocreators.items(): + label = creator.label + failed = False + add_traceback = False try: creator.create() - except Exception: - # TODO raise report exception if any crashed - msg = ( - "Failed to run AutoCreator with identifier \"{}\" ({})." - ).format(identifier, inspect.getfile(creator.__class__)) - self.log.warning(msg, exc_info=True) + except CreatorError: + failed = True + exc_info = sys.exc_info() + + # Use bare except because some hosts raise their exceptions that + # do not inherit from python's `BaseException` + except: + failed = True + add_traceback = True + exc_info = sys.exc_info() + + if not failed: + continue + + failed_info.append( + prepare_failed_creator_operation_info( + identifier, label, exc_info, add_traceback + ) + ) + + self.log.warning( + ( + "Failed to run AutoCreator with identifier \"{}\"." + ).format(identifier), + exc_info=exc_info + ) + + if failed_info: + raise CreatorsCreateFailed(failed_info) def validate_instances_context(self, instances=None): """Validate 'asset' and 'task' instance context.""" @@ -1349,7 +1499,7 @@ class CreateContext: identifier = instance.creator_identifier instances_by_identifier[identifier].append(instance) - failed_creators = [] + failed_info = [] for identifier, creator_instances in instances_by_identifier.items(): update_list = [] for instance in creator_instances: @@ -1358,26 +1508,29 @@ class CreateContext: update_list.append(UpdateData(instance, instance_changes)) creator = self.creators[identifier] - if update_list: - try: - creator.update_instances(update_list) + if not update_list: + continue - except: - failed_creators.append(creator) - self.log.warning( - "Instances update of creator {} ({}) failed".format( - creator.label, creator.identifier), - exc_info=True + label = creator.label + try: + creator.update_instances(update_list) + + except: + exc_info = sys.exc_info() + self.log.warning( + "Instances update of creator \"{}\" failed".format( + identifier), + exc_info=True + ) + + failed_info.append( + prepare_failed_creator_operation_info( + identifier, label, exc_info ) + ) - if failed_creators: - joined_creators = ", ".join( - [creator.label for creator in failed_creators] - ) - - raise CreatorsSaveFailed( - "Failed save changes of creators {}".format(joined_creators) - ) + if failed_info: + raise CreatorsSaveFailed(failed_info) def remove_instances(self, instances): """Remove instances from context. @@ -1392,30 +1545,27 @@ class CreateContext: identifier = instance.creator_identifier instances_by_identifier[identifier].append(instance) - failed_creators = [] + failed_info = [] for identifier, creator_instances in instances_by_identifier.items(): creator = self.creators.get(identifier) + label = creator.label try: creator.remove_instances(creator_instances) except: - failed_creators.append(creator) + exc_info = sys.exc_info() self.log.warning( - "Instances removement of creator {} ({}) failed".format( - creator.label, creator.identifier), + "Instances removement of creator \"{}\" failed".format( + identifier), exc_info=True ) - - if failed_creators: - joined_creators = ", ".join( - [creator.label for creator in failed_creators] - ) - - raise CreatorsRemoveFailed( - "Failed to remove instances of creators {}".format( - joined_creators + failed_info.append( + prepare_failed_creator_operation_info( + identifier, label, exc_info + ) ) - ) + if failed_info: + raise CreatorsRemoveFailed(failed_info) def _get_publish_plugins_with_attr_for_family(self, family): """Publish plugin attributes for passed family.