Make serve generic over the HTTP implementations

[mladedav]

Motivation

Currently, only serving with hyper with predefined configuration is supported out of the box. This runs into issues where people want to configure hyper settings (#3305, #2980, #2939). It's fairly complicated to use anything but the default hyper setup, though it is of course possible even now as shown in several low-level examples.

I see this as the natural extension of #2941. We can allow serve to not only be generic over the way connections are accepted, but also how they're handled before a Request is created for our router to handle (and how to handle the returned Response).

Solution

This adds a ConnectionBuilder trait that receives an Io and Service and connects them to an underlying logic that handles HTTP, e.g. a hyper connection.

The current implementation adds hyper-util as a public dependency which might not be desirable, in which case we may want to not expose the constructor with Builder, and have a 3rd party crate like axum-server create something like this with exposed internals. That would go a bit against the issues listed in motivation, but even with that, I think it would be nicer to expose hyper-specific settings this way as well as letting others to build custom ConnectionBuilders without having to copy the whole serve implementation from here.

I've also added two examples. One is TLS usage with otherwise the default hyper implementation and the other is for showcasing using something else than hyper, in this case Rama.

[jplatte]

That's a big diff (even excluding Cargo.lock). I'll see about finding time to review it. Also the Cargo.lock makes me once again want to split examples into their own repo...

[yanns]

This is looking very interesting. Should we move forward with this?

[mladedav]

I'd be happy to iterate on this, if you have any feedback, I can update the code or just start cleaning up the code/docs (I remember writing some, but it would probably deserve a bit more).

[jplatte]

As before, I'd really like to see the new public API extracted into a comment so we can build consensus on that separately from implementation details :)

[mladedav]

There are two new traits that users can implement:

/// Types that can handle connections accepted by a [`Listener`].
pub trait ConnectionBuilder<Io, S>: Clone {
    /// Take an accepted connection from the [`Listener`] (for example a `TcpStream`) and handle
    /// requests on it using the provided service (usually a [`Router`](crate::Router)).
    fn build_connection(&mut self, io: Io, service: S) -> impl Connection;

    /// Signal to all ongoing connections that the server is shutting down.
    fn graceful_shutdown(&mut self);
}

/// A connection returned by [`ConnectionBuilder`].
///
/// This type must be driven by calling [`Connection::poll_connection`].
///
/// Note that each [`Connection`] may handle multiple requests.
pub trait Connection: Send {
    /// Poll the connection.
    fn poll_connection(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<Result<(), Box<dyn StdError + Send + Sync>>>;
}

all of these methods are expected to be called by axum directly, not by users (that's something that I should add to the docs explicitly).

There is a new public struct axum::serve::Hyper which implements ConnectionBuilder similarly to what we did until now in the connection loop. It returns HyperConnection which implements Connection but users should not interact with that type directly in any way (it is public, I think it doesn't have to be).

Hyper can be built either with Hyper::default which uses the same configuration as we're using now. Otherwise, users can create it with Hyper::new(builder: hyper_util::server::conn::auto::Builder<TokioExecutor>) where they've already set all settings they want on the Builder.

I know we historically didn't want to have hyper_util in public API so we might not want to provide this in axum itself and move it elsewhere, be it axum-extra or a 3rd party crate.

There's a new axum::serve::serve_with_connection_builder method that works just like axum::serve::serve except that it takes an additional argument of impl ConnectionBuilder which handles the connections. axum::serve::serve has the same API as until now and should behave the same way, it just internally uses serve_with_connection_builder with the default hyper-specific implementation of ConnectionBuilder.

The idea being people who do not care about this can keep using axum::serve::serve as they did until now and nothing changes. If they want to tweak hyper configuration, they can use custom axum::serve::Hyper along with serve_with_connection_builder. If they want to do something more advanced (TLS/not using hyper at all), they have to implement ConnectionBuilder.

---

Technically, Serve (as in the return value of axum::serve::serve) is changed by having an additional type parameter. The same applies to WithGracefulShutdown. I don't think most people will notice, but it is a change to the public API.

[jplatte]

I'm guessing it's serve_with_connection_builder rather than serve(...).with_connection_builder() because serve continues to be feature-gated on tokio? I think it should be possible to make serve(...) compile but not .awaitable w/o the connection builder if that feature is disabled, to support the more composable form.

[mladedav]

I didn't think of composability but yeah, I think we can do that. Just to make sure, you mean serve(...) would produce a Serve struct that would use hyper connections and then calling with_connection_builder would just swap that out, right? So this

serve(listener, router).await;

and this

serve(listener, router).with_connection_builder(conn).await;

would work?

In any case, the serve implementation still uses internally tokio::spawn, after an Io is obtained from a Listener, a task is spawned which hands the Io to the ConnectionBuilder and then the resulting Connection is handled. If we wanted to support other runtimes, I'd say we should add another method like with_runtime. From the top of my head, I think we depend just on being able to spawn tasks. So everything to do with serve still needs the tokio feature regardless of the ConnectionBuilder used.

[mladedav]

I've redone it as serve(listener, router).with_connection_builder(conn).await;.

Just note that with_graceful_shutdown does not return Serve so currently, one can do

serve(listener, router).with_connection_builder(conn).with_graceful_shtudown().await;

but not

serve(listener, router).with_graceful_shutdown().with_connection_builder(conn).await;

We could add with_connection_builder to WithGracefulShutdown too but we're duplicating a bit more stuff, which is not great but so far possibly still managable. This was raised before so I'm just flagging it as an alternative.

Another alternative would be to add a different WithConnectionBuilder type so users cannot call with_connection_builder and we force the order of Serve (-> WithConnectionBuilder) -> WithGracefulShutdown where each transition can be called just once. Currently, users can chain with_connection_builder as many times as they want.

Or we can just keep it as is but it just feels a little weird to not be consistent with how the two functions are handled (one changing just a genering on Serve and the other returning a different type).

Another open question still is what to do with the public reexports of hyper/hyper-util types. We can have the base implementation without the option to tweak it in axum and have another with better control in axum-extra.