hyper icon indicating copy to clipboard operation
hyper copied to clipboard

Provide convenience around serving `ServiceFn`

Open rylev opened this issue 5 years ago • 7 comments

When making a service to pass to server I've never used make_service_fn in any other way than:

let server = Server::bind(&addr).serve(make_service_fn(|_| async move {
        Ok::<_, Infallible>(service_fn(handler))
}));

It would be nice to be able to write:

let server = Server::bind(&addr).serve(service_fn(handler));

or at least:

let server = Server::bind(&addr).serve(serve_service_fn(service_fn(handler)));

Thoughts?

rylev avatar Mar 18 '20 13:03 rylev

I get what you mean, I even had filed something similar a while ago and never gotten back to it: https://github.com/tower-rs/tower/issues/262

seanmonstar avatar Mar 18 '20 23:03 seanmonstar

Tower recently got tower::make::Shared which changes this

let server = Server::bind(&addr).serve(make_service_fn(|_| async move {
        Ok::<_, Infallible>(service_fn(handler))
}));

Into this

let server = Server::bind(&addr).serve(Shared::new(service_fn(handler));

There is also an example in the hyper docs here https://docs.rs/hyper/0.14.7/hyper/server/index.html#example

@rylev Do you think this is sufficient to close this?

davidpdrsn avatar May 02 '21 12:05 davidpdrsn

I would have preferred if Hyper had a solution that didn't require taking another dependency. The example is still not what I would call easy and intuitive to understand.

rylev avatar May 05 '21 08:05 rylev

Thats true. Having to pull in tower is unfortunate. I'm working on a proper fix.

davidpdrsn avatar May 05 '21 16:05 davidpdrsn

The new serve_service looks good :+1: (For now I'm just using Shared::new directly.)

What do you think about updating the "passing data" example or adding an extra example?

Original:

//!     let make_service = make_service_fn(move |conn: &AddrStream| {
//!         // We have to clone the context to share it with each invocation of
//!         // `make_service`. If your data doesn't implement `Clone` consider using
//!         // an `std::sync::Arc`.
//!         let context = context.clone();
//!
//!         // You can grab the address of the incoming connection like so.
//!         let addr = conn.remote_addr();
//!
//!         // Create a `Service` for responding to the request.
//!         let service = service_fn(move |req| {
//!             handle(context.clone(), addr, req)
//!         });
//!
//!         // Return the service to hyper.
//!         async move { Ok::<_, Infallible>(service) }
//!     });
//!
//!     // Run the server like above...
//!     let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
//!
//!     let server = Server::bind(&addr).serve(make_service);

I think it can be simplified to this (assuming conn/addr isn't needed):

//!     let service = service_fn(move |req| {
//!         handle(context.clone(), req)
//!     });
//!
//!     // Run the server like above...
//!     let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
//!
//!     let server = Server::bind(&addr).serve_service(service);

dcecile avatar Nov 05 '21 04:11 dcecile

We ended up not merging that PR https://github.com/hyperium/hyper/pull/2541

davidpdrsn avatar Nov 05 '21 06:11 davidpdrsn

😅 Oh I see! (Checking out #2321 linked in the PR...)

dcecile avatar Nov 05 '21 16:11 dcecile