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

Using tower layers #2132

Closed
mishriky opened this issue Feb 13, 2020 · 3 comments
Closed

Using tower layers #2132

mishriky opened this issue Feb 13, 2020 · 3 comments

Comments

@mishriky
Copy link

Hi, Rust and Hyper noob here so excuse my ignorance.
I am trying to build on the Tower server example (https://github.com/hyperium/hyper/blob/master/examples/tower_server.rs) by adding a timeout layer but it does not seem to work as expected.

I'm using a ServiceBuilder to construct the service:

let svc = ServiceBuilder::new()
        .timeout(Duration::from_secs(1))
        .service(MakeSvc);

and then passing that to serve.

The stack seems to be constructed correctly:

Stack: Timeout {
    inner: MakeSvc,
    timeout: 1s,
}

and after some debugging I can see that the call function in Timeout does get called but it looks like the response is returned from MakeSvc immediately before call gets called in Svc, effectively bypassing the timeout check.

Any ideas on what I could be missing?

@mishriky
Copy link
Author

Adding the full example to reproduce:

use std::future::Future;
use std::net::SocketAddr;
use std::pin::Pin;
use std::task::{Context, Poll};
use std::time::Duration;

use hyper::{Body, http, Request, Response, Server};
use hyper::service::Service;
use tokio::time::delay_for;
use tower::ServiceBuilder;

#[derive(Debug)]
pub struct Svc;

impl Service<Request<Body>> for Svc {
    type Response = Response<Body>;
    type Error = http::Error;
    type Future = Pin<Box<dyn Future<Output=Result<Self::Response, Self::Error>> + Send>>;

    fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { Poll::Ready(Ok(())) }

    fn call(&mut self, _: Request<Body>) -> Self::Future {
        let rsp = Response::builder();
        let body = Body::from(Vec::from(&b"ack!"[..]));
        let rsp = rsp.status(200).body(body).unwrap();

        let fut = async {
            delay_for(Duration::from_secs(5)).await;
            Ok(rsp)
        };
        Box::pin(fut)
    }
}

#[derive(Debug)]
pub struct MakeSvc;

impl<T> Service<T> for MakeSvc {
    type Response = Svc;
    type Error = std::io::Error;
    type Future = Pin<Box<dyn Future<Output=Result<Self::Response, Self::Error>> + Send>>;

    fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { Ok(()).into() }

    fn call(&mut self, _: T) -> Self::Future {
        let fut = async { Ok(Svc) };
        Box::pin(fut)
    }
}

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    env_logger::init();

    let listen_addr =
        format!("{}", "127.0.0.1:8088")
            .parse::<SocketAddr>()
            .unwrap_or_else(|e| {
                panic!("An error occurred while reading listen address {:?}", e);
            });

    let svc = ServiceBuilder::new()
        .timeout(Duration::from_secs(2))
        .service(MakeSvc);

    let server = Server::bind(&listen_addr).serve(svc);
    println!("Listening on http://{}", listen_addr);
    server.await?;
    Ok(())
}

If I move delay_for(Duration::from_secs(5)).await; to call in the MakeSvc impl, the timeout works as expected but that won't actually account for the time spent processing the request

@seanmonstar
Copy link
Member

There's two steps involved here, and both make use of Service:

  1. The MakeSvc is a Service that creates Svcs for each connection.
  2. The Svc is a Service to handle requests on a single connection.

By adding the timeout to the MakeSvc part, you're putting a limit on how long your system should set up resources to handle the connection and return a Svc.

If you want to instead put the timeout on handling requests/responses, you could use a ServiceBuilder inside MakeSvc:

impl<T> Service<T> for MakeSvc {
    type Response = Svc;
    type Error = std::io::Error;
    type Future = Pin<Box<dyn Future<Output=Result<Self::Response, Self::Error>> + Send>>;

    fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { Ok(()).into() }

    fn call(&mut self, _: T) -> Self::Future {
        Box::pin(async {
            // A service to handle the requests of the connection `T`
            let svc = ServiceBuilder::new()
                .timeout(Duration::from_secs(2))
                .service(Svc);
            Ok(svc)
        })
        Box::pin(fut)
    }
}

Note that hyper doesn't have specific knowledge of the timeout error that will occur, so you may wish to add a layer that converts errors into http::Responses.


I'm going to close this as it's more a usage question than a bug in hyper.

@mishriky
Copy link
Author

Thank you, that makes sense. and yes, definitely not a bug

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