Fix empty ValueError in FakeTcpServer

[antoniojkim]

The fact that the the FakeTcpServer is raising an empty ValueError is causing unexpected errors here

fakeredis-py/fakeredis/_tcp_server.py

Lines 101 to 103 in 49c46f7

	elif isinstance(value, Exception):
	prefix = _get_exception_prefix(value)
	self.writer.write(f"-{prefix} {value.args[0]}\r\n".encode())

This is easily fixed by adding proper error messages to the ValueError

[cunla]

Can you write tests for this?

[antoniojkim]

Can you write tests for this?

Done. @cunla can you please re-review?

[cunla]

I am not sure whether this is needed anymore. Check out https://github.com/cunla/fakeredis-py/pull/441/files

[antoniojkim]

I am not sure whether this is needed anymore. Check out https://github.com/cunla/fakeredis-py/pull/441/files

@cunla While you removed the Reader class in your PR, I still see the error happening

Traceback (most recent call last):
  ...
  File ".../fakeredis-py/fakeredis/_tcp_server.py", line 126, in handle
    self.writer.dump(e)
  File ".../fakeredis-py/fakeredis/_tcp_server.py", line 82, in dump
    self.write(f"-{prefix} {value.args[0]}\r\n".encode())
                            ~~~~~~~~~~^^^
IndexError: tuple index out of range

This is reproducible using the latest changes from your PR with the following reproducer

import socket
import time
from contextlib import closing, contextmanager
from threading import Thread

import redis

from fakeredis import TcpFakeServer


@contextmanager
def redis_server():
    host = "127.0.0.1"
    with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as s:
        s.bind(("", 0))
        s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
        port = s.getsockname()[1]

    class TcpFakeServerWithExceptions(TcpFakeServer):
        def handle_error(self, request, client_address):
            super().handle_error(request, client_address)
            # Send an error message back to the client here
            request.sendall(b"An error occurred on the server.")

    server_address = (host, port)
    server = TcpFakeServerWithExceptions(server_address)
    thread = Thread(target=server.serve_forever, daemon=True)
    thread.start()
    time.sleep(0.1)

    try:
        with redis.Redis(host=host, port=port) as r:
            yield r
    finally:
        server.server_close()
        server.shutdown()
        thread.join()


def test_bulk_string_length(redis_server):
    """Test that malformed bulk string input is handled correctly."""
    host = redis_server.connection_pool.connection_kwargs.get("host")
    port = redis_server.connection_pool.connection_kwargs.get("port")

    with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as s:
        s.connect((host, port))
        s.sendall(b"$ 1\ntest")
        data = s.recv(1024).decode()
        assert data != "An error occurred on the server.", data


if __name__ == "__main__":
    with redis_server() as r:
        test_bulk_string_length(r)

[antoniojkim]

The exception appears to originate here

Traceback (most recent call last):
  File ".../fakeredis-py/fakeredis/_tcp_server.py", line 123, in handle
    self.current_client.get_socket().sendall(data)
  File ".../fakeredis-py/fakeredis/_basefakesocket.py", line 360, in sendall
    self._parser.send(data)
  File ".../fakeredis-py/fakeredis/_basefakesocket.py", line 207, in _parse_commands
    line, buf = self._extract_line(buf)
                ^^^^^^^^^^^^^^^^^^^^^^^
  File ".../fakeredis-py/fakeredis/_basefakesocket.py", line 194, in _extract_line
    assert line.endswith(b"\r\n")
           ^^^^^^^^^^^^^^^^^^^^^^
AssertionError

The fix being to just add an exception message here.

Though, one further comment. Checks like these really should be made into proper exceptions instead of using assert