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

Custom metadata on error responses #343

Closed
dvshur opened this issue May 5, 2020 · 9 comments
Closed

Custom metadata on error responses #343

dvshur opened this issue May 5, 2020 · 9 comments
Labels
A-tonic C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@dvshur
Copy link
Contributor

dvshur commented May 5, 2020

Feature Request

Motivation

Currently it seems only possible to return some custom info with a non-zero gRPC status in one predefined header:
const GRPC_STATUS_DETAILS_HEADER: &str = "grpc-status-details-bin";

#[derive(Clone)]
pub struct Status {
    /// The gRPC status code, found in the `grpc-status` header.
    code: Code,
    /// A relevant error message, found in the `grpc-message` header.
    message: String,
    /// Binary opaque details, found in the `grpc-status-details-bin` header.
    details: Bytes,
}

What if I want to use a different header name? What if I want to return more than one header/trailer on error response? gRPC spec does support that possibility. Libraries in other languages that I've worked with (Go, Scala) also don't have that restriction. We actually use a different header name in production in our company, so it's a quite real use case.

Proposal

Min: allow details from Status to go to a header with a custom name.
Max: allow any number of custom metadata key/values, ASCII or binary (via *-bin headers), on non-zero gRPC statuses.

@LucioFranco LucioFranco added A-tonic C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue. labels May 7, 2020
@LucioFranco
Copy link
Member

I'd like to expose a similar ability to get access to a metadata map like https://docs.rs/tonic/0.2.0/tonic/struct.Response.html#method.metadata

This way you can add your own custom metadata values.

@avantgardnerio
Copy link

Sorry to comment on an old issue, but it appears that isn't possible because the only way to create metadata is through Response:

metadata: MetadataMap::new(),

and that calls a Metadata constructor with 0 capacity:

MetadataMap::with_capacity(0)

Resulting in:

thread 'tokio-runtime-worker' panicked at 'index out of bounds: the len is 0 but the index is 0'

At runtime. Is there a way at present to achieve this? A quick googling didn't reveal any currently open issues around this behavior. I can open one if that's the correct next step.

@LucioFranco
Copy link
Member

@avantgardnerio if you're still seeing this, lets open another issue and really what would be helpful would be to recreate this in a test in map.rs (there are already a bunch of tests). Thanks!

@avantgardnerio
Copy link

I am still seeing it, and I will PR a test, thank you for your prompt response!

@avantgardnerio
Copy link

I take that back. I am seeing it in my project, but I can't get it to reproduce with a unit test in tonic. I'll try to look into it more when I free up. I think just disregard for now unless anyone else is seeing the same thing. Thanks @LucioFranco !

@markocoric
Copy link

markocoric commented Jan 10, 2023

one note, just to look into ... if you use name like "X-Device" instead of "x-device" error will appear.

@shravanshetty1
Copy link

one note, just to look into ... if you use name like "X-Device" instead of "x-device" error will appear.

Yea similar issue here, had to set metadata "authorization" instead of "Authorization" on the client - it became "Authorization" on the server though..

@kirito41dd
Copy link

is it possible set metadata in middleware? now only when the request is successful, resp can read the metadata,when error,it appears in message:

rpc err: status: Internal, message: "an internal server error occurred No such file or directory (os error 2)", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "date": "Wed, 03 Apr 2024 02:13:11 GMT", "content-length": "0", "logid": "666"} }
impl<T, RB> Future for LogidFuture<T>
where
    T: Future<Output = Result<http::Response<RB>, Infallible>>,
{
    type Output = T::Output;

    fn poll(
        mut self: std::pin::Pin<&mut Self>,
        cx: &mut std::task::Context<'_>,
    ) -> std::task::Poll<Self::Output> {
        match self.inner.as_mut().poll(cx) {
            std::task::Poll::Ready(mut resp) => {
                resp = resp.map(|mut v| {
                    v.headers_mut().insert(
                        HEADER_LOGID,
                        HeaderValue::from_str(&self.logid).expect("logid"),
                    );
                    v
                });
                std::task::Poll::Ready(resp)
            }
            std::task::Poll::Pending => std::task::Poll::Pending,
        }
    }
}

@BatmanAoD
Copy link

one note, just to look into ... if you use name like "X-Device" instead of "x-device" error will appear

I found this issue because of this. Honeycomb documents its auth header as X-Honeycomb-Team, so that's what I tried to use... and the error message is not helpful at all. Either insert and append should handle capital letters, or, at a minimum, the panic message should say something to the effect that capital letters aren't allowed.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-tonic C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

No branches or pull requests

7 participants