Add bounds to constructors

[Diegovsky]

Feature Request

Motivation

Currently on axum, most generic types have unbounded constructors (e.g: Html, HandleErrorLayer) but their relevant trait implementations (IntoResponse for Html, Layer for HandleErrorLayer) have bounds, leaving confusing error messages when using them with non conforming types (such as using a Cow<'a, str> with Html).

Proposal

Add redundant bounds to constructors, also making the transition easier when rust's Implied Trait Bounds land.

Example of Status Quo

Say I'm writing an error handling middleware. According to the docs, this could work:

pub async fn error_function(err: BoxError) -> () {
    todo!()
}

#[tokio::main]
async main() {
    let app = Router::new()
        .layer(
        tower::ServiceBuilder::new()
            .layer(HandleErrorLayer::new(error_function)),
    );


    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    axum::Server::bind(&addr)
        .serve(app.into_make_service())
        .await
        .unwrap();

}

However I get this error on line 8 which should appear on HandleErrorLayer::new instead:

[davidpdrsn]

Let me start by saying that I feel your pain. HandleErrorLayer can lead to particularly hairy type errors if you don't fully understand how it works.

Unfortunately what you're suggesting isn't possible. One might try to add the necessary bounds to HandleErrorLayer::new:

impl<F, T> HandleErrorLayer<F, T> {
    pub fn new<S, B>(f: F) -> Self
    where
        Self: Layer<S>,
        <Self as Layer<S>>::Service: Service<Request<B>>,
    {
        Self {
            f,
            _extractor: PhantomData,
        }
    }
}

However using it in practice:

#[tokio::main]
async fn main() {
    let app = Router::new().layer(
        ServiceBuilder::new()
            .layer(HandleErrorLayer::new(handle_error))
            // removing this line gives you another error that forces you to add this line
            // so it doesn't change the outcome
            .timeout(Duration::from_secs(30))
    );

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    axum::Server::bind(&addr)
        .serve(app.into_make_service())
        .await
        .unwrap();
}

async fn handle_error(_: BoxError) {}

Gives an inference error:

 --> examples/hello-world/src/main.rs:7:46
  |
7 | use axum::{error_handling::HandleErrorLayer, response::Html, routing::get, BoxError, Router};
  |                                              ^^^^^^^^^^^^^^  ^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

error[E0282]: type annotations needed
  --> examples/hello-world/src/main.rs:16:20
   |
16 |             .layer(HandleErrorLayer::new(handle_error))
   |                    ^^^^^^^^^^^^^^^^^^^^^ cannot infer type for type parameter `S` declared on the associated function `new`

You cannot just copy the bounds from the Service impl either since HandleError implements Service for many types of functions, not just one.

---

According to the docs, this could work:

You've missed a key part from the docs: .timeout(Duration::from_secs(30)). That layer changes the error type which makes HandleErrorLayer work. The rest of the error handling model docs should hopefully make it clear why that is.

---

Html<Cow<'static, str>> does implement IntoResponse so not sure what your problem was there. You cannot return borrowed values from handler functions so maybe that was it. You must return fully owned data.

Now we could change the definition of Html to

pub struct Html<T>(pub T) where T: Into<Full<Bytes>>;

Which if you tried to return something silly like Html<bool> would give you this error:

error[E0277]: the trait bound `axum::body::Full<axum::body::Bytes>: From<bool>` is not satisfied
  --> examples/hello-world/src/main.rs:24:34
   |
24 |   async fn handler() -> Html<bool> {
   |  __________________________________^
25 | |     todo!()
26 | | }
   | |_^ the trait `From<bool>` is not implemented for `axum::body::Full<axum::body::Bytes>`
   |
   = help: the following implementations were found:
             <axum::body::Full<D> as From<&'static [u8]>>
             <axum::body::Full<D> as From<&'static str>>
             <axum::body::Full<D> as From<Cow<'static, B>>>
             <axum::body::Full<D> as From<String>>
           and 2 others
   = note: required because of the requirements on the impl of `Into<axum::body::Full<axum::body::Bytes>>` for `bool`
note: required by a bound in `Html`

I'm not sure whether that error is more helpful but axum::body::Full<axum::body::Bytes> is probably going to be just as foreign to people.

However this cannot be done consistently. Consider Extension which implements both Layer and IntoResponseParts. There isn't one set of bounds we can put on Extension that covers both. Same goes for Json which implements both FromRequest and IntoResponse.

Because of this, in Rust, its generally recommended to only add trait bounds where they're needed as this leads for more flexibility. For example HashMap from std doesn't put any bounds on the key and value type, although they'll must likely have to be Hash + Eq to actually use the map.

Really I think the compiler should give better error messages for these cases. Until I've learned how to contribute the compiler and worked on this, we have tools like #[axum_macros::debug_handler] which is mentioned here. While it doesn't help with HandleErrorLayer it does help with many cases like this.

[jplatte]

I think it may actually be worthwhile to specify the minimal generics on the type itself, though Html is a bad example.

For Extension this would be T: Send + Sync + 'static I think, which should give good error messages when violated. It's unfortunate we can't do this for Json, but there's others where we can clearly do it like Query<T: DeserializeOwned>.

Unfortunately, all of these things are breaking changes so they're unlikely to happen anytime soon. I still see this as a reasonable thing down the road though because it doesn't constrain in any way: Removing or relaxing the bounds again later is fully backwards-compatible.

[Diegovsky]

Thank you very much for giving such a detailed and easy to understand explanation. Indeed, I missed the docs and the compiler message didn't help a lot either. It just seemed weird to me the compiler was complaining about the function while the problem was something else.

Also, I noticed this pattern of not adding bounds to structs themselves and never quite understand why, so I'm also grateful for your thorough explanation.

I had no idea timeout was actually needed as it's signature seemed to be the same as every other method. I guess it's the way cargo doc presents return types and bounds that made me miss it (specially with generics).

I will try the debug handler as suggested :)

Edit: wording

[davidpdrsn]

I think it may actually be worthwhile to specify the minimal generics on the type itself, though Html is a bad example.

Yeah it might be worth while to add some minimal bounds where possible. It probably wont improve the error messages related to FromRequest much but worth investigating.

Unfortunately, all of these things are breaking changes so they're unlikely to happen anytime soon

Yep this isn't something can address without breakage and I'd like to wait with that since we just shipped 0.5 😊

[davidpdrsn]

I think I'll close this. People are welcome to submit PRs to improve this but I don't think its high priority.