Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RPC name conflicts with generated code #1261

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lucia3e8
Copy link

@lucia3e8 lucia3e8 commented Feb 7, 2023

Motivation

Issues: #747 and #1253

Still a draft - need to add tests

TLDR: Tonic adds predefined methods to codegenned structs, like connect. If that name conflicts with an RPC name, the resulting code fails to compile.

Solution

This is a problem for other gRPC codegenners too. The C++ one just prepends the problematic method names with a _.
I took the same approach here.

For example, given the RPC definition

syntax = "proto3";
service MyService {
  rpc Connect(ConnectRequest) returns (ConnectResponse);
}

Will generate the following code now

    impl MyServiceClient<tonic::transport::Channel> {
        /// VVVVVVV This is the tonic-provided method
        /// Attempt to create a new client by connecting to a given endpoint.
        pub async fn connect<D>(dst: D) -> Result<Self, tonic::transport::Error>
        where
            D: std::convert::TryInto<tonic::transport::Endpoint>,
            D::Error: Into<StdError>,
        {
            let conn = tonic::transport::Endpoint::new(dst)?.connect().await?;
            Ok(Self::new(conn))
        }
    }
    impl<T> MyServiceClient<T>
    where
        T: tonic::client::GrpcService<tonic::body::BoxBody>,
        T::Error: Into<StdError>,
        T::ResponseBody: Body<Data = Bytes> + Send + 'static,
        <T::ResponseBody as Body>::Error: Into<StdError> + Send,
    {
        [...]
        /// VVVVVVV This is the method generated from the .proto file
        pub async fn _connect(
            &mut self,
            request: impl tonic::IntoRequest<super::ConnectRequest>,
        ) -> Result<tonic::Response<super::ConnectResponse>, tonic::Status> {
            [...]
        }

czaloj added 2 commits April 13, 2023 11:34
* Add prost-reflect-build
* Optional reflect
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants