Skip to content
New issue

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

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

Already on GitHub? # 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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants