From 042009b544df4a159f286f29dd03c245f0e3aaf8 Mon Sep 17 00:00:00 2001 From: Azizul Haque Ananto Date: Sun, 22 Oct 2023 19:37:40 +0200 Subject: [PATCH] Fix http request handle issue (#42) * Fix http request handle issue * Add docker tests and coverage upload only on ubuntu --- .github/workflows/codecov.yml | 12 ++++++-- Dockerfile.test.py310 | 9 ++++++ Dockerfile.test.py38 | 9 ++++++ Dockerfile.test.py39 | 9 ++++++ Makefile | 13 ++++++++- .../single_server/client_generation_test.py | 3 ++ .../single_server/client_server_test.py | 28 +++++++++++++++++++ tests/functional/single_server/client_test.py | 15 ++++++++-- tests/functional/single_server/server.py | 5 ++++ tests/requirements.txt | 4 ++- tests/unit/test_server.py | 10 +++---- zero/client_server/worker.py | 10 +++++-- zero/error.py | 5 ++++ 13 files changed, 117 insertions(+), 15 deletions(-) create mode 100644 Dockerfile.test.py310 create mode 100644 Dockerfile.test.py38 create mode 100644 Dockerfile.test.py39 diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml index 7f9dc3f..b94192b 100644 --- a/.github/workflows/codecov.yml +++ b/.github/workflows/codecov.yml @@ -16,15 +16,21 @@ jobs: os: [ubuntu-latest, macos-latest, windows-latest] python-version: ["3.8", "3.9", "3.10"] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} - - name: Generate Report - timeout-minutes: 10 + cache: pip + - name: Install dependencies + timeout-minutes: 5 run: | + python -m pip install pip==23.3.1 pip install -r tests/requirements.txt + - name: Run tests + timeout-minutes: 5 + run: | make test - name: Upload Coverage to Codecov + if: ${{ matrix.os == 'ubuntu-latest' && matrix.python-version == '3.9' }} uses: codecov/codecov-action@v3 diff --git a/Dockerfile.test.py310 b/Dockerfile.test.py310 new file mode 100644 index 0000000..fc33837 --- /dev/null +++ b/Dockerfile.test.py310 @@ -0,0 +1,9 @@ +FROM python:3.10-slim + +COPY tests/requirements.txt . +RUN pip install -r requirements.txt + +COPY zero ./zero +COPY tests ./tests + +CMD ["pytest", "tests", "--cov=zero", "--cov-report=term-missing", "-vv"] diff --git a/Dockerfile.test.py38 b/Dockerfile.test.py38 new file mode 100644 index 0000000..209a32e --- /dev/null +++ b/Dockerfile.test.py38 @@ -0,0 +1,9 @@ +FROM python:3.8-slim + +COPY tests/requirements.txt . +RUN pip install -r requirements.txt + +COPY zero ./zero +COPY tests ./tests + +CMD ["pytest", "tests", "--cov=zero", "--cov-report=term-missing", "-vv"] diff --git a/Dockerfile.test.py39 b/Dockerfile.test.py39 new file mode 100644 index 0000000..5e6c2fd --- /dev/null +++ b/Dockerfile.test.py39 @@ -0,0 +1,9 @@ +FROM python:3.9-slim + +COPY tests/requirements.txt . +RUN pip install -r requirements.txt + +COPY zero ./zero +COPY tests ./tests + +CMD ["pytest", "tests", "--cov=zero", "--cov-report=term-missing", "-vv"] diff --git a/Makefile b/Makefile index a0178c2..413330e 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,18 @@ setup: ) test: - python3 -m pytest tests --cov=zero --cov-report=term-missing -vv + python3 -m pytest tests --cov=zero --cov-report=term-missing -vv --durations=10 --timeout=280 + +docker-test: + docker build -t zero-test -f Dockerfile.test.py38 . + docker run --rm zero-test + docker rmi zero-test + docker build -t zero-test -f Dockerfile.test.py39 . + docker run --rm zero-test + docker rmi zero-test + docker build -t zero-test -f Dockerfile.test.py310 . + docker run --rm zero-test + docker rmi zero-test format: isort . --profile black -l 99 diff --git a/tests/functional/single_server/client_generation_test.py b/tests/functional/single_server/client_generation_test.py index fbfbd6b..22d3fc7 100644 --- a/tests/functional/single_server/client_generation_test.py +++ b/tests/functional/single_server/client_generation_test.py @@ -41,6 +41,9 @@ def error(self, msg: str) -> str: def msgspec_struct(self, start: datetime.datetime) -> Message: return self._zero_client.call("msgspec_struct", start) + def send_bytes(self, msg: bytes) -> bytes: + return self._zero_client.call("send_bytes", msg) + def echo(self, msg: str) -> str: return self._zero_client.call("echo", msg) diff --git a/tests/functional/single_server/client_server_test.py b/tests/functional/single_server/client_server_test.py index 444c585..1dbf8de 100644 --- a/tests/functional/single_server/client_server_test.py +++ b/tests/functional/single_server/client_server_test.py @@ -1,6 +1,7 @@ import datetime import pytest +import requests import zero.error from zero import AsyncZeroClient, ZeroClient @@ -100,3 +101,30 @@ def test_msgspec_struct(): msg = zero_client.call("msgspec_struct", now, return_type=Message) assert msg.msg == "hello world" assert msg.start_time == now + + +def test_send_bytes(): + zero_client = ZeroClient(server.HOST, server.PORT) + msg = zero_client.call("send_bytes", b"hello") + assert msg == b"hello" + + +def test_send_http_request(): + with pytest.raises(requests.exceptions.ReadTimeout): + requests.get(f"http://{server.HOST}:{server.PORT}", timeout=2) + + +def test_server_works_after_multiple_http_requests(): + """Because of this issue https://github.com/Ananto30/zero/issues/41""" + try: + requests.get(f"http://{server.HOST}:{server.PORT}", timeout=2) + requests.get(f"http://{server.HOST}:{server.PORT}", timeout=2) + requests.get(f"http://{server.HOST}:{server.PORT}", timeout=2) + requests.get(f"http://{server.HOST}:{server.PORT}", timeout=2) + requests.get(f"http://{server.HOST}:{server.PORT}", timeout=2) + requests.get(f"http://{server.HOST}:{server.PORT}", timeout=2) + except requests.exceptions.ReadTimeout: + pass + zero_client = ZeroClient(server.HOST, server.PORT) + msg = zero_client.call("hello_world", "") + assert msg == "hello world" diff --git a/tests/functional/single_server/client_test.py b/tests/functional/single_server/client_test.py index 07cf0c3..8f5ce41 100644 --- a/tests/functional/single_server/client_test.py +++ b/tests/functional/single_server/client_test.py @@ -31,15 +31,26 @@ async def test_concurrent_divide(): (534, 11): 48, } + total_pass = 0 + async def divide(semaphore, req): async with semaphore: - assert await async_client.call("divide", req, timeout=500) == req_resp[req] + try: + assert ( + await async_client.call("divide", req, timeout=500) == req_resp[req] + ) + nonlocal total_pass + total_pass += 1 + except zero.error.TimeoutException: + pass - semaphore = asyncio.BoundedSemaphore(3) + semaphore = asyncio.BoundedSemaphore(4) tasks = [divide(semaphore, req) for req in req_resp] await asyncio.gather(*tasks) + assert total_pass > 2 + def test_server_error(): client = ZeroClient(server.HOST, server.PORT) diff --git a/tests/functional/single_server/server.py b/tests/functional/single_server/server.py index 7b97f2f..7d4d278 100644 --- a/tests/functional/single_server/server.py +++ b/tests/functional/single_server/server.py @@ -79,6 +79,11 @@ def msgspec_struct(start: datetime.datetime) -> Message: return Message(msg="hello world", start_time=start) +@app.register_rpc +def send_bytes(msg: bytes) -> bytes: + return msg + + def run(port): print("Starting server on port", port) app.register_rpc(echo) diff --git a/tests/requirements.txt b/tests/requirements.txt index 412e089..4aef7c4 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -4,4 +4,6 @@ pytest pytest-cov PyJWT pytest-asyncio -tornado>=6.1 \ No newline at end of file +tornado>=6.1 +requests +pytest-timeout diff --git a/tests/unit/test_server.py b/tests/unit/test_server.py index 29cf53b..fed6de4 100644 --- a/tests/unit/test_server.py +++ b/tests/unit/test_server.py @@ -3,6 +3,7 @@ from typing import Any, Tuple from unittest.mock import patch +# import pytest import zmq from zero import ZeroServer @@ -177,10 +178,7 @@ def add(msg: Tuple[int, int]) -> int: return msg[0] + msg[1] with self.assertRaises(ValueError): - - @server.register_rpc - def add(msg: Tuple[int, int]) -> int: - return msg[0] + msg[1] + server.register_rpc(add) def test_register_rpc_with_invalid_input_type(self): server = ZeroServer() @@ -216,7 +214,7 @@ class Message: @server.register_rpc def add(msg: Message) -> Message: - return Message("1") + return Message() def test_server_run(self): server = ZeroServer() @@ -239,6 +237,8 @@ def add(msg: Tuple[int, int]) -> int: server._broker.backend, # type: ignore ) + # @pytest.mark.skipif(sys.platform == "win32", reason="Does not run on windows") + # @pytest.mark.skip def test_server_run_keyboard_interrupt(self): server = ZeroServer() diff --git a/zero/client_server/worker.py b/zero/client_server/worker.py index ca559a7..6e485df 100644 --- a/zero/client_server/worker.py +++ b/zero/client_server/worker.py @@ -7,6 +7,7 @@ from zero import config from zero.codegen.codegen import CodeGen from zero.encoder.protocols import Encoder +from zero.error import SERVER_PROCESSING_ERROR from zero.zero_mq.factory import get_worker @@ -40,10 +41,13 @@ def process_message(data: bytes) -> Optional[bytes]: req_id, func_name, msg = decoded response = self.handle_msg(func_name, msg) return self._encoder.encode([req_id, response]) - except Exception as inner_exc: # pylint: disable=broad-except + except ( + Exception + ) as inner_exc: # pragma: no cover pylint: disable=broad-except logging.exception(inner_exc) - # TODO what to return - return None + return self._encoder.encode( + ["", {"__zerror__server_exception": SERVER_PROCESSING_ERROR}] + ) worker = get_worker(config.ZEROMQ_PATTERN, worker_id) try: diff --git a/zero/error.py b/zero/error.py index d04d514..74f949b 100644 --- a/zero/error.py +++ b/zero/error.py @@ -1,3 +1,8 @@ +SERVER_PROCESSING_ERROR = ( + "server cannot process message, check server logs for more details" +) + + class ZeroException(Exception): pass