From c6400b66737cf9fafcc85d3ba6b8c01aae1527f4 Mon Sep 17 00:00:00 2001 From: Benjamin S Wolf Date: Sat, 20 Dec 2025 14:06:32 -0800 Subject: [PATCH] Core: Process all player files before reporting errors (#4039) * Process all player files before reporting errors Full tracebacks will still be in the console and in the logs, but this creates a relatively compact summary at the bottom. * Include full typename in output * Update module access and address style comments * Annotate variables * multi-errors: Revert to while loop * Core: Handle each roll in its own try-catch * multi-errors: Updated style and comments * Undo accidental index change * multi-errors: fix last remaining ref to erargs --- Generate.py | 119 ++++++++++++++++++++++++++++++++++++---------------- Utils.py | 32 ++++++++++++++ 2 files changed, 116 insertions(+), 35 deletions(-) diff --git a/Generate.py b/Generate.py index ae575a0a76..5ad50df2b7 100644 --- a/Generate.py +++ b/Generate.py @@ -119,9 +119,9 @@ def main(args=None) -> tuple[argparse.Namespace, int]: else: meta_weights = None - - player_id = 1 - player_files = {} + player_id: int = 1 + player_files: dict[int, str] = {} + player_errors: list[str] = [] for file in os.scandir(args.player_files_path): fname = file.name if file.is_file() and not fname.startswith(".") and not fname.lower().endswith(".ini") and \ @@ -137,7 +137,11 @@ def main(args=None) -> tuple[argparse.Namespace, int]: weights_cache[fname] = tuple(weights_for_file) except Exception as e: - raise ValueError(f"File {fname} is invalid. Please fix your yaml.") from e + logging.exception(f"Exception reading weights in file {fname}") + player_errors.append( + f"{len(player_errors) + 1}. " + f"File {fname} is invalid. Please fix your yaml.\n{Utils.get_all_causes(e)}" + ) # sort dict for consistent results across platforms: weights_cache = {key: value for key, value in sorted(weights_cache.items(), key=lambda k: k[0].casefold())} @@ -152,6 +156,10 @@ def main(args=None) -> tuple[argparse.Namespace, int]: args.multi = max(player_id - 1, args.multi) if args.multi == 0: + if player_errors: + errors = "\n\n".join(player_errors) + raise ValueError(f"Encountered {len(player_errors)} error(s) in player files. " + f"See logs for full tracebacks.\n\n{errors}") raise ValueError( "No individual player files found and number of players is 0. " "Provide individual player files or specify the number of players via host.yaml or --multi." @@ -161,6 +169,10 @@ def main(args=None) -> tuple[argparse.Namespace, int]: f"{seed_name} Seed {seed} with plando: {args.plando}") if not weights_cache: + if player_errors: + errors = "\n\n".join(player_errors) + raise ValueError(f"Encountered {len(player_errors)} error(s) in player files. " + f"See logs for full tracebacks.\n\n{errors}") raise Exception(f"No weights found. " f"Provide a general weights file ({args.weights_file_path}) or individual player files. " f"A mix is also permitted.") @@ -171,10 +183,6 @@ def main(args=None) -> tuple[argparse.Namespace, int]: args.sprite_pool = dict.fromkeys(range(1, args.multi+1), None) args.name = {} - settings_cache: dict[str, tuple[argparse.Namespace, ...]] = \ - {fname: (tuple(roll_settings(yaml, args.plando) for yaml in yamls) if args.sameoptions else None) - for fname, yamls in weights_cache.items()} - if meta_weights: for category_name, category_dict in meta_weights.items(): for key in category_dict: @@ -197,7 +205,24 @@ def main(args=None) -> tuple[argparse.Namespace, int]: else: yaml[category_name][key] = option - player_path_cache = {} + settings_cache: dict[str, tuple[argparse.Namespace, ...]] = {fname: None for fname in weights_cache} + if args.sameoptions: + for fname, yamls in weights_cache.items(): + try: + settings_cache[fname] = tuple(roll_settings(yaml, args.plando) for yaml in yamls) + except Exception as e: + logging.exception(f"Exception reading settings in file {fname}") + player_errors.append( + f"{len(player_errors) + 1}. " + f"File {fname} is invalid. Please fix your yaml.\n{Utils.get_all_causes(e)}" + ) + # Exit early here to avoid throwing the same errors again later + if player_errors: + errors = "\n\n".join(player_errors) + raise ValueError(f"Encountered {len(player_errors)} error(s) in player files. " + f"See logs for full tracebacks.\n\n{errors}") + + player_path_cache: dict[int, str] = {} for player in range(1, args.multi + 1): player_path_cache[player] = player_files.get(player, args.weights_file_path) name_counter = Counter() @@ -206,38 +231,62 @@ def main(args=None) -> tuple[argparse.Namespace, int]: player = 1 while player <= args.multi: path = player_path_cache[player] - if path: + if not path: + player_errors.append(f'No weights specified for player {player}') + player += 1 + continue + + for doc_index, yaml in enumerate(weights_cache[path]): + name = yaml.get("name") try: - settings: tuple[argparse.Namespace, ...] = settings_cache[path] if settings_cache[path] else \ - tuple(roll_settings(yaml, args.plando) for yaml in weights_cache[path]) - for settingsObject in settings: - for k, v in vars(settingsObject).items(): - if v is not None: - try: - getattr(args, k)[player] = v - except AttributeError: - setattr(args, k, {player: v}) - except Exception as e: - raise Exception(f"Error setting {k} to {v} for player {player}") from e + # Use the cached settings object if it exists, otherwise roll settings within the try-catch + # Invariant: settings_cache[path] and weights_cache[path] have the same length + settingsObject: argparse.Namespace = ( + settings_cache[path][doc_index] + if settings_cache[path] + else roll_settings(yaml, args.plando) + ) + + for k, v in vars(settingsObject).items(): + if v is not None: + try: + getattr(args, k)[player] = v + except AttributeError: + setattr(args, k, {player: v}) + except Exception as e: + raise Exception(f"Error setting {k} to {v} for player {player}") from e - # name was not specified - if player not in args.name: - if path == args.weights_file_path: - # weights file, so we need to make the name unique - args.name[player] = f"Player{player}" - else: - # use the filename - args.name[player] = os.path.splitext(os.path.split(path)[-1])[0] - args.name[player] = handle_name(args.name[player], player, name_counter) + # name was not specified + if player not in args.name: + if path == args.weights_file_path: + # weights file, so we need to make the name unique + args.name[player] = f"Player{player}" + else: + # use the filename + args.name[player] = os.path.splitext(os.path.split(path)[-1])[0] + args.name[player] = handle_name(args.name[player], player, name_counter) - player += 1 except Exception as e: - raise ValueError(f"File {path} is invalid. Please fix your yaml.") from e - else: - raise RuntimeError(f'No weights specified for player {player}') + logging.exception(f"Exception reading settings in file {path} document #{doc_index + 1} " + f"(name: {args.name.get(player, name)})") + player_errors.append( + f"{len(player_errors) + 1}. " + f"File {path} document #{doc_index + 1} (name: {args.name.get(player, name)}) is invalid. " + f"Please fix your yaml.\n{Utils.get_all_causes(e)}") + + # increment for each yaml document in the file + player += 1 if len(set(name.lower() for name in args.name.values())) != len(args.name): - raise Exception(f"Names have to be unique. Names: {Counter(name.lower() for name in args.name.values())}") + player_errors.append( + f"{len(player_errors) + 1}. " + f"Names have to be unique. Names: {Counter(name.lower() for name in args.name.values())}" + ) + + if player_errors: + errors = "\n\n".join(player_errors) + raise ValueError(f"Encountered {len(player_errors)} error(s) in player files. " + f"See logs for full tracebacks.\n\n{errors}") return args, seed diff --git a/Utils.py b/Utils.py index 63718bef26..2fe5d0f562 100644 --- a/Utils.py +++ b/Utils.py @@ -1222,3 +1222,35 @@ class DaemonThreadPoolExecutor(concurrent.futures.ThreadPoolExecutor): t.start() self._threads.add(t) # NOTE: don't add to _threads_queues so we don't block on shutdown + + +def get_full_typename(t: type) -> str: + """Returns the full qualified name of a type, including its module (if not builtins).""" + module = t.__module__ + if module and module != "builtins": + return f"{module}.{t.__qualname__}" + return t.__qualname__ + + +def get_all_causes(ex: Exception) -> str: + """Return a string describing the recursive causes of this exception. + + :param ex: The exception to be described. + :return A multiline string starting with the initial exception on the first line and each resulting exception + on subsequent lines with progressive indentation. + + For example: + + ``` + Exception: Invalid value 'bad'. + Which caused: Options.OptionError: Error generating option + Which caused: ValueError: File bad.yaml is invalid. + ``` + """ + cause = ex + causes = [f"{get_full_typename(type(ex))}: {ex}"] + while cause := cause.__cause__: + causes.append(f"{get_full_typename(type(cause))}: {cause}") + top = causes[-1] + others = "".join(f"\n{' ' * (i + 1)}Which caused: {c}" for i, c in enumerate(reversed(causes[:-1]))) + return f"{top}{others}"