From bccee3a556467a138080641e5efa66d8f7351d7c Mon Sep 17 00:00:00 2001 From: Carina Straub Date: Tue, 27 Feb 2024 19:43:55 +0100 Subject: [PATCH 1/9] input validation on client side --- comprl/client/networking.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/comprl/client/networking.py b/comprl/client/networking.py index 95d6b926..904e2d9c 100644 --- a/comprl/client/networking.py +++ b/comprl/client/networking.py @@ -108,7 +108,12 @@ def step(self, obv: list[float]): dict: A dictionary containing the action that should be executed. Example: {"action": 1} """ - return {"action": self.agent.get_step(obv)} + action = self.agent.get_step(obv) + if type(action) is list and all((type(x) is float) for x in action): + return {"action": action} + else: + raise Exception(f"Tried to send an action with wrong type. Only actions of type list[float] can be send.") + @Error.responder def on_error(self, msg): From 42549c40d3e95537013f01200687cdc9471c7aa7 Mon Sep 17 00:00:00 2001 From: Carina Straub Date: Wed, 28 Feb 2024 00:26:29 +0100 Subject: [PATCH 2/9] differentiate in connected and disconnected errors --- comprl/client/networking.py | 5 ++- comprl/server/__main__.py | 7 ++++ comprl/server/interfaces.py | 11 +++++ comprl/server/networking.py | 81 ++++++++++++++++++++++++++----------- 4 files changed, 78 insertions(+), 26 deletions(-) diff --git a/comprl/client/networking.py b/comprl/client/networking.py index 904e2d9c..babc997d 100644 --- a/comprl/client/networking.py +++ b/comprl/client/networking.py @@ -112,8 +112,9 @@ def step(self, obv: list[float]): if type(action) is list and all((type(x) is float) for x in action): return {"action": action} else: - raise Exception(f"Tried to send an action with wrong type. Only actions of type list[float] can be send.") - + raise Exception( + f"Tried to send an action with wrong type. Only actions of type list[float] can be send." + ) @Error.responder def on_error(self, msg): diff --git a/comprl/server/__main__.py b/comprl/server/__main__.py index 49dc12c1..a993d243 100644 --- a/comprl/server/__main__.py +++ b/comprl/server/__main__.py @@ -64,6 +64,13 @@ def on_timeout(self, player: IPlayer, failure, timeout): log.debug(f"Player {player.id} had timeout after {timeout}s") player.disconnect(reason=f"Timeout after {timeout}s") + def on_remote_error(self, player: IPlayer, error: Exception): + """gets called when there is an error in deferred""" + if player.is_connected: + log.error(f"Connected player caused remote error \n {error.with_traceback}") + else: + log.debug("Disconnected player caused remote error") + def on_update(self): """gets called every update cycle""" self.matchmaking.update() diff --git a/comprl/server/interfaces.py b/comprl/server/interfaces.py index 9d6e9665..a52b2012 100644 --- a/comprl/server/interfaces.py +++ b/comprl/server/interfaces.py @@ -23,6 +23,7 @@ class IPlayer(abc.ABC): def __init__(self) -> None: self.id: PlayerID = IDGenerator.generate_player_id() self.user_id: Optional[int] = None + self.is_connected = True @abc.abstractmethod def authenticate(self, result_callback): @@ -318,6 +319,16 @@ def on_timeout(self, player: IPlayer, failure, timeout): """ ... + @abc.abstractmethod + def on_remote_error(self, player: IPlayer, error): + """ + Gets called when an error in deferred occurs. + Args: + player (IPlayer): The player that caused the error. + error (Exception): Error that occurred + """ + ... + @abc.abstractmethod def on_update(self): """ diff --git a/comprl/server/networking.py b/comprl/server/networking.py index 79cdd3a3..461c2a05 100644 --- a/comprl/server/networking.py +++ b/comprl/server/networking.py @@ -1,7 +1,7 @@ """contains the networking components of the server""" import logging as log -from typing import Callable +from typing import Callable, Any from twisted.internet.interfaces import IAddress from twisted.internet.protocol import Protocol, ServerFactory @@ -36,9 +36,10 @@ def __init__(self, boxReceiver=None, locator=None): self.connection_made_callbacks: list[Callable[[], None]] = [] self.connection_lost_callbacks: list[Callable[[], None]] = [] - self.connection_timeout_callbacks: list[Callable[[any, any], None]] = [] + self.connection_timeout_callbacks: list[Callable[[Any, Any], None]] = [] + self.connection_error_callbacks: list[Callable[[Any], None]] = [] - def add_connection_made_callback(self, callback): + def add_connection_made_callback(self, callback: Callable[[], None]): """adds callback that is executed, when the connection is made Args: @@ -49,7 +50,7 @@ def add_connection_made_callback(self, callback): """ self.connection_made_callbacks.append(callback) - def add_connection_lost_callback(self, callback): + def add_connection_lost_callback(self, callback: Callable[[], None]): """ Adds a callback function to be executed when the connection is lost. @@ -61,7 +62,7 @@ def add_connection_lost_callback(self, callback): """ self.connection_lost_callbacks.append(callback) - def add_connection_timeout_callback(self, callback): + def add_connection_timeout_callback(self, callback: Callable[[Any, Any], None]): """ Adds a callback function to be executed when there is a timeout. @@ -73,6 +74,18 @@ def add_connection_timeout_callback(self, callback): """ self.connection_timeout_callbacks.append(callback) + def add_connection_error_callback(self, callback: Callable[[Any], None]): + """ + Adds a callback function to be executed when there occurs an error in deferred. + + Args: + callback: The callback function to be added. + + Returns: + None + """ + self.connection_error_callbacks.append(callback) + def connectionMade(self) -> None: """ Called when the connection to the client is established. @@ -116,6 +129,16 @@ def connectionTimeout(self, failure, timeout) -> None: for c in self.connection_timeout_callbacks: c(failure, timeout) + def connection_error(self, error) -> None: + """Is called when an error in Deferred occurs + + Args: + place : where the error was caused + error : description of the error + """ + for c in self.connection_error_callbacks: + c(error) + def get_token(self, return_callback: Callable[[str], None]) -> None: """ Retrieves a token from the client and calls the return_callback @@ -141,7 +164,7 @@ def callback(res): self.callRemote(Auth) .addCallback(callback=callback) .addTimeout(ConfigProvider.get("timeout"), reactor, self.connectionTimeout) - .addErrback(self.handle_remote_error) + .addErrback(self.connection_error) ) def is_ready(self, return_callback: Callable[[bool], None]) -> bool: @@ -159,7 +182,7 @@ def is_ready(self, return_callback: Callable[[bool], None]) -> bool: self.callRemote(Ready) .addCallback(callback=lambda res: return_callback(res["ready"])) .addTimeout(ConfigProvider.get("timeout"), reactor, self.connectionTimeout) - .addErrback(self.handle_remote_error) + .addErrback(self.connection_error) ) def notify_start(self, game_id: GameID) -> None: @@ -190,7 +213,7 @@ def get_step( self.callRemote(Step, obv=obv) .addCallback(callback=lambda res: return_callback(res["action"])) .addTimeout(ConfigProvider.get("timeout"), reactor, self.connectionTimeout) - .addErrback(self.handle_remote_error) + .addErrback(self.connection_error) ) def notify_end(self, result, stats) -> None: @@ -207,7 +230,7 @@ def notify_end(self, result, stats) -> None: return ( self.callRemote(EndGame, result=result, stats=stats) .addTimeout(ConfigProvider.get("timeout"), reactor, self.connectionTimeout) - .addErrback(self.handle_remote_error) + .addErrback(self.connection_error) ) def send_error(self, msg: str): @@ -221,7 +244,7 @@ def send_error(self, msg: str): None """ return self.callRemote(Error, msg=str.encode(msg)).addErrback( - self.handle_remote_error + self.connection_error ) def disconnect(self): @@ -233,15 +256,6 @@ def disconnect(self): """ self.transport.loseConnection() - def handle_remote_error(place, error): - """Is called when an error in Deferred occurs - - Args: - place : where the error was caused - error : description of the error - """ - log.debug(f"Caught error in remote Callback at {place}") - class COMPPlayer(IPlayer): """Represents a player in the COMP game. @@ -258,6 +272,17 @@ def __init__(self, connection: COMPServerProtocol) -> None: """ super().__init__() self.connection: COMPServerProtocol = connection + self.is_connected: bool = False + + def set_connection(c: bool): + self.is_connected = c + + self.connection.add_connection_made_callback( + callback=lambda: set_connection(True) + ) + self.connection.add_connection_lost_callback( + callback=lambda: set_connection(False) + ) def authenticate(self, result_callback): """Authenticates the player. @@ -269,7 +294,8 @@ def authenticate(self, result_callback): Returns: token (string): The authentication token. """ - self.connection.get_token(result_callback) + if self.is_connected: + self.connection.get_token(result_callback) def is_ready(self, result_callback) -> bool: """Checks if the player is ready to play. @@ -288,7 +314,8 @@ def notify_start(self, game_id: GameID): Args: game_id (GameID): The ID of the game. """ - self.connection.notify_start(game_id=game_id) + if self.is_connected: + self.connection.notify_start(game_id=game_id) def get_action(self, obv, result_callback): """Receives the action from the server. @@ -297,7 +324,8 @@ def get_action(self, obv, result_callback): obv (any): The observation. result_callback (callback function): The callback to handle the result. """ - self.connection.get_step(obv, result_callback) + if self.is_connected: + self.connection.get_step(obv, result_callback) def notify_end(self, result, stats): """Called when the game ends. @@ -306,7 +334,8 @@ def notify_end(self, result, stats): result (any): The result of the game. stats (any): The stats of the game. """ - self.connection.notify_end(result=result, stats=stats) + if self.is_connected: + self.connection.notify_end(result=result, stats=stats) def disconnect(self, reason: str): """Disconnects the player. @@ -323,7 +352,8 @@ def notify_error(self, error: str): Args: error (str): The error message. """ - self.connection.send_error(error) + if self.is_connected: + self.connection.send_error(error) class COMPFactory(ServerFactory): @@ -375,6 +405,9 @@ def buildProtocol(self, addr: IAddress) -> Protocol | None: comp_player, failure, timeout ) ) + protocol.add_connection_error_callback( + lambda error: self.server.on_remote_error(comp_player, error) + ) return protocol From 236b259e059726f9131d562e70b0911bbabd78d4 Mon Sep 17 00:00:00 2001 From: Carina Straub Date: Wed, 28 Feb 2024 00:29:44 +0100 Subject: [PATCH 3/9] formatting --- comprl/client/networking.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/comprl/client/networking.py b/comprl/client/networking.py index babc997d..c94dd741 100644 --- a/comprl/client/networking.py +++ b/comprl/client/networking.py @@ -109,11 +109,12 @@ def step(self, obv: list[float]): Example: {"action": 1} """ action = self.agent.get_step(obv) - if type(action) is list and all((type(x) is float) for x in action): + if isinstance(action, list) and all(isinstance(x, float) for x in action): return {"action": action} else: raise Exception( - f"Tried to send an action with wrong type. Only actions of type list[float] can be send." + "Tried to send an action with wrong type. " + "Only actions of type list[float] can be send." ) @Error.responder From de8c7e462016957f19e04f40287371d8d54e10c9 Mon Sep 17 00:00:00 2001 From: Carina Straub Date: Fri, 8 Mar 2024 23:19:29 +0100 Subject: [PATCH 4/9] Added message to protocol --- comprl/client/networking.py | 12 +++++++++++- comprl/server/networking.py | 16 +++++++++++++++- comprl/shared/commands.py | 6 ++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/comprl/client/networking.py b/comprl/client/networking.py index 95d6b926..b153904d 100644 --- a/comprl/client/networking.py +++ b/comprl/client/networking.py @@ -8,7 +8,7 @@ from twisted.internet import reactor from twisted.internet.endpoints import TCP4ClientEndpoint, connectProtocol -from comprl.shared.commands import Ready, StartGame, EndGame, Step, Auth, Error +from comprl.shared.commands import Ready, StartGame, EndGame, Step, Auth, Error, Message from .interfaces import IAgent @@ -119,6 +119,16 @@ def on_error(self, msg): """ self.agent.on_error(msg=str(msg, encoding="utf-8")) return {} + + @Message.responder + def on_message(self, msg): + """Called if a message from the server is sent. + + Args: + msg (object): The message. + """ + self.agent.on_message(msg=str(msg, encoding="utf-8")) + return {} def connect_agent(agent: IAgent, host: str = "localhost", port: int = 65335): diff --git a/comprl/server/networking.py b/comprl/server/networking.py index 79cdd3a3..5283ea78 100644 --- a/comprl/server/networking.py +++ b/comprl/server/networking.py @@ -11,7 +11,7 @@ from comprl.server.interfaces import IPlayer, IServer from comprl.server.util import ConfigProvider -from comprl.shared.commands import Auth, EndGame, Error, Ready, StartGame, Step +from comprl.shared.commands import Auth, EndGame, Error, Ready, StartGame, Step, Message from comprl.shared.types import GameID VERSION: int = 1 @@ -223,6 +223,20 @@ def send_error(self, msg: str): return self.callRemote(Error, msg=str.encode(msg)).addErrback( self.handle_remote_error ) + + def send_message(self, msg: str): + """ + Send a message string to the client. + + Args: + msg (str): The message to send. + + Returns: + None + """ + return self.callRemote(Message, msg=str.encode(msg)).addErrback( + self.handle_remote_error + ) def disconnect(self): """ diff --git a/comprl/shared/commands.py b/comprl/shared/commands.py index 806abd59..9484d8dc 100644 --- a/comprl/shared/commands.py +++ b/comprl/shared/commands.py @@ -56,3 +56,9 @@ class Error(Command): arguments = [(b"msg", String())] response = [] + +class Message(Command): + """Command interface for a generic message""" + + arguments = [(b"msg", String())] + response = [] \ No newline at end of file From cbad3c96aabe1a779b0baabd763b330aa53456d7 Mon Sep 17 00:00:00 2001 From: Carina Straub Date: Fri, 8 Mar 2024 23:27:16 +0100 Subject: [PATCH 5/9] client supports message --- comprl/client/agent.py | 6 +++++- comprl/client/interfaces.py | 14 +++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/comprl/client/agent.py b/comprl/client/agent.py index 23a3ebff..f1a0f67b 100644 --- a/comprl/client/agent.py +++ b/comprl/client/agent.py @@ -53,6 +53,10 @@ def run(self, token: str, host: str = "localhost", port: int = 65335) -> None: super().run(token) networking.connect_agent(self, host, port) - def on_error(self, msg): + def on_error(self, msg: str): """Called if an error occurred on the server side.""" print(f"Error: {msg}") + + def on_message(self, msg: str): + """Called if a message is sent from the server.""" + print(f"Info: {msg}") diff --git a/comprl/client/interfaces.py b/comprl/client/interfaces.py index 7e1b03f4..2cf998ca 100644 --- a/comprl/client/interfaces.py +++ b/comprl/client/interfaces.py @@ -57,7 +57,7 @@ def get_step(self, obv: list[float]) -> list[float]: """ raise NotImplementedError("step function not implemented") - def on_end_game(self, result, stats) -> None: + def on_end_game(self, result: bool, stats: list[float]) -> None: """ Called when a game ends. @@ -70,11 +70,19 @@ def on_end_game(self, result, stats) -> None: """ pass - def on_error(self, msg): + def on_error(self, msg: str): """ Called when an error occurs. Args: - msg: The error message. + msg (str): The error message. + """ + pass + + def on_message(self, msg: str): + """Called when a message is sent from the server. + + Args: + msg (str): The message """ pass From 7fc4f0df190cfb80ce9596f0382dc108fef1c91c Mon Sep 17 00:00:00 2001 From: Carina Straub Date: Fri, 8 Mar 2024 23:55:29 +0100 Subject: [PATCH 6/9] Server supports message --- comprl/client/networking.py | 2 +- comprl/server/__main__.py | 1 + comprl/server/interfaces.py | 5 +++++ comprl/server/managers.py | 8 +++++++- comprl/server/networking.py | 10 +++++++++- comprl/shared/commands.py | 3 ++- 6 files changed, 25 insertions(+), 4 deletions(-) diff --git a/comprl/client/networking.py b/comprl/client/networking.py index b153904d..5fd8de24 100644 --- a/comprl/client/networking.py +++ b/comprl/client/networking.py @@ -119,7 +119,7 @@ def on_error(self, msg): """ self.agent.on_error(msg=str(msg, encoding="utf-8")) return {} - + @Message.responder def on_message(self, msg): """Called if a message from the server is sent. diff --git a/comprl/server/__main__.py b/comprl/server/__main__.py index 49dc12c1..2b677e5d 100644 --- a/comprl/server/__main__.py +++ b/comprl/server/__main__.py @@ -47,6 +47,7 @@ def on_connect(self, player: IPlayer): def __auth(token): if self.player_manager.auth(player.id, token): self.matchmaking.try_match(player.id) + player.notify_info(msg="Authentication successful") else: player.disconnect("Authentication failed") diff --git a/comprl/server/interfaces.py b/comprl/server/interfaces.py index 9d6e9665..10601b9a 100644 --- a/comprl/server/interfaces.py +++ b/comprl/server/interfaces.py @@ -75,6 +75,11 @@ def notify_error(self, error: str): """notifies the player of an error""" ... + @abc.abstractmethod + def notify_info(self, msg: str): + """notifies the player of an information""" + ... + class IGame(abc.ABC): """ diff --git a/comprl/server/managers.py b/comprl/server/managers.py index 42413395..42fd0005 100644 --- a/comprl/server/managers.py +++ b/comprl/server/managers.py @@ -244,7 +244,13 @@ def try_match(self, player_id: PlayerID) -> None: if player is not None: # FIXME: we might wan't to kick the player - player.is_ready(lambda res: self.match(player_id) if res else None) + + def __match(ready: bool): + if ready: + player.notify_info(msg="Waiting in queue") + self.match(player_id) + + player.is_ready(result_callback=__match) def match(self, player_id: PlayerID) -> None: """ diff --git a/comprl/server/networking.py b/comprl/server/networking.py index 5283ea78..5057f505 100644 --- a/comprl/server/networking.py +++ b/comprl/server/networking.py @@ -223,7 +223,7 @@ def send_error(self, msg: str): return self.callRemote(Error, msg=str.encode(msg)).addErrback( self.handle_remote_error ) - + def send_message(self, msg: str): """ Send a message string to the client. @@ -339,6 +339,14 @@ def notify_error(self, error: str): """ self.connection.send_error(error) + def notify_info(self, msg: str): + """Notifies the player of an information. + + Args: + msg (str): The information message. + """ + self.connection.send_message(msg) + class COMPFactory(ServerFactory): """Factory for COMP servers. diff --git a/comprl/shared/commands.py b/comprl/shared/commands.py index 9484d8dc..77007ccd 100644 --- a/comprl/shared/commands.py +++ b/comprl/shared/commands.py @@ -57,8 +57,9 @@ class Error(Command): arguments = [(b"msg", String())] response = [] + class Message(Command): """Command interface for a generic message""" arguments = [(b"msg", String())] - response = [] \ No newline at end of file + response = [] From 2cea4549ef802a5857479aea0619b1dddfe3fd49 Mon Sep 17 00:00:00 2001 From: Carina Straub Date: Sat, 9 Mar 2024 11:13:28 +0100 Subject: [PATCH 7/9] Added requested changes --- comprl/server/__main__.py | 2 +- comprl/server/interfaces.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/comprl/server/__main__.py b/comprl/server/__main__.py index a993d243..c1048ab0 100644 --- a/comprl/server/__main__.py +++ b/comprl/server/__main__.py @@ -67,7 +67,7 @@ def on_timeout(self, player: IPlayer, failure, timeout): def on_remote_error(self, player: IPlayer, error: Exception): """gets called when there is an error in deferred""" if player.is_connected: - log.error(f"Connected player caused remote error \n {error.with_traceback}") + log.error(f"Connected player caused remote error \n {error}") else: log.debug("Disconnected player caused remote error") diff --git a/comprl/server/interfaces.py b/comprl/server/interfaces.py index a52b2012..eeb487ee 100644 --- a/comprl/server/interfaces.py +++ b/comprl/server/interfaces.py @@ -139,6 +139,8 @@ def _end(self, reason="unknown"): callback(self) for player in self.players.values(): + if player.id == self.disconnected_player_id: + continue player.notify_end( self._player_won(player.id), self._player_stats(player.id) ) From 5b6a87e99b58fe65c15de61ca7d1b7e9c67d24f9 Mon Sep 17 00:00:00 2001 From: Carina Straub Date: Sat, 9 Mar 2024 11:38:24 +0100 Subject: [PATCH 8/9] Added disconnect message on client side --- comprl/client/agent.py | 4 ++++ comprl/client/interfaces.py | 4 ++++ comprl/client/networking.py | 1 + 3 files changed, 9 insertions(+) diff --git a/comprl/client/agent.py b/comprl/client/agent.py index f1a0f67b..a2329722 100644 --- a/comprl/client/agent.py +++ b/comprl/client/agent.py @@ -60,3 +60,7 @@ def on_error(self, msg: str): def on_message(self, msg: str): """Called if a message is sent from the server.""" print(f"Info: {msg}") + + def on_disconnect(self): + """Called when the agent disconnects from the server.""" + print("Error: Agent disconnected from the server.") diff --git a/comprl/client/interfaces.py b/comprl/client/interfaces.py index 2cf998ca..3cabd6f9 100644 --- a/comprl/client/interfaces.py +++ b/comprl/client/interfaces.py @@ -86,3 +86,7 @@ def on_message(self, msg: str): msg (str): The message """ pass + + def on_disconnect(self): + """Called when the agent disconnects from the server.""" + pass diff --git a/comprl/client/networking.py b/comprl/client/networking.py index 5fd8de24..548638d8 100644 --- a/comprl/client/networking.py +++ b/comprl/client/networking.py @@ -46,6 +46,7 @@ def connectionLost(self, reason): log.debug(f"Disconnected from the server. Reason: {reason}") if reactor.running: reactor.stop() + self.agent.on_disconnect() return super().connectionLost(reason) @Auth.responder From 27cb12d8d9ab90a821c934c74a27785cbbfb0fd0 Mon Sep 17 00:00:00 2001 From: Carina Straub Date: Mon, 11 Mar 2024 11:24:45 +0100 Subject: [PATCH 9/9] Fixed automatic update --- comprl/server/networking.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/comprl/server/networking.py b/comprl/server/networking.py index 7b290a70..3573dd6f 100644 --- a/comprl/server/networking.py +++ b/comprl/server/networking.py @@ -258,7 +258,7 @@ def send_message(self, msg: str): None """ return self.callRemote(Message, msg=str.encode(msg)).addErrback( - self.handle_remote_error + self.connection_error ) def disconnect(self):