3
\$\begingroup\$

I'm currently writing a python script which needs some simple networking. After some playing around with sockets, this emerged, but as this is my first foray into sockets I'd really like a second opinion.

Edit: I know my script is not PEP8 compliant, as this is just an experiment. I posted it here to get Feedback about my usage of Sockets, and if there are any pitfalls or corner cases I overlooked.

Package Format:

datalength+1 \0 data \0

import socket

sep: bytes = b'\0'

minpacketsize: int = 10


def sendpacket(sock: socket.socket, data: bytes) -> None:
    send: bytes = str(len(data) + 1).encode('utf-8') + sep + data + sep

    if len(send) < minpacketsize:
        raise ValueError("Packet too small!")

    sock.sendall(send)


def recivepacket(sock: socket.socket) -> bytes:
    datalen = 0
    header: bytearray = bytearray()
    while not datalen:
        header += sock.recv(1)
        if header.endswith(sep):
            try:
                datalen = int(header[:-1].decode('utf-8'))
            except UnicodeDecodeError or ValueError as e:
                raise ValueError("Error while decoding header!") from e

        if len(header) == 10:
            raise ValueError("Could not find Header!")

    rawdata: bytes = sock.recv(datalen)

    if rawdata.endswith(sep):
        return rawdata[:-1]
    else:
        raise ValueError("Could not find Packet end!")
\$\endgroup\$
1
\$\begingroup\$

Docstrings

You should include a docstring at the beginning of every method, class, and module you write. This will allow documentation to identify what your code is supposed to do. It also helps external text editors, such as VSCode, to display what types of parameters your functions accept, and a short description of your method.

Error or Error

This line

except UnicodeDecodeError or ValueError as e:

should be this

except (UnicodeDecodeError, ValueError) as e:

Your current line of code raises this warning from pylint:

Exception to catch is the result of a binary "or" operation

If you need to handle multiple errors, you should group them into a tuple.

Method Names

According to PEP-8 Function Name Conventions, your method names sendpacket and recivepacket should really be send_packet and receive_packet.

Unnecessary else

This section of code

if rawdata.endswith(sep):
    return rawdata[:-1]
else:
    raise ValueError("Could not find Packet end!")

should be this

if rawdata.endswith(sep):
    return rawdata[:-1]
raise ValueError("Could not find Packet end!")

Since you are returning a value if the condition is satisfied, the ValueError line will not run. And if the condition is not satisfied, it will simply move to the next line. Therefore, the else is unnecessary and should be removed.

Updated Code

"""
Description of this module goes here
"""

import socket

sep: bytes = b'\0'

minpacketsize: int = 10


def send_packet(sock: socket.socket, data: bytes) -> None:
    """

    Description of this method goes here

    :param sock -> socket.socket: Socket to send the data over\n
    :param data -> bytes: Data to send over the socket

    :return: None
    """
    send: bytes = str(len(data) + 1).encode('utf-8') + sep + data + sep

    if len(send) < minpacketsize:
        raise ValueError("Packet too small!")

    sock.sendall(send)


def receive_packet(sock: socket.socket) -> bytes:
    """

    Description of this method goes here

    :param sock -> socket.socket: Socket to receive the data over

    :return: bytes
    """
    datalen = 0
    header: bytearray = bytearray()
    while not datalen:
        header += sock.recv(1)
        if header.endswith(sep):
            try:
                datalen = int(header[:-1].decode('utf-8'))
            except (UnicodeDecodeError, ValueError) as e:
                raise ValueError("Error while decoding header!") from e

        if len(header) == 10:
            raise ValueError("Could not find Header!")

    rawdata: bytes = sock.recv(datalen)

    if rawdata.endswith(sep):
        return rawdata[:-1]
    raise ValueError("Could not find Packet end!")
\$\endgroup\$
  • \$\begingroup\$ Thanks for catching the exception handling error. I don't want to sound rude, but I know PEP8. This is just a quick and dirty experimentation script, and I posted it here to get feedback on my usage of sockets, not my PEP8 compliance. I will edit my Question to clarify. \$\endgroup\$ – Xtrem532 Nov 15 at 19:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.