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.
StreamExpect: 100-continue (#838)Http::bind_connection(socket, service) -> ConnectionCompared to the current bind_connection:
socket is T: AsyncRead + AsyncWrite.service is currently a S: Service.addr is not needed anymore. Anyone wanting to know the address can store it on their Service.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.
ConnectionThis is returned by Http::bind_connection, and implements Future.
Item = ()Error = hyper::ErrorIf 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:
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).Connection future, as it should keep working.Connection shouldn't be a Future directly, but more like a Stream... of Errs...Connection could alternatively grow a on_stream_error<F>(callback: F) where F: Fn(hyper::Error) or something...ServerThe 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.Server could be a Stream of already bound Connections.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.
bind_connectionlet 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);
}));
Core and hyper Serverlet 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)?;
Http is more like a ServerBuilder or ConnectionBuilder...Http were named a builder, Http could replace Connection? Such that Server is a Stream<Item=Http>?Server type, but hyper currently asks you to grab the Http to configure a Server.(aside)
The tokio-proto crate is being de-emphasized.
@seanmonstar is there any discussion to read on this?
@kamalmarhubi https://github.com/tokio-rs/tokio-rfcs/pull/3 has some background.
(TIL https://github.com/tokio-rs/tokio-rfcs)
cc @alexcrichton @withoutboats
Glad to see things heading this way -- please let me know if we can be of any help.
@aturon I'd certainly welcome any thoughts around the API. Pulling out some questions:
Stream<Item=impl Future>? That is what is suggested by having impl Stream for Server { type Item = Connection }, and impl Future for Connection...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.
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.
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:
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.
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.@seanmonstar: Do you have any idea how hard it will be to migrate Hyper + rustls to the new API?
See ctz/hyper-rustls#36. How could we adapt hyper-rustls to not use tokio_proto::TcpServer::new?
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.