feat(client): remove lifetime requirement for service request

[joelwurtz]

Main purpose of this is, i think this will remove a lot of confusion when creating middleware.

Actually middleware system take a &mut request so someone writing a middleware may think that after calling the next middleware we could still read / manipulate the request.

However when looking at the implementation for h2 / h3 it will take the request (due to the core::mem::take and replace it with default values) If user is not aware of the implementation he may be confused (that was my case when trying this lib for the first time)

This change enforce that the request is taken by the next middleware and cannot be read or manipulate after it, unless cloned (like its done for the redirect middleware) and so i believe this will be more understandable for people writing middleware

[fakeshadow]

Side effect based service call is a design choice on high level of the framework. This is consistent between xitca-client and xitca-web. In plain terms it enables following pattern where most other popular frameworks (in Rust) not allowing:

match service.call(req).await {
    Ok(_) => ..
    Err(_) => service2.call(req).await
}

With traditional function style service call (like most existing tower usage) the request must be cloned beforehand or stuffed into error as value to be able to do any meaningful work on error path.

Therefore its a trade off between enabling side effect or not but currently we decide it worth to keep it as is and make improvements on the short side of it:

1. http dispatcher takes ownership of request. This is a limitation introduced by our library dependencies' API style choice and it's not out of necessity.(This also means even with function style API you still lose all the information owned by request type unless you actively clone it beforehand. Which means every layer of middleware will lose request context on the error path unless with linear scaled memory copying) In the long term we will introduce our own http dispatcher work more friendly with zero copy parsing and side effect based services.
2. middleware introducing accidental side effect. This is a limitation of the pattern itself and some extra work could be put into it to help user identify and/or prevent issue related to it.

[joelwurtz]

or stuffed into error as value to be able to do any meaningful work on error path.

This was roughly the plan if this is accepted, something like :

match service.call(req).await {
    Ok(_) => ..
    Err(Error::RetryableErr(req, ..) => service2.call(req).await
    Err(_) => ..
}

In the long term we will introduce our own http dispatcher work more friendly with zero copy parsing and side effect based services.

If there is a plan like this, then yes this PR could be closed

[joelwurtz]

There may be a third way, most reasons that dispatcher need to have a Request<B> is to be able to to transform this into (Request<()>, B) by using into_parts

Maybe the into parts can be done before middleware processing and then the service request will look like :

pub struct ServiceRequest<'r, 'b, 'c> {
    pub req: &'r mut Request<()>,
    pub body: &'b mut BoxBody,
    pub client: &'c Client,
    pub timeout: Duration,
}

Then we could delay taking the request latter in the implementation or we even could clone it since most data of Request are either Copy or based on Bytes which is relatively cheap to clone

[joelwurtz]

My main concern with this change is to be able to end user to know if the request has been used after calling the next middleware.

Another proposition would be to propose an enum to the end user like :

pub enum ServiceRequest<'c> {
   NotUsed(ServiceRequestInner<'c>),
   Used
}

pub struct ServiceRequestInner<'c> {
    pub req: Request<BoxBody>,
    pub client: &'c Client,
    pub timeout: Duration,
}

then a middleware would be like

impl<'r, 'c> Service<&'r mut ServiceRequest<'c>> for MyMiddleware {
    type Response = Response;
    type Error = Error;

    async fn call(&self, req: &'r mut ServiceRequest<'c>) -> Result<Self::Response, Self::Error> {
         
    }
}

Basically the request lifetime would be transfered to the ServiceRequest struct which would still allow

match service.call(req).await {
    Ok(_) => ..
    Err(_) => service2.call(req).await
}

But end user would have the possibility to know if the request has been used or not (allowing him to retry safely if needed)