tower-rs / tower-http

HTTP specific Tower utilities.
702 stars 165 forks source link

Incompatible with tower 0.4.XX #367

Closed stan-irl closed 1 year ago

stan-irl commented 1 year ago

Bug Report

Version

tower: 0.4.14 tower_http: 0.4.0

Platform

Crates

Description

I just tried to use SetRequestHeaderLayer to override my user agent but it looks like its incompatible with tower 0.4.xx. I downgraded tower to 0.3.xx and a lot of these errors disappeared:

\

error[E0599]: the method `service` exists for struct `ServiceBuilder<Stack<SetRequestHeaderLayer<Result<..., ...>>, ...>>`, but its trait bounds were not satisfied
   --> src/components/grpc/src/lib.rs:59:14
    |
52  |           let inner = ServiceBuilder::new()
    |  _____________________-
53  | |             // 
54  | |             //  
55  | |             .layer(SetRequestHeaderLayer::overriding(
...   |
58  | |             ))
59  | |             .service(inner_transport);
    | |_____________-^^^^^^^
    |
   ::: ~/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-http-0.4.0/src/set_header/request.rs:97:1
    |
97  |   pub struct SetRequestHeaderLayer<M> {
    |   ----------------------------------- doesn't satisfy `_: Layer<_>`
    |
   ::: ~/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/result.rs:503:1
    |
503 |   pub enum Result<T, E> {
    |   --------------------- doesn't satisfy `Result<HeaderValue, InvalidHeaderValue>: Clone`
    |
    = note: the full type name has been written to '<REDACTED>/target/debug/deps/grpc-24c37669d399a2de.long-type-4426545394699333500.txt'
    = note: the following trait bounds were not satisfied:
            `SetRequestHeaderLayer<Result<HeaderValue, InvalidHeaderValue>>: Layer<_>`
            `Result<HeaderValue, InvalidHeaderValue>: Clone`

Workaround

I just whipped up some quick and dirty middleware to add the header:

//! This module contains middleware which injects the x-user-agent to outgoing requests
use http::header::HeaderValue;
use http::Request;
use std::fmt::Debug;
use std::task::{Context, Poll};
use tonic::body::BoxBody;
use tower::{Layer, Service};

// allowing this because it seems to be a standard naming convention for tower middleware
#[allow(clippy::module_name_repetitions)]
/// Middleware layer that adds user agent header to outgoing requests
#[derive(Clone, Debug)]
pub struct UserAgentLayer {
    /// TODO - pass in device,os,version etc here
    /// to formulate the string
    user_agent: String,
}

impl UserAgentLayer {
    /// Create a new ``UserAgentLayer`` instance from the given user agent string
    pub fn from_str(user_agent: &str) -> Self {
        Self {
            user_agent: user_agent.to_owned(),
        }
    }

    // Create a new ``UserAgentLayer`` instance from the given component pieces
    // pub fn from_components(os: &str, device: &str, version: &str) -> Self {
    //     Self { user_agent }
    // }
}

impl<S: Clone> Layer<S> for UserAgentLayer {
    type Service = UserAgentService<S>;

    fn layer(&self, inner: S) -> Self::Service {
        UserAgentService {
            inner,
            user_agent: self.user_agent.clone(),
        }
    }
}

// allowing this because it seems to be a standard naming convention for tower middleware
#[allow(clippy::module_name_repetitions)]
/// Middleware service that adds x-user-agent header to outgoing requests
#[derive(Clone, Debug)]
pub struct UserAgentService<S: Clone> {
    /// the service to wrap
    inner: S,
    /// the user agent string
    user_agent: String,
}

impl<S> Service<Request<BoxBody>> for UserAgentService<S>
where
    // Needs to be static
    // https://github.com/tower-rs/tower/pull/732
    S: Service<Request<BoxBody>> + Clone + 'static,
{
    type Response = S::Response;
    type Error = S::Error;
    type Future = S::Future;

    fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        self.inner.poll_ready(cx).map_err(Into::into)
    }

    fn call(&mut self, mut request: Request<BoxBody>) -> Self::Future {
        let header_val =
            HeaderValue::from_str(&self.user_agent).unwrap_or(HeaderValue::from_static(""));
        request
            .headers_mut()
            .insert(http::header::USER_AGENT, header_val);
        self.inner.call(request)
    }
}
davidpdrsn commented 1 year ago

Would you mind making a reproduction script that we can test with? Otherwise its hard to debug.

stan-irl commented 1 year ago

tried to repro and it works fine. obviously a problem on my end. sorry for the noise

stan-irl commented 1 year ago

ah its because I had HeaderValue::from_str() in the make field which returns a Result<HeaderValue, E>, not a HeaderValue