axum icon indicating copy to clipboard operation
axum copied to clipboard

Remove request body type param

Open davidpdrsn opened this issue 3 years ago • 2 comments

Fixes https://github.com/tokio-rs/axum/issues/1110

This removes the request body type param from everything (Router, FromRequest, RequestParts, etc...). The goal being to improve usability and solve issues like https://github.com/tokio-rs/axum/issues/1110.

The general approach is to implement Service like this:

impl<B> Service<Request<B>> for Router
where
    B: http_body::Body,
{
    fn call(&mut self, req: Request<B>) -> Self::Future {
        let req = req.map(|body| hyper::Body::wrap_body(body));
        // ...
    }
}

This allows us to accept any impl http_body::Body on the outside but convert that to a hyper::Body on the inside.

We need to accept any impl http_body::Body instead of only hyper::Body to support middleware that change the request body (such as RequestBodyLimit).

I also considered using BoxBody rather than hyper::Body but that means users cannot extract Request<hyper::Body> which I think would confuse many users.

Note that hyper::Body:wrap_body isn't shipped yet. I have made a PR to hyper that adds that. If hyper decides not to merge that then our only option is to use BoxBody. We need that to construct a hyper::Body from a generic impl http_body::Body.

This is also a win in another elsewhere because it removes a type param from FromRequest and RequestParts. In https://github.com/tokio-rs/axum/pull/1121 and a WIP fix for https://github.com/tokio-rs/axum/issues/1142 I'm adding two more type params to those types. So by removing the body we'll only have two type params instead of three.

TODO

  • [ ] Update docs
  • [ ] Update changelog, including guide on how to upgrade

davidpdrsn avatar Jul 11 '22 11:07 davidpdrsn

I wrote about why hyper::Body::wrap_body is necessary here.

davidpdrsn avatar Jul 11 '22 13:07 davidpdrsn

The main downside is that it locks us into one body type for extractors. I think a lot depends on the hyper/hyper-util split for their 1.0 and where stuff ends up. We cannot publicly depend on the body from hyper-util and hyper probably won't have a single body type.

davidpdrsn avatar Jul 19 '22 17:07 davidpdrsn

I'm gonna close this. All this is gonna have to change anyway when we get closer to hyper 1.0 so I'd rather deal with it then. See https://github.com/tokio-rs/axum/issues/1110#issuecomment-1222265042.

This PR is also too large to review so if/when we do this we have to do it in smaller chunks.

davidpdrsn avatar Aug 22 '22 12:08 davidpdrsn