search

vmagamedov / grpclib

Pure-Python gRPC implementation for asyncio
http://grpclib.readthedocs.io
BSD 3-Clause "New" or "Revised" License
934 stars 94 forks source link

Connection reset talking to a server written with tonic (rust) when using IPv6 #197

Closed llucax closed 1 month ago

llucax commented 1 month ago

Hello, and thanks for the great library. We are currently trying out it to see if we replace our use of grpcio and are having an issue when using it to connect to a server written with tonic and using IPv6 (with IPv4 it works as expected). grpcio works fine, and it's what we've been using for a while.

Since this is an interaction issue, I'm not certain if grpclib is to blame, I guess it could be tonic or h2 for example, but from all the involved projects this one seems to be the less mature, so I will take my chances here first, but please let me know if the issue is in another project to I can report it where appropriate.

Reproducing

[!TIP] The same instructions with all the code ready to be tested is available, as well as the network captures, at: https://github.com/llucax/grpclib-bug

Here is the minimum way to reproduce the issue I've found:

mkdir /tmp/bug
cd /tmp/bug
# create helloworld.proto
sudo apt install rustup
cargo new helloworld-tonic
cd helloworld-tonic
# create build.rs
# create src/server.rs
cargo build --bin helloworld-server # possibly leave it running in the background &
cd ..
mkdir client
cd client
python -m venv venv
. venv/bin/activate
python -m pip install "betterproto[compiler]==2.0.0b6" grpcio-tools grpcio protobuf
mkdir better
python -m grpc_tools.protoc -I .. --python_betterproto_out=better helloworld.proto
# create grpclib_client.py

Run the server

cargo run --bin helloworld-server

Run the client

$ python grpclib_client.py 
Traceback (most recent call last):
  File "/home/luca/tmp/grpclib-bug/venv/lib/python3.11/site-packages/grpclib/client.py", line 368, in recv_initial_metadata
    headers = await self._stream.recv_headers()
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/luca/tmp/grpclib-bug/venv/lib/python3.11/site-packages/grpclib/protocol.py", line 342, in recv_headers
    await self.headers_received.wait()
  File "/usr/lib/python3.11/asyncio/locks.py", line 213, in wait
    await fut
asyncio.exceptions.CancelledError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/luca/tmp/grpclib-bug/client/grpclib_client.py", line 16, in <module>
    asyncio.run(main("::1" if len(sys.argv) < 2 else sys.argv[1]))
  File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/luca/tmp/grpclib-bug/client/grpclib_client.py", line 12, in main
    response = await service.say_hello(better.helloworld.HelloRequest(name="world"))
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/luca/tmp/grpclib-bug/client/better/helloworld/__init__.py", line 43, in say_hello
    return await self._unary_unary(
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/luca/tmp/grpclib-bug/venv/lib/python3.11/site-packages/betterproto/grpc/grpclib_client.py", line 85, in _unary_unary
    response = await stream.recv_message()
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/luca/tmp/grpclib-bug/venv/lib/python3.11/site-packages/grpclib/client.py", line 425, in recv_message
    await self.recv_initial_metadata()
  File "/home/luca/tmp/grpclib-bug/venv/lib/python3.11/site-packages/grpclib/client.py", line 367, in recv_initial_metadata
    with self._wrapper:
  File "/home/luca/tmp/grpclib-bug/venv/lib/python3.11/site-packages/grpclib/utils.py", line 70, in __exit__
    raise self._error
grpclib.exceptions.StreamTerminatedError: Stream reset by remote party, error_code: 1

Using grpcio instead works

python -m pip install grpcio protobuf
python -m grpc_tools.protoc -I .. --python_out=. --grpc_python_out=. ../helloworld.proto
# create grpcio_client.py

Running it:

$ python grpcio_client.py 
WARNING: All log messages before absl::InitializeLog() is called are written to STDERR
I0000 00:00:1721400412.394474  316293 config.cc:230] gRPC experiments enabled: call_status_override_on_cancellation, event_engine_dns, event_engine_listener, http2_stats_fix, monitoring_experiment, pick_first_new, trace_record_callops, work_serializer_clears_time_cache
Greeter client received: Hello you!

Using IPv4 also works

If we make the server listen to 127.0.0.1 instead, and connect to it, both grpclib and grpcio work.

Files

helloworld.proto ```protobuf syntax = "proto3"; package helloworld; service Greeter { rpc SayHello (HelloRequest) returns (HelloReply); } message HelloRequest { string name = 1; } message HelloReply { string message = 1; } ```
helloworld-tonic/Cargo.toml ```toml [package] name = "helloworld-tonic" version = "0.1.0" edition = "2021" [[bin]] # Bin to run the HelloWorld gRPC server name = "helloworld-server" path = "src/server.rs" [dependencies] tonic = "0.12" prost = "0.13" tokio = { version = "1.0", features = ["macros", "rt-multi-thread"] } [build-dependencies] tonic-build = "0.12" ```
helloworld-tonic/build.rs ```rust fn main() -> Result<(), Box> { tonic_build::compile_protos("../helloworld.proto")?; Ok(()) } ```
helloworld-tonic/src/server.rs ```rust use tonic::{transport::Server, Request, Response, Status}; use hello_world::greeter_server::{Greeter, GreeterServer}; use hello_world::{HelloReply, HelloRequest}; pub mod hello_world { tonic::include_proto!("helloworld"); } #[derive(Debug, Default)] pub struct MyGreeter {} #[tonic::async_trait] impl Greeter for MyGreeter { async fn say_hello( &self, request: Request, ) -> Result, Status> { println!("Got a request: {:?}", request); let reply = HelloReply { message: format!("Hello {}!", request.into_inner().name), }; Ok(Response::new(reply)) } } #[tokio::main] async fn main() -> Result<(), Box> { let addr = "[::1]:50051".parse()?; //let addr = "127.0.0.1:50051".parse()?; // This works let greeter = MyGreeter::default(); Server::builder() .add_service(GreeterServer::new(greeter)) .serve(addr) .await?; Ok(()) } ```
client/grpclib_client.py ```python import asyncio import sys from grpclib.client import Channel import better.helloworld async def main(host: str) -> None: async with Channel(host=host, port=50051) as channel: service = better.helloworld.GreeterStub(channel) response = await service.say_hello(better.helloworld.HelloRequest(name="world")) print(response) asyncio.run(main("::1" if len(sys.argv) < 2 else sys.argv[1])) ```
client/grpcio_client.py ```python import asyncio import sys import grpc import helloworld_pb2 import helloworld_pb2_grpc async def main(host: str) -> None: async with grpc.aio.insecure_channel(f"{host}:50051") as channel: stub = helloworld_pb2_grpc.GreeterStub(channel) response = await stub.SayHello(helloworld_pb2.HelloRequest(name="you")) print("Greeter client received: " + response.message) asyncio.run(main("[::1]" if len(sys.argv) < 2 else sys.argv[1])) ```

Captures

Here are pcapng captures using IPv6 and IPv4 with both the grpclib and grpcio client (renamed to .log because dumb GitHub doesn't allow attaching files named .pcapng.

vmagamedov commented 1 month ago

Hello. Thanks for the detailed report.

I found the issue. grpclib sends host:port as :authority header, which results in ::1:50051, but in case of IPv6 the proper value would be [::1]:50051. At the same time asyncio can't use [::1] as a host parameter. So we need to convert ::1 into [::1] when constructing :authority header.

vmagamedov commented 1 month ago

BTW, what are the reasons to replace grpcio with grpclib?

llucax commented 1 month ago

Cool, thanks for the fix and for the explanation!

We are not happy with grpcio ill type hints and general unpythonic design. We got a couple of bugs related to this and found out betterproto and decided to give it a go, which leaded us to grpclib.

Would you not recommend to switch?

llucax commented 1 month ago

BTW, are you planning to release the fix soon?

vmagamedov commented 1 month ago

are you planning to release the fix soon?

I can issue a release candidate any time soon. And I want to fix one more issue to make a proper release.

llucax commented 1 month ago

Great, thanks!

llucax commented 1 month ago

BTW, we could verify the fix works for us. Thanks again! A release will be highly appreciated.

Also, I'm still curious about why did you asked why are we looking to swtich from grcpio to grpclib.

:innocent:

vmagamedov commented 1 month ago

Issued release candidate: https://pypi.org/project/grpclib/0.4.8rc2/

I'm still curious about why did you asked why are we looking to swtich from grcpio to grpclib

This project was created mostly because grpcio didn't had any support for asyncio at that time. So now when they implemented asyncio support I'm curious why people don't want to use it. This is kinda strange given grpcio popularity (PyPI download statistics) and that there is Google as a company behind that project.

llucax commented 1 month ago

Thanks for the rc!

Sorry to continue with the off-topic, I hope it is OK, but it is very interesting for me to learn as much as possible for considering the switch. So basically I arrived indirectly to grpclib via betterproto. Do you know why betterproto chose grpclib? Was it also for "historical reasons" (grpcio not supporting async at some point)?

My main issue with grpcio for both protobuf and grpc code generation is the lack of type hints (2.5 years old issue):

Is evident that when grpcio is backed by a big company, Google doesn't have it as a high priority.

For example methods in the generated grpc stub are not marked as async, so you can call them without await and get no error from mypy. This happened to us in actual code. We use https://github.com/nipunn1313/mypy-protobuf but it doesn't mark them as async properly either.

So my question is, is grpclib used in production anywhere and being actively developed or since grpcio supports async only small bugs like this one are being fixed and it is mainly in maintenance mode?

vmagamedov commented 1 month ago

Do you know why betterproto chose grpclib?

I don't know 🤷🏻‍♂️

So my question is, is grpclib used in production anywhere and being actively developed or since grpcio supports async only small bugs like this one are being fixed and it is mainly in maintenance mode?

It is used in production but it is not actively developed, it is kinda in maintenance mode unless there will be contributions from the community.

llucax commented 1 month ago

Thanks for all the info.

khodedawsh commented 1 month ago

This project was created mostly because grpcio didn't had any support for asyncio at that time. So now when they implemented asyncio support I'm curious why people don't want to use it. This is kinda strange given grpcio popularity (PyPI download statistics) and that there is Google as a company behind that project.

There are a few problems with grpcio, the major ones being that it doesn't provide a wheel for alpine arm64(takes a lifetime to compile on some github runner) and that it forces you to have an authorized certificate. server-side had some bugs too which I haven't dig any further. both are issued in their repo but they don't have much priority for these. Anyways I really appreciate your work and that grpclib exists.