feat(error): revamp hyper::Error type

**The `Error` is now an opaque struct**, which allows for more variants to be added freely, and the internal representation to change without being breaking changes. For inspecting an `Error`, there are several `is_*` methods to check for certain classes of errors, such as `Error::is_parse()`. The `cause` can also be inspected, like before. This likely seems like a downgrade, but more inspection can be added as needed! The `Error` now knows about more states, which gives much more context around when a certain error occurs. This is also expressed in the description and `fmt` messages. **Most places where a user would provide an error to hyper can now pass any error type** (`E: Into<Box<std::error::Error>>`). This error is passed back in relevant places, and can be useful for logging. This should make it much clearer about what error a user should provide to hyper: any it feels is relevant! Closes #1128 Closes #1130 Closes #1431 Closes #1338 BREAKING CHANGE: `Error` is no longer an enum to pattern match over, or to construct. Code will need to be updated accordingly. For body streams or `Service`s, inference might be unable to determine what error type you mean to return. Starting in Rust 1.26, you could just label that as `!` if you never return an error.
2018-04-10 14:29:34 -07:00
parent 33874f9a75
commit 5d3c472228
22 changed files with 519 additions and 407 deletions
--- a/src/client/conn.rs
+++ b/src/client/conn.rs
@@ -45,7 +45,7 @@ pub struct SendRequest<B> {
 pub struct Connection<T, B>
 where
    T: AsyncRead + AsyncWrite,
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
 {
    inner: proto::dispatch::Dispatcher<
        proto::dispatch::Client<B>,
@@ -138,7 +138,7 @@ impl<B> SendRequest<B>

 impl<B> SendRequest<B>
 where
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
 {
    /// Sends a `Request` on the associated connection.
    ///
@@ -262,7 +262,7 @@ impl<B> fmt::Debug for SendRequest<B> {
 impl<T, B> Connection<T, B>
 where
    T: AsyncRead + AsyncWrite,
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
 {
    /// Return the inner IO object, and additional information.
    pub fn into_parts(self) -> Parts<T> {
@@ -289,7 +289,7 @@ where
 impl<T, B> Future for Connection<T, B>
 where
    T: AsyncRead + AsyncWrite,
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
 {
    type Item = ();
    type Error = ::Error;
@@ -302,7 +302,7 @@ where
 impl<T, B> fmt::Debug for Connection<T, B>
 where
    T: AsyncRead + AsyncWrite + fmt::Debug,
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
 {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        f.debug_struct("Connection")
@@ -331,7 +331,7 @@ impl Builder {
    pub fn handshake<T, B>(&self, io: T) -> Handshake<T, B>
    where
        T: AsyncRead + AsyncWrite,
-        B: Entity<Error=::Error> + 'static,
+        B: Entity + 'static,
    {
        Handshake {
            inner: HandshakeInner {
@@ -345,7 +345,7 @@ impl Builder {
    pub(super) fn handshake_no_upgrades<T, B>(&self, io: T) -> HandshakeNoUpgrades<T, B>
    where
        T: AsyncRead + AsyncWrite,
-        B: Entity<Error=::Error> + 'static,
+        B: Entity + 'static,
    {
        HandshakeNoUpgrades {
            inner: HandshakeInner {
@@ -362,7 +362,7 @@ impl Builder {
 impl<T, B> Future for Handshake<T, B>
 where
    T: AsyncRead + AsyncWrite,
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
 {
    type Item = (SendRequest<B>, Connection<T, B>);
    type Error = ::Error;
@@ -387,7 +387,7 @@ impl<T, B> fmt::Debug for Handshake<T, B> {
 impl<T, B> Future for HandshakeNoUpgrades<T, B>
 where
    T: AsyncRead + AsyncWrite,
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
 {
    type Item = (SendRequest<B>, proto::dispatch::Dispatcher<
        proto::dispatch::Client<B>,
@@ -406,7 +406,7 @@ where
 impl<T, B, R> Future for HandshakeInner<T, B, R>
 where
    T: AsyncRead + AsyncWrite,
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
    R: proto::Http1Transaction<
        Incoming=StatusCode,
        Outgoing=proto::RequestLine,
@@ -470,7 +470,7 @@ impl<B: Send> AssertSendSync for SendRequest<B> {}
 impl<T: Send, B: Send> AssertSend for Connection<T, B>
 where
    T: AsyncRead + AsyncWrite,
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
    B::Data: Send + 'static,
 {}

@@ -478,7 +478,7 @@ where
 impl<T: Send + Sync, B: Send + Sync> AssertSendSync for Connection<T, B>
 where
    T: AsyncRead + AsyncWrite,
-    B: Entity<Error=::Error> + 'static,
+    B: Entity + 'static,
    B::Data: Send + Sync + 'static,
 {}

--- a/src/client/connect.rs
+++ b/src/client/connect.rs
@@ -36,7 +36,7 @@ pub trait Connect: Send + Sync {
    /// The connected IO Stream.
    type Transport: AsyncRead + AsyncWrite + Send + 'static;
    /// An error occured when trying to connect.
-    type Error;
+    type Error: Into<Box<StdError + Send + Sync>>;
    /// A Future that will resolve to the connected Transport.
    type Future: Future<Item=(Self::Transport, Connected), Error=Self::Error> + Send;
    /// Connect to a destination.
--- a/src/client/dispatch.rs
+++ b/src/client/dispatch.rs
@@ -39,10 +39,10 @@ impl<T, U> Sender<T, U> {
                // there's room in the queue, but does the Connection
                // want a message yet?
                self.giver.poll_want()
-                    .map_err(|_| ::Error::Closed)
+                    .map_err(|_| ::Error::new_closed())
            },
            Ok(Async::NotReady) => Ok(Async::NotReady),
-            Err(_) => Err(::Error::Closed),
+            Err(_) => Err(::Error::new_closed()),
        }
    }

@@ -184,9 +184,12 @@ mod tests {
            drop(rx);

            promise.then(|fulfilled| {
-                let res = fulfilled.expect("fulfilled");
-                match res.unwrap_err() {
-                    (::Error::Cancel(_), Some(_)) => (),
+                let err = fulfilled
+                    .expect("fulfilled")
+                    .expect_err("promise should error");
+
+                match (err.0.kind(), err.1) {
+                    (&::error::Kind::Canceled, Some(_)) => (),
                    e => panic!("expected Error::Cancel(_), found {:?}", e),
                }

--- a/src/client/mod.rs
+++ b/src/client/mod.rs
@@ -96,10 +96,10 @@ impl<C, B> Client<C, B> {
 }

 impl<C, B> Client<C, B>
-where C: Connect<Error=io::Error> + Sync + 'static,
+where C: Connect + Sync + 'static,
      C::Transport: 'static,
      C::Future: 'static,
-      B: Entity<Error=::Error> + Send + 'static,
+      B: Entity + Send + 'static,
      B::Data: Send,
 {

@@ -139,13 +139,14 @@ where C: Connect<Error=io::Error> + Sync + 'static,
            Version::HTTP_11 => (),
            other => {
                error!("Request has unsupported version \"{:?}\"", other);
-                return FutureResponse(Box::new(future::err(::Error::Version)));
+                //TODO: replace this with a proper variant
+                return FutureResponse(Box::new(future::err(::Error::new_user_unsupported_version())));
            }
        }

        if req.method() == &Method::CONNECT {
            debug!("Client does not support CONNECT requests");
-            return FutureResponse(Box::new(future::err(::Error::Method)));
+            return FutureResponse(Box::new(future::err(::Error::new_user_unsupported_request_method())));
        }

        let uri = req.uri().clone();
@@ -154,7 +155,8 @@ where C: Connect<Error=io::Error> + Sync + 'static,
                format!("{}://{}", scheme, auth)
            }
            _ => {
-                return FutureResponse(Box::new(future::err(::Error::Io(
+                //TODO: replace this with a proper variant
+                return FutureResponse(Box::new(future::err(::Error::new_io(
                    io::Error::new(
                        io::ErrorKind::InvalidInput,
                        "invalid URI for Client Request"
@@ -203,13 +205,13 @@ where C: Connect<Error=io::Error> + Sync + 'static,
            };
            future::lazy(move || {
                connector.connect(dst)
-                    .from_err()
+                    .map_err(::Error::new_connect)
                    .and_then(move |(io, connected)| {
                        conn::Builder::new()
                            .h1_writev(h1_writev)
                            .handshake_no_upgrades(io)
                            .and_then(move |(tx, conn)| {
-                                executor.execute(conn.map_err(|e| debug!("client connection error: {}", e)))?;
+                                executor.execute(conn.map_err(|e| debug!("client connection error: {}", e)));
                                Ok(pool.pooled(pool_key, PoolClient {
                                    is_proxied: connected.is_proxied,
                                    tx: tx,
@@ -260,9 +262,7 @@ where C: Connect<Error=io::Error> + Sync + 'static,
                    } else if !res.body().is_empty() {
                        let (delayed_tx, delayed_rx) = oneshot::channel();
                        res.body_mut().delayed_eof(delayed_rx);
-                        // If the executor doesn't have room, oh well. Things will likely
-                        // be blowing up soon, but this specific task isn't required.
-                        let _ = executor.execute(
+                        executor.execute(
                            future::poll_fn(move || {
                                pooled.tx.poll_ready()
                            })
@@ -277,7 +277,6 @@ where C: Connect<Error=io::Error> + Sync + 'static,
                    Ok(res)
                });

-
            fut
        });

@@ -290,9 +289,9 @@ where C: Connect<Error=io::Error> + Sync + 'static,
 }

 impl<C, B> Service for Client<C, B>
-where C: Connect<Error=io::Error> + 'static,
+where C: Connect + 'static,
      C::Future: 'static,
-      B: Entity<Error=::Error> + Send + 'static,
+      B: Entity + Send + 'static,
      B::Data: Send,
 {
    type Request = Request<B>;
@@ -353,9 +352,9 @@ struct RetryableSendRequest<C, B> {

 impl<C, B> Future for RetryableSendRequest<C, B>
 where
-    C: Connect<Error=io::Error> + 'static,
+    C: Connect + 'static,
    C::Future: 'static,
-    B: Entity<Error=::Error> + Send + 'static,
+    B: Entity + Send + 'static,
    B::Data: Send,
 {
    type Item = Response<Body>;
@@ -564,10 +563,10 @@ impl<C, B> Config<C, B> {
 }

 impl<C, B> Config<C, B>
-where C: Connect<Error=io::Error>,
+where C: Connect,
      C::Transport: 'static,
      C::Future: 'static,
-      B: Entity<Error=::Error> + Send,
+      B: Entity + Send,
      B::Data: Send,
 {
    /// Construct the Client with this configuration.
@@ -590,7 +589,7 @@ where C: Connect<Error=io::Error>,

 impl<B> Config<UseDefaultConnector, B>
 where
-    B: Entity<Error=::Error> + Send,
+    B: Entity + Send,
    B::Data: Send,
 {
    /// Construct the Client with this configuration.
@@ -640,7 +639,6 @@ impl<C: Clone, B> Clone for Config<C, B> {
    }
 }

-
 // ===== impl Exec =====

 #[derive(Clone)]
@@ -655,24 +653,19 @@ impl Exec {
        Exec::Executor(Arc::new(executor))
    }

-    fn execute<F>(&self, fut: F) -> io::Result<()>
+    fn execute<F>(&self, fut: F)
    where
        F: Future<Item=(), Error=()> + Send + 'static,
    {
        match *self {
            Exec::Default => spawn(fut),
            Exec::Executor(ref e) => {
-                e.execute(bg(Box::new(fut)))
+                let _ = e.execute(bg(Box::new(fut)))
                    .map_err(|err| {
-                        debug!("executor error: {:?}", err.kind());
-                        io::Error::new(
-                            io::ErrorKind::Other,
-                            "executor error",
-                        )
-                    })?
+                        panic!("executor error: {:?}", err.kind());
+                    });
            },
        }
-        Ok(())
    }
 }

--- a/src/client/pool.rs
+++ b/src/client/pool.rs
@@ -245,7 +245,7 @@ impl<T: Closed + Send + 'static> Pool<T> {
            interval: interval,
            pool: Arc::downgrade(&self.inner),
            pool_drop_notifier: rx,
-        }).unwrap();
+        });
    }
 }

--- a/src/client/tests.rs
+++ b/src/client/tests.rs
@@ -35,7 +35,7 @@ fn retryable_request() {
            try_ready!(sock1.read(&mut [0u8; 512]));
            try_ready!(sock1.write(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"));
            Ok(Async::Ready(()))
-        });
+        }).map_err(|e: ::std::io::Error| panic!("srv1 poll_fn error: {}", e));
        res1.join(srv1).wait().expect("res1");
    }
    drop(sock1);
@@ -52,7 +52,7 @@ fn retryable_request() {
        try_ready!(sock2.read(&mut [0u8; 512]));
        try_ready!(sock2.write(b"HTTP/1.1 222 OK\r\nContent-Length: 0\r\n\r\n"));
        Ok(Async::Ready(()))
-    });
+    }).map_err(|e: ::std::io::Error| panic!("srv2 poll_fn error: {}", e));

    res2.join(srv2).wait().expect("res2");
 }
@@ -82,7 +82,7 @@ fn conn_reset_after_write() {
            try_ready!(sock1.read(&mut [0u8; 512]));
            try_ready!(sock1.write(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"));
            Ok(Async::Ready(()))
-        });
+        }).map_err(|e: ::std::io::Error| panic!("srv1 poll_fn error: {}", e));
        res1.join(srv1).wait().expect("res1");
    }

@@ -105,10 +105,10 @@ fn conn_reset_after_write() {
        try_ready!(sock1.as_mut().unwrap().read(&mut [0u8; 512]));
        sock1.take();
        Ok(Async::Ready(()))
-    });
+    }).map_err(|e: ::std::io::Error| panic!("srv2 poll_fn error: {}", e));
    let err = res2.join(srv2).wait().expect_err("res2");
-    match err {
-        ::Error::Incomplete => (),
+    match err.kind() {
+        &::error::Kind::Incomplete => (),
        other => panic!("expected Incomplete, found {:?}", other)
    }
 }