search

ASPP / pelita

Actor-based Toolkit for Interactive Language Education in Python
https://github.com/ASPP/pelita_template
Other
62 stars 68 forks source link

Do not attempt to send an exit message to a team with a fatal error #805

Closed Debilski closed 2 months ago

Debilski commented 2 months ago

As was mentioned in https://github.com/ASPP/pelita/pull/804 we sometimes have timeouts in our CI. In this case, it could be tracked down to the test:

pytest test/test_remote_game.py::test_remote_move_failures[player_move_import_error-False-ModuleNotFoundError]

I cannot reproduce it right now but it is likely that one of two things happens:

The tested player has an import error, so the game setup fails and triggers a fatalerror. This error causes the game to finish and we end up in def check_exit_remote_teams where we notify the players of how the game went down and that they should shut down. As the player has already had a fatal error this can

a) cause a hang in zmq.send (similar to https://github.com/ASPP/pelita/issues/799); this should actually have been taken care of with our helper class ZMQConnection but it will still induce a one second timeout.

b) cause a hang in RemotePlayer.del when the process is terminated or maybe cause it in this method’s call to exit

Since we do not need to send anything to a player with a fatalerror (after all, we should assume that it cannot receive any more messages), we simply stop sending a message.

Coincidentally, this would also be enough to fix https://github.com/ASPP/pelita/issues/799 but our fix in https://github.com/ASPP/pelita/pull/801 is still more generic and appropriate.

Debilski commented 2 months ago

Nice side effect: running the tests in test_remote_game.py is now 10 seconds faster.