From 184129fca02bf9b23e51f6dc51dd1ec47aa65caf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Ke=C3=9Fler?= Date: Wed, 5 Jul 2023 09:00:03 +0200 Subject: [PATCH] improve error handling when downloading games: Throw proper assertions, assume nothing about returned data --- hanabi/hanab_game.py | 32 +++++++++-- hanabi/live/download_data.py | 105 ++++++++++++++++++++++++++++------- hanabi/live/site_api.py | 5 +- 3 files changed, 118 insertions(+), 24 deletions(-) diff --git a/hanabi/hanab_game.py b/hanabi/hanab_game.py index 92900c7..c4ec23a 100644 --- a/hanabi/hanab_game.py +++ b/hanabi/hanab_game.py @@ -5,6 +5,10 @@ from termcolor import colored from hanabi import constants +class ParseError(ValueError): + pass + + class DeckCard: def __init__(self, suitIndex: int, rank: int, deck_index=None): self.suitIndex: int = suitIndex @@ -13,7 +17,13 @@ class DeckCard: @staticmethod def from_json(deck_card): - return DeckCard(**deck_card) + suit_index = deck_card.get('suitIndex', None) + rank = deck_card.get('rank', None) + if suit_index is None: + raise ParseError("No suit index specified in deck_card") + if rank is None: + raise ParseError("No rank specified in deck_card") + return DeckCard(suit_index, rank) def colorize(self): color = ["green", "blue", "magenta", "yellow", "white", "cyan"][self.suitIndex] @@ -54,10 +64,24 @@ class Action: @staticmethod def from_json(action): + action_type_int = action.get('type', None) + action_target = action.get('target', None) + action_value = action.get('value', None) + if action_type_int is None: + raise ParseError("No action type specified in action, found {}".format(action_type)) + if action_target is None: + raise ParseError("No action target specified in action, found {}".format(action_target)) + for val in [action_type_int, action_target, action_value]: + if val is not None and type(val) != int: + raise ParseError("Invalid data type in action, expected int, found {}".format(type(val))) + try: + action_type = ActionType(action_type_int) + except ValueError as e: + raise ParseError("Invalid action type, found {}".format(action_type_int)) from e return Action( - ActionType(action['type']), - int(action['target']), - action.get('value', None) + action_type, + action_target, + action_value ) def __repr__(self): diff --git a/hanabi/live/download_data.py b/hanabi/live/download_data.py index b4cbf66..8464630 100644 --- a/hanabi/live/download_data.py +++ b/hanabi/live/download_data.py @@ -13,30 +13,84 @@ from hanabi.live import hanab_live from hanabi import logger +class GameExportError(ValueError): + def __init__(self, game_id, msg): + super().__init__("When exporting game {}: {}".format(game_id, msg)) + pass + + +class GameExportNoResponseFromSiteError(GameExportError): + def __init__(self, game_id): + super().__init__(game_id, "No response from site") + + +class GameExportInvalidResponseTypeError(GameExportError): + def __init__(self, game_id, response_type): + super().__init__(game_id, "Invalid response type (expected json, got {})".format( + response_type, game_id + )) + pass + + +class GameExportInvalidFormatError(GameExportError): + def __init__(self, game_id, msg): + super().__init__("Invalid response format: {}".format(game_id), msg) + + +class GameExportInvalidNumberOfPlayersError(GameExportInvalidFormatError): + def __init__(self, game_id, expected, received): + super().__init__(game_id, "Received invalid list of players: Expected {}, got {}".format(expected, received)) + pass + + # -def detailed_export_game(game_id: int, score: Optional[int] = None, var_id: Optional[int] = None, - seed_exists: bool = False) -> None: +def detailed_export_game( + game_id: int, + score: Optional[int] = None, + num_players: Optional[int] = None, + var_id: Optional[int] = None, + seed_exists: bool = False + ) -> None: """ - Downloads full details of game, inserts seed and game into DB + Downloads full details of game from hanab.live, inserts seed and game into DB If seed is already present, it is left as is If game is already present, game details will be updated - :param game_id: + :param game_id: Id of game to export :param score: If given, this will be inserted as score of the game. If not given, score is calculated - :param var_id If given, this will be inserted as variant id of the game. If not given, this is looked up + :param num_players: If given, the number of players reported by the site is checked against this. If inconsistent, + InvalidNumberOfPlayersError is raised + :param var_id: If given, this will be inserted as variant id of the game. If not given, this is looked up :param seed_exists: If specified and true, assumes that the seed is already present in database. If this is not the case, call will raise a DB insertion error + + :raises GameExportError and its child classes """ + logger.debug("Importing game {}".format(game_id)) - assert_msg = "Invalid response format from hanab.live while exporting game id {}".format(game_id) - game_json = site_api.get("export/{}".format(game_id)) - assert game_json.get('id') == game_id, assert_msg + if game_json is None: + raise GameExportNoResponseFromSiteError + if type(game_json) != dict: + raise GameExportInvalidResponseTypeError(game_id, type(game_json)) + + if game_json.get('id', None) != game_id: + raise GameExportInvalidFormatError(game_id, "Unexpected game_id {} received, expected {}".format( + game_json.get('id'), game_id + )) players = game_json.get('players', []) + if num_players is not None and len(players) != num_players: + raise GameExportInvalidNumberOfPlayersError(game_id, num_players, game_json.get('players', [])) num_players = len(players) + if num_players < 2: + raise GameExportInvalidNumberOfPlayersError(game_id, "≥2", num_players) + seed = game_json.get('seed', None) + if type(seed) != str: + raise GameExportInvalidFormatError(game_id, "Unexpected seed, expected string, got {}".format(seed)) + options = game_json.get('options', {}) var_id = var_id or variants.variant_id(options.get('variant', 'No Variant')) deck_plays = options.get('deckPlays', False) @@ -44,11 +98,16 @@ def detailed_export_game(game_id: int, score: Optional[int] = None, var_id: Opti one_less_card = options.get('oneLessCard', False) all_or_nothing = options.get('allOrNothing', False) starting_player = options.get('startingPlayer', 0) - actions = [hanab_game.Action.from_json(action) for action in game_json.get('actions', [])] - deck = [hanab_game.DeckCard.from_json(card) for card in game_json.get('deck', None)] - assert players != [], assert_msg - assert seed is not None, assert_msg + try: + actions = [hanab_game.Action.from_json(action) for action in game_json.get('actions', [])] + except hanab_game.ParseError as e: + raise GameExportInvalidFormatError(game_id, "Failed to parse actions") from e + + try: + deck = [hanab_game.DeckCard.from_json(card) for card in game_json.get('deck', None)] + except hanab_game.ParseError as e: + raise GameExportInvalidFormatError(game_id, "Failed to parse deck") from e if score is None: # need to play through the game once to find out its score @@ -69,14 +128,15 @@ def detailed_export_game(game_id: int, score: Optional[int] = None, var_id: Opti try: compressed_deck = compress.compress_deck(deck) - except compress.InvalidFormatError: + except compress.InvalidFormatError as e: logger.error("Failed to compress deck while exporting game {}: {}".format(game_id, deck)) - raise + raise GameExportInvalidFormatError(game_id, "Failed to compress deck") from e + try: compressed_actions = compress.compress_actions(actions) - except compress.InvalidFormatError: + except compress.InvalidFormatError as e: logger.error("Failed to compress actions while exporting game {}".format(game_id)) - raise + raise GameExportInvalidFormatError(game_id, "Failed to compress actions") from e if not seed_exists: database.cur.execute( @@ -107,7 +167,7 @@ def detailed_export_game(game_id: int, score: Optional[int] = None, var_id: Opti logger.debug("Imported game {}".format(game_id)) -def process_game_row(game: Dict, var_id): +def process_game_row(game: Dict, var_id, export_all_games: bool = False): game_id = game.get('id', None) seed = game.get('seed', None) num_players = game.get('num_players', None) @@ -116,6 +176,11 @@ def process_game_row(game: Dict, var_id): if any(v is None for v in [game_id, seed, num_players, score]): raise ValueError("Unknown response format on hanab.live") + if export_all_games: + detailed_export_game(game_id, score=score, num_players=num_players, var_id=var_id) + logger.debug("Imported game {}".format(game_id)) + return + database.cur.execute("SAVEPOINT seed_insert") try: database.cur.execute( @@ -126,13 +191,15 @@ def process_game_row(game: Dict, var_id): (game_id, seed, num_players, score, var_id) ) except psycopg2.errors.ForeignKeyViolation: + # Sometimes, seed is not present in the database yet, then we will have to query the full game details + # (including the seed) to export it accordingly database.cur.execute("ROLLBACK TO seed_insert") detailed_export_game(game_id, score, var_id) database.cur.execute("RELEASE seed_insert") logger.debug("Imported game {}".format(game_id)) -def download_games(var_id): +def download_games(var_id, export_all_games: bool = False): name = variants.variant_name(var_id) page_size = 100 if name is None: @@ -179,7 +246,7 @@ def download_games(var_id): if not (page == last_page or len(rows) == page_size): logger.warn('WARN: received unexpected row count ({}) on page {}'.format(len(rows), page)) for row in rows: - process_game_row(row, var_id) + process_game_row(row, var_id, export_all_games) bar() database.cur.execute( "INSERT INTO variant_game_downloads (variant_id, last_game_id) VALUES" diff --git a/hanabi/live/site_api.py b/hanabi/live/site_api.py index fbd4d90..41fbfc5 100644 --- a/hanabi/live/site_api.py +++ b/hanabi/live/site_api.py @@ -1,4 +1,5 @@ import json +from typing import Optional, Dict import requests_cache import platformdirs @@ -10,7 +11,7 @@ from hanabi import constants session = requests_cache.CachedSession(platformdirs.user_cache_dir(constants.APP_NAME) + '/hanab.live') -def get(url, refresh=False): +def get(url, refresh=False) -> Optional[Dict | str]: # print("sending request for " + url) query = "https://hanab.live/" + url logger.debug("GET {} (force_refresh={})".format(query, refresh)) @@ -19,9 +20,11 @@ def get(url, refresh=False): logger.error("Failed to get request {} from hanab.live".format(query)) return None if not response.status_code == 200: + logger.error("Request {} from hanab.live produced status code {}".format(query, response.status_code)) return None if "application/json" in response.headers['content-type']: return json.loads(response.text) + return response.text def api(url, refresh=False):