Skip to content

hyper-util pool: unnamable types are hard to work with #4059

@Reza-Darius

Description

@Reza-Darius

Hello! I hope this is the right place to post this sort of feedback.

Ive played around a lot with the new pool module in hyper_util and here are some thoughts from a still-noobish developer:

  1. the lack of documentation and examples makes it hard to understand how these APIs are supposed to be used

  2. i had a hard time dealing with all the returning types being unnamable

For my reverse proxy i wanted to create a simple connection pool, so my instinct was to build a type that wraps the new Cache service from hyper_util and implement Service for it

this was very challenging because of the "service that returns another service" behaviour and because Cache isnt directly namable, so my wrapper had to be generic too. This is what i came up with:

impl<Cache, Sender> Service<Request<Incoming>> for ProxyService<Cache>
where
    // the cache is a service which returns another impl service
    Cache: Service<PeerAddr, Response = Sender> + Send + 'static + Clone,
    Cache::Future: Send + 'static,
    Cache::Error: Into<BoxError>,
    // this is the sender the cache spits out, which itself is another server
    Sender: Service<Request<Incoming>, Response = Response<Body>> + Send + 'static,
    Sender::Future: Send + 'static,
    Sender::Error: Into<BoxError>,

i would have much rather written a very simple Service that takes a requests and gives a response.

Ok, maybe we can erase the type with .boxed_clone(), well that only half works because you lose the special methods like .retain() since you can only coerce them into a "regular" service types since Cache isnt namable

the workaround i came up with was an intermediate service that calls the cache on the requests behalf

    // wrapper service to avoide double service calls and because "Cache" isnt nameable
    let http1con = service_fn(move |req: Request<_>| {
        let mut cache = http1cache.clone();
        async move {
            debug!("calling connector");
            let peer_addr = req
                .extensions()
                .get()
                .cloned()
                .ok_or_else(|| anyhow!("PeerAddr extension not found, req: {req:?}"))?;

            let Ok(mut sender) = cache
                .ready() 
                .await?
                .call(peer_addr) // call pool
                .await
                .inspect_err(|e| tracing::error!(%e, "couldnt get sender"))
            else {
                return Ok::<Response<Body>, BoxError>(response(StatusCode::INTERNAL_SERVER_ERROR));
            };
            
            // send off request
            sender.ready().await?.call(req).await.or_else(|e| {
                tracing::error!(%e, "sending failed");
                Ok(response(StatusCode::INTERNAL_SERVER_ERROR))
            })
        }
    });
    http1con.boxed_clone()

It doesnt feel right but maybe this is the intended behaviour?
Regardless, i greatly appreciate the work that went into hyper so i hope this was helpful

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions