From 98271c421412467fa387f3a6530fe8d24e360fa4 Mon Sep 17 00:00:00 2001 From: Askaholic Date: Wed, 10 Jan 2024 00:12:06 -0500 Subject: [PATCH] Fix v1.14.0 prod errors (#999) * Fix bad reference to _player * Fix duplicate key error for social_add * Fix duplicate key error for coop_leaderboard * Add user agent to ClientError log message This would help to keep track of which errors are relevant to the latest client build and which can be ignored because they have already been fixed or are coming from an alternate client implementation. --- server/gameconnection.py | 29 ++++++++++-------- server/games/coop.py | 3 ++ server/lobbyconnection.py | 9 +++++- tests/conftest.py | 6 ++-- tests/unit_tests/test_gameconnection.py | 23 ++++++++------ tests/unit_tests/test_lobbyconnection.py | 38 ++++++++++++++++++++---- 6 files changed, 77 insertions(+), 31 deletions(-) diff --git a/server/gameconnection.py b/server/gameconnection.py index 958a52ac0..6d2e10c87 100644 --- a/server/gameconnection.py +++ b/server/gameconnection.py @@ -288,7 +288,11 @@ async def handle_game_result(self, army: Any, result: Any): self._logger.warning("Invalid result for %s reported: %s", army, result) else: await self.game.add_result( - self.player.id, army, result_type, int(score), frozenset(metadata) + self.player.id, + army, + result_type, + int(score), + frozenset(metadata), ) async def handle_operation_complete( @@ -332,17 +336,18 @@ async def handle_operation_complete( # Each player in a co-op game will send the OperationComplete # message but we only need to perform this insert once - if not self.game.leaderboard_saved: - await conn.execute( - coop_leaderboard.insert().values( - mission=mission, - gameuid=self.game.id, - secondary=secondary, - time=delta, - player_count=len(self.game.players), + async with self.game.leaderboard_lock: + if not self.game.leaderboard_saved: + await conn.execute( + coop_leaderboard.insert().values( + mission=mission, + gameuid=self.game.id, + secondary=secondary, + time=delta, + player_count=len(self.game.players), + ) ) - ) - self.game.leaderboard_saved = True + self.game.leaderboard_saved = True async def handle_json_stats(self, stats: str): try: @@ -350,7 +355,7 @@ async def handle_json_stats(self, stats: str): except json.JSONDecodeError as e: self._logger.warning( "Malformed game stats reported by %s: '...%s...'", - self._player.login, + self.player.login, stats[e.pos-20:e.pos+20] ) diff --git a/server/games/coop.py b/server/games/coop.py index b6a4b27e9..b8531822a 100644 --- a/server/games/coop.py +++ b/server/games/coop.py @@ -1,3 +1,5 @@ +import asyncio + from .game import Game from .typedefs import FA, GameType, InitMode, ValidityState, Victory @@ -19,6 +21,7 @@ def __init__(self, *args, **kwargs): "Difficulty": 3, "Expansion": "true" }) + self.leaderboard_lock = asyncio.Lock() self.leaderboard_saved = False async def validate_game_mode_settings(self): diff --git a/server/lobbyconnection.py b/server/lobbyconnection.py index e7879436f..bf028d8b8 100644 --- a/server/lobbyconnection.py +++ b/server/lobbyconnection.py @@ -189,7 +189,11 @@ async def on_message_received(self, message): }) await self.abort(e.message()) except ClientError as e: - self._logger.warning("Client error: %s", e.message) + self._logger.warning( + "ClientError[%s]: %s", + self.user_agent, + e.message, + ) await self.send({ "command": "notice", "style": "error", @@ -333,6 +337,9 @@ async def command_social_add(self, message): else: return + if subject_id in player_attr: + return + async with self._db.acquire() as conn: await conn.execute(friends_and_foes.insert().values( user_id=self.player.id, diff --git a/tests/conftest.py b/tests/conftest.py index 8d550740e..7b8934f8e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -160,7 +160,7 @@ def opt(val): user=user, password=pw or "", port=port, - db=name + db=name, ) @@ -183,7 +183,7 @@ def opt(val): user=user, password=pw or "", port=port, - db=name + db=name, ) await db.connect() @@ -211,7 +211,7 @@ async def game(database, players): GAME_UID = 1 -COOP_GAME_UID = 1 +COOP_GAME_UID = 2 @pytest.fixture diff --git a/tests/unit_tests/test_gameconnection.py b/tests/unit_tests/test_gameconnection.py index 6269f6ff4..26c47a230 100644 --- a/tests/unit_tests/test_gameconnection.py +++ b/tests/unit_tests/test_gameconnection.py @@ -475,6 +475,14 @@ async def test_json_stats_malformed( await game_connection.handle_action("JsonStats", ['{"stats": {}']) +async def test_handle_json_stats_malformed( + real_game: Game, + game_connection: GameConnection, +): + game_connection.game = real_game + await game_connection.handle_json_stats('{"stats": {}') + + async def test_handle_action_EnforceRating( game: Game, game_connection: GameConnection @@ -519,9 +527,7 @@ async def test_handle_action_TeamkillHappened( async def test_handle_action_TeamkillHappened_AI( - game: Game, game_connection: GameConnection, - database ): # Should fail with a sql constraint error if this isn't handled correctly game_connection.abort = mock.AsyncMock() @@ -667,13 +673,12 @@ async def test_handle_action_OperationComplete_duplicate( ) with caplog.at_level(logging.ERROR): - await game_connection.handle_action( - "OperationComplete", [1, 1, time_taken] - ) - caplog.clear() - await game_connection.handle_action( - "OperationComplete", [1, 1, time_taken] - ) + await asyncio.gather(*( + game_connection.handle_action( + "OperationComplete", [1, 1, time_taken] + ) + for _ in range(10) + )) assert not any( record.exc_info diff --git a/tests/unit_tests/test_lobbyconnection.py b/tests/unit_tests/test_lobbyconnection.py index da41d3e02..b3977531f 100644 --- a/tests/unit_tests/test_lobbyconnection.py +++ b/tests/unit_tests/test_lobbyconnection.py @@ -610,7 +610,7 @@ async def test_command_avatar_list(mocker, lobbyconnection: LobbyConnection): }) -async def test_command_avatar_select(mocker, database, lobbyconnection: LobbyConnection): +async def test_command_avatar_select(database, lobbyconnection: LobbyConnection): lobbyconnection.player.id = 2 # Dostya test user await lobbyconnection.on_message_received({ @@ -656,6 +656,24 @@ async def test_command_social_add_friend(lobbyconnection, database): assert lobbyconnection.player.friends == {2} +async def test_command_social_add_friend_idempotent(lobbyconnection, database): + lobbyconnection.player.id = 1 + + friends = await get_friends(lobbyconnection.player.id, database) + assert friends == [] + assert lobbyconnection.player.friends == set() + + for _ in range(5): + await lobbyconnection.command_social_add({ + "command": "social_add", + "friend": 2 + }) + + friends = await get_friends(lobbyconnection.player.id, database) + assert friends == [2] + assert lobbyconnection.player.friends == {2} + + async def test_command_social_remove_friend(lobbyconnection, database): lobbyconnection.player.id = 2 @@ -672,11 +690,19 @@ async def test_command_social_remove_friend(lobbyconnection, database): assert friends == [] assert lobbyconnection.player.friends == set() - # Removing twice does nothing - await lobbyconnection.on_message_received({ - "command": "social_remove", - "friend": 1 - }) + +async def test_command_social_remove_friend_idempotent(lobbyconnection, database): + lobbyconnection.player.id = 2 + + friends = await get_friends(lobbyconnection.player.id, database) + assert friends == [1] + lobbyconnection.player.friends = {1} + + for _ in range(5): + await lobbyconnection.command_social_remove({ + "command": "social_remove", + "friend": 1 + }) friends = await get_friends(lobbyconnection.player.id, database) assert friends == []