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

Adding an IntoResponse trait #1416

Open
dae opened this issue Jun 20, 2023 · 1 comment
Open

Adding an IntoResponse trait #1416

dae opened this issue Jun 20, 2023 · 1 comment

Comments

@dae
Copy link

dae commented Jun 20, 2023

Feature Request

Crates

tonic-build

Motivation

When using a generated client, you can pass either a Message or a Request<Message>, as the client accepts impl IntoRequest. This makes things more ergonomic when you don't have any headers/options to attach.

The generated server code lacks a similar IntoResponse and IntoStreamingResponse trait, so all responses need to be wrapped in Response::new(). This makes things a bit more cumbersome, and inconsistent with the client side of things (I remember being a bit confused by this when I started using Tonic).

Are there any technical limitations or other reasons that would prevent adding such functionality in the next breaking update?

(Apologies if this has been discussed before. I found a brief mention on #66 (comment) but couldn't see an issue for this specifically)

Proposal

Adding a similar trait:

pub trait IntoResponse<T> {
    fn into_response(self) -> Response<T>;
}

impl<T> IntoResponse<T> for T {
    fn into_response(self) -> Response<Self> {
        Response::new(self)
    }
}

impl<T> IntoResponse<T> for Response<T> {
    fn into_response(self) -> Response<T> {
        self
    }
}

async fn some_method(...) -> Result<impl IntoResponse<SomeMessage>, tonic::Status> {
    Ok(Response::new(SomeMessage {}))
}

async fn some_method(...) -> Result<impl IntoResponse<SomeMessage>, tonic::Status> {
    Ok(SomeMessage {})
}

prost-build would presumably need to be updated to change the signature of the generated methods, and call .into_response() inside call().

One potential downside is increased compile time due to the extra type inferences, but such a cost is already paid when compiling the client.

If this is something you'd be amenable to, I can look into it further.

Alternatives

#1064 would continue to require manual wrapping, but would reduce the keystrokes required to do so.

@aviramha
Copy link

Hey, I just stumbled upon the same annoyance - would love to see this - also Into<Status> is helpful since then we can implement our own error type and provide Into implementation for it.

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

No branches or pull requests

2 participants