From c760ed063e1fcc69308b0708501e8fd2d74e6444 Mon Sep 17 00:00:00 2001 From: Jonathan Reem Date: Sun, 7 Sep 2014 09:15:35 +0200 Subject: [PATCH 1/2] Change the Handler trait to receive an Iterator of (Request, Response) pairs. This allows downstream users to have total control of their concurrency strategy, while also exposing a very nice, streaming interface for frameworks to build on. This also resolves issues surrounding the use of IoResult as the return type of Handler::handle, because handlers now have complete control over how to handle internal failure. Fixes #3 Fixes #4 --- src/server/mod.rs | 77 ++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/src/server/mod.rs b/src/server/mod.rs index bc9f0bb8a9..fee5f150b0 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -1,6 +1,6 @@ //! HTTP Server use std::io::net::tcp::{TcpListener, TcpAcceptor}; -use std::io::{Acceptor, Listener, IoResult, EndOfFile}; +use std::io::{Acceptor, Listener, IoResult, EndOfFile, IncomingConnections}; use std::io::net::ip::{IpAddr, Port, SocketAddr}; pub use self::request::Request; @@ -30,7 +30,7 @@ impl Server { } /// Binds to a socket, and starts handling connections. - pub fn listen(self, mut handler: H) -> IoResult { + pub fn listen(self, handler: H) -> IoResult { let mut listener = try!(TcpListener::bind(self.ip.to_string().as_slice(), self.port)); let socket = try!(listener.socket_name()); let acceptor = try!(listener.listen()); @@ -38,34 +38,7 @@ impl Server { spawn(proc() { let mut acceptor = worker; - for conn in acceptor.incoming() { - match conn { - Ok(stream) => { - debug!("Incoming stream"); - let clone = stream.clone(); - let req = match Request::new(stream) { - Ok(r) => r, - Err(err) => { - error!("creating Request: {}", err); - continue; - } - }; - let mut res = Response::new(clone); - res.version = req.version; - match handler.handle(req, res) { - Ok(..) => debug!("Stream handled"), - Err(e) => { - error!("Error from handler: {}", e) - //TODO try to send a status code - } - } - }, - Err(ref e) if e.kind == EndOfFile => break, // server closed - Err(e) => { - error!("Connection failed: {}", e); - } - } - } + handler.handle(Incoming { from: acceptor.incoming() }); }); Ok(Listening { @@ -76,6 +49,41 @@ impl Server { } +/// An iterator over incoming connections, represented as pairs of +/// hyper Requests and Responses. +pub struct Incoming<'a> { + from: IncomingConnections<'a, TcpAcceptor> +} + +impl<'a> Iterator<(Request, Response)> for Incoming<'a> { + fn next(&mut self) -> Option<(Request, Response)> { + for conn in self.from { + match conn { + Ok(stream) => { + debug!("Incoming stream"); + let clone = stream.clone(); + let req = match Request::new(stream) { + Ok(r) => r, + Err(err) => { + error!("creating Request: {}", err); + continue; + } + }; + let mut res = Response::new(clone); + res.version = req.version; + return Some((req, res)) + }, + Err(ref e) if e.kind == EndOfFile => return None, // server closed + Err(e) => { + error!("Connection failed: {}", e); + continue; + } + } + } + None + } +} + /// A listening server, which can later be closed. pub struct Listening { acceptor: TcpAcceptor, @@ -96,11 +104,12 @@ pub trait Handler: Send { /// Receives a `Request`/`Response` pair, and should perform some action on them. /// /// This could reading from the request, and writing to the response. - fn handle(&mut self, req: Request, res: Response) -> IoResult<()>; + fn handle(self, Incoming); } -impl Handler for fn(Request, Response) -> IoResult<()> { - fn handle(&mut self, req: Request, res: Response) -> IoResult<()> { - (*self)(req, res) +impl Handler for fn(Incoming) { + fn handle(self, incoming: Incoming) { + (self)(incoming) } } + From 3fbf3040cdd4b685a9270c4d9cd165a6dd667147 Mon Sep 17 00:00:00 2001 From: Jonathan Reem Date: Sun, 7 Sep 2014 09:37:53 +0200 Subject: [PATCH 2/2] Updated example for new Handler trait This introduces a bit of complexity to the example, mainly the use of the try_continue! macro for dealing with errors, but I think this is an appropriate trade-off as users of this library are likely to be framework authors instead of end users. --- examples/server.rs | 55 ++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/examples/server.rs b/examples/server.rs index 2848336922..30e24969d1 100644 --- a/examples/server.rs +++ b/examples/server.rs @@ -1,38 +1,55 @@ +#![feature(macro_rules)] + extern crate hyper; extern crate debug; -use std::io::{IoResult}; use std::io::util::copy; use std::io::net::ip::Ipv4Addr; use hyper::{Get, Post}; -use hyper::server::{Server, Handler, Request, Response}; +use hyper::server::{Server, Handler, Incoming}; use hyper::header::ContentLength; struct Echo; +macro_rules! try_continue( + ($e:expr) => {{ + match $e { + Ok(v) => v, + Err(e) => { println!("Error: {}", e); continue; } + } + }} +) + impl Handler for Echo { - fn handle(&mut self, mut req: Request, mut res: Response) -> IoResult<()> { - match req.uri { - hyper::uri::AbsolutePath(ref path) => match (&req.method, path.as_slice()) { - (&Get, "/") | (&Get, "/echo") => { - let out = b"Try POST /echo"; - - res.headers.set(ContentLength(out.len())); - try!(res.write(out)); - return res.end(); + fn handle(self, mut incoming: Incoming) { + for (mut req, mut res) in incoming { + match req.uri { + hyper::uri::AbsolutePath(ref path) => match (&req.method, path.as_slice()) { + (&Get, "/") | (&Get, "/echo") => { + let out = b"Try POST /echo"; + + res.headers.set(ContentLength(out.len())); + try_continue!(res.write(out)); + try_continue!(res.end()); + continue; + }, + (&Post, "/echo") => (), // fall through, fighting mutable borrows + _ => { + res.status = hyper::status::NotFound; + try_continue!(res.end()); + continue; + } }, - (&Post, "/echo") => (), // fall through, fighting mutable borrows _ => { - res.status = hyper::status::NotFound; - return res.end(); + try_continue!(res.end()); + continue; } - }, - _ => return res.end() - }; + }; - try!(copy(&mut req, &mut res)); - res.end() + try_continue!(copy(&mut req, &mut res)); + try_continue!(res.end()); + } } }