Problem
I am starting to learn network programming. I have implemented a very simple chat server based upon TCP, where the users only need to provide username.
The assumption is that the server makes use of no additional threads and the sockets are not blocking. The code:
import sys
import time
import socket
import select
class User:
def __init__(self, name, sock):
self.name = name
self.sock = sock
class Server:
def __init__(self, host, port):
self.users = []
self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
self.sock.bind((host, port))
def accept_usr(self):
usock, _ = self.sock.accept()
usock.send(b"Please, choose your username: ")
uname = usock.recv(1024)
self.users.append(User(uname, usock))
_, writes, _ = select.select([], self._usocks(), [])
for w in writes:
w.send(bytes(f"{uname} has joined us!", encoding="ascii"))
def handlerr(self, errs):
for s in errs:
s.close()
def update(self):
reads, writes, errs = select.select([*self._usocks(), self.sock], self._usocks(), self._usocks())
for r in reads:
if r is self.sock:
self.accept_usr()
else:
uname = [u for u in self.users if u.sock is u.sock][0].name
msg = r.recv(1024)
for w in writes:
w.send(bytes(f"{uname}: {msg}", encoding="ascii"))
self.handlerr(errs)
def run(self):
self.sock.listen(5)
while 1:
self.update()
time.sleep(0.1)
def _usocks(self):
return [u.sock for u in self.users]
if __name__ == "__main__":
s = Server("localhost", int(sys.argv[1]))
s.run()
I would be grateful for any hints and comments.
EDIT:
An obvious improvement that comes to my mind is to store the mapping of socket->User in a dict so that I’ll be able to quickly determine the author of the message being sent.
Solution
Packet fragmentation
Have a read through this: https://stackoverflow.com/a/17668009/313768
usock.recv(1024) is fragile, and under many circumstances simply won’t work for a handful of reasons. You’ll need to do some reading about the fundamentals of TCP/IP to understand why that is, but basically, you can’t assume that you’ll get the data you want all in one chunk.
Context manager
You hold onto some sockets, which are resources that should be closed when you’re done with them even if something goes wrong. This is a job for a context manager. You can make your class a context manager so that calling code can be sure that your resources are closed off, regardless of what else happens.
Have a read through this: https://docs.python.org/3/library/stdtypes.html#typecontextmanager
Presentation versus logic
usock.send(b"Please, choose your username: ")
Typically, this is not done, and instead a pre-chosen number is sent over the wire, leaving the client to choose how to ask the user a particular thing. This becomes especially important when graphics get involved, or when you need to do translation/internationalization.
The first thing
[u for u in self.users if u.sock is u.sock][0].name
should be
next(u for u in self.users if u.sock is u.sock).name
That said… what on earth is this doing? When would that condition ever evaluate to False?
Type hints
PEP484 type hints, such as host: str, port: int, will clarify your function arguments.