A hyper without tokio-proto

[seanmonstar]

It would be nice to have hyper working without internally using tokio-proto. It should be possible to do this in a backwards-compatible way at first, such that the server still provides a ServerProto implementation that makes use of the internal stuff, and that can be deprecated in the future.

Motivation

• The tokio-proto crate is being de-emphasized.
• We can do some things better, specifically for HTTP
  • We can better detect if a body has a length if we control the Stream
  • We improve handling of Expect: 100-continue (Change 100-continue behavior to send when Body has been polled #838)
  • We can probably improve performance a little
• Errors that are encountered reading, parsing, and writing are kind of just lost.
  • They are given back to tokio-proto, but it's pretty tricky for a user to get those errors.
• It doesn't allow hyper to currently spawn a server that can handle both HTTP/1 and HTTP/2

API

Http::bind_connection(socket, service) -> Connection

Compared to the current bind_connection:

• The socket is T: AsyncRead + AsyncWrite.
• The service is currently a S: Service.
• The addr is not needed anymore. Anyone wanting to know the address can store it on their Service.
• The handle is not provided, since it's only purpose was to spawn a task on it. With the desire to separate tokio from being a task executor, we can plan ahead.

The returned type is a Connection<T, S>, which implements Future. Instead of taking a Handle, a user can instead receive this future and spawn it on any executor they desire.

Of course, since bind_connection already exist, this would need a new name. It could be bind_connection2, with the previous deprecated, with the intent to finish the replacement in 0.12.

Or maybe bind_connection itself isn't the best of names. The action of this method is to apply the Http protocol to a connection, aided by the Service to respond to requests.

Connection

This is returned by Http::bind_connection, and implements Future.

• Item = ()
• Error = hyper::Error

If there are never any errors when processing the connection, the future yields Ok(Async::Ready) when Http says it should close.

If there are errors encountered that would require tearing down the connection, the future returns the Err. Importantly, this allows users to very easily know if there was an error reading or writing the HTTP protocol.

Some unresolved questions:

• This could perhaps have the Item be the socket (T) instead, in case there is desire to be able to do something with the socket after HTTP is "done" (perhaps due to a protocol upgrade).
• When HTTP/2 is introduced, it is possible to have stream errors which aren't fatal to the connection, but do abort a stream. It wouldn't be feasible to make those stream errors be returned from the Connection future, as it should keep working.
  • Perhaps Connection shouldn't be a Future directly, but more like a Stream... of Errs...
  • The Connection could alternatively grow a on_stream_error<F>(callback: F) where F: Fn(hyper::Error) or something...

Server

The current Server type is supposed to be an "easy mode" for starting up a TCP listener and accepting plain text HTTP requests. There is a related proposal (#1322) to make Server accept any kind of listener, and be a Future so users can execute it with their own executor.

Perhaps that proposal should be updated to support this new Connection future:

• impl Stream for Server instead of Future.
  • The Server could be a Stream of already bound Connections.
  • This would allow someone to use the Server to automatically call Http::bind_connection with a service, and manage graceful shutdown, while still being able to watch for errors that happen in that connection.

We could add some convenience methods to Server, like run_ignore_errors() -> impl Future, so a user could easily just "run" the server without worrying about it being a Stream of Connections.

Usage

Bare bones bind_connection

let http = Http::new();

// we've received a socket and made a service somewhere else
let conn = http.bind_connection(socket, service);
// whatever your executor is
executor.spawn(conn.map_err(move |err| {
    println!("connection [{}] error: {}", addr, err);
}));

Fuller example with a tokio Core and hyper Server

let mut core = Core::new()?;
let handle = core.handle();

let tcp = TcpListener::bind(addr, &handle).map(|listener| {
  listener.incoming()
});
let tls = tcp.and_then(|tcp_streams| {
    tcp_streams.and_then(|(socket, _addr)| {
        TlsAcceptorThing::handshake(socket)
    })
});

let server = tls.map(|tls_streams| {
    let http = Http::new();
    http.serve(tls_streams, || Ok(Echo))
});

let connections = server.and_then(move |srv| {
    // srv is `Server`, so it is a `Stream<Item=Connection>`
    srv.for_each(move |conn| {
        // each connection should be its own task, so spawn it on the executor
        handle.spawn(conn.map_err(|err| {
            println!("http error: {}", err);
        }));
        Ok(())
    })
});

core.run(connections)?;

Meta unresolved questions

• Perhaps the naming of types could be improved as well? (Of course, only finish renaming in 0.12)
  • Currently:
    • Http is more like a ServerBuilder or ConnectionBuilder...
    • If the Http were named a builder, Http could replace Connection? Such that Server is a Stream<Item=Http>?
  • Many people are used to reaching for a Server type, but hyper currently asks you to grab the Http to configure a Server.

[kamalmarhubi]

(aside)

The tokio-proto crate is being de-emphasized.

@seanmonstar is there any discussion to read on this?

[sfackler]

@kamalmarhubi tokio-rs/tokio-rfcs#3 has some background.

[kamalmarhubi]

(TIL https://github.com/tokio-rs/tokio-rfcs)

[aturon]

cc @alexcrichton @withoutboats

Glad to see things heading this way -- please let me know if we can be of any help.

[seanmonstar]

@aturon I'd certainly welcome any thoughts around the API. Pulling out some questions:

• Does it seem bonkers to have a Stream<Item=impl Future>? That is what is suggested by having impl Stream for Server { type Item = Connection }, and impl Future for Connection...

• Do the names make sense? Should the names be shuffled some, considering there would (eventually) no longer be a type exposed as a "protocol" (ServerProto)? Perhaps that makes Http more meaningful as a Builder type.

• With Connection returning fatal connection errors, it's still possible for stream-specific errors to occur on a Connection that otherwise don't tear down the connection. Thoughts on Connection growing a on_h2_stream_error callback option?

  The point here is that one could return a Response, and then part way through flushing it, the client resets the stream (perhaps a cancel notice, perhaps something worse without being a connection error), it would be nice to know if that response were successfully written or not.

[carllerche]

I'd like to help work on this, but I would feel like I have a better sense of how everything should fit together once some of our work on h2 and other surrounding libs start to stabilize.

That said, it is good to start writing this down.

[seanmonstar]

#1362 is available, which starts to address this. Specifically, it adds a configuration option to disable the tokio-proto usage in the server and client. It is not enabled by default, until some more testing shows that at least it has fewer bugs than the tokio-proto dispatch does.

Once determined it should be the default, the no_proto methods will be deprecated, along with the ServerProto pieces that are exposed, and we'll try to move towards the naming discussed in here.

The work in #1362 already has some benefits:

• Seems to have fixed some bugs around keep-alive.
• Can have a small performance gain.
• Better errors are returned on both the client and server.

[seanmonstar]

An update: starting in v0.11.11, the new dispatcher was set to be enabled by default, and the tokio-proto specific parts were deprecated.

• If you use the client, you don't have to do anything, it's just better.
• If you use the server, it depends:
  • Using tokio_proto::TcpServer::new(Http::new()) is deprecated, but will still work for all of hyper 0.11.
  • Http::bind still works the same, just uses the new dispatcher internally.
  • Http::bind_connetion is deprecated in favor of Http::serve_connection.

[sanmai-NL]

@seanmonstar: Do you have any idea how hard it will be to migrate Hyper + rustls to the new API?
See rustls/hyper-rustls#36. How could we adapt hyper-rustls to not use tokio_proto::TcpServer::new?

[seanmonstar]

There is a new Http::serve_incoming that can take any stream of IO objects. The returned Serve is a little harder to use, but it allows direct control to spawn connection tasks on any executor.