refactor(error): improve organization of Error kinds

- Placed all cases of "unexpected bytes" errors into the
  `UnexpectedMessage` variant.
- Placed all cases of "unexpected EOF" errors into the
  `IncompleteMessage` variant. Description is now generic about
  "connection closed before message completed", instead of mentioning
  "request" or "response.
- Added `Error::is_incomplete_message()` accessor to help checking for
  unexpected closures.
- Renamed some variants to be clearer when viewing the `Debug` format.
- Collected all "user" errors into an internal `User` enum, to prevent
  forgetting to update the `is_user()` method.

src/error.rs

@@ -25,14 +25,15 @@ struct ErrorImpl {
 #[derive(Debug, PartialEq)]
 pub(crate) enum Kind {
     Parse(Parse),
+    User(User),
     /// A message reached EOF, but is not complete.
-    Incomplete,
-    /// A client connection received a response when not waiting for one.
-    MismatchedResponse,
+    IncompleteMessage,
+    /// A connection received a message (or bytes) when not waiting for one.
+    UnexpectedMessage,
     /// A pending item was dropped before ever being processed.
     Canceled,
-    /// Indicates a connection is closed.
-    Closed,
+    /// Indicates a channel (client or body sender) is closed.
+    ChannelClosed,
     /// An `io::Error` that occurred while trying to read or write to a network stream.
     Io,
     /// Error occurred while connecting.
@@ -42,22 +43,40 @@ pub(crate) enum Kind {
     Listen,
     /// Error accepting on an Incoming stream.
     Accept,
-    /// Error calling user's NewService::new_service().
-    NewService,
-    /// Error from future of user's Service::call().
-    Service,
     /// Error while reading a body from connection.
     Body,
     /// Error while writing a body to connection.
     BodyWrite,
-    /// Error calling user's Payload::poll_data().
-    BodyUser,
     /// Error calling AsyncWrite::shutdown()
     Shutdown,
 
     /// A general error from h2.
     Http2,
+}
 
+#[derive(Debug, PartialEq)]
+pub(crate) enum Parse {
+    Method,
+    Version,
+    VersionH2,
+    Uri,
+    Header,
+    TooLarge,
+    Status,
+}
+
+#[derive(Debug, PartialEq)]
+pub(crate) enum User {
+    /// Error calling user's Payload::poll_data().
+    Body,
+    /// Error calling user's MakeService.
+    MakeService,
+    /// Error from future of user's Service.
+    Service,
+    /// User tried to send a certain header in an unexpected context.
+    ///
+    /// For example, sending both `content-length` and `transfer-encoding`.
+    UnexpectedHeader,
     /// User tried to create a Request with bad version.
     UnsupportedVersion,
     /// User tried to create a CONNECT Request with the Client.
@@ -77,26 +96,6 @@ pub(crate) enum Kind {
     Execute,
 }
 
-#[derive(Debug, PartialEq)]
-pub(crate) enum Parse {
-    Method,
-    Version,
-    VersionH2,
-    Uri,
-    Header,
-    TooLarge,
-    Status,
-}
-
-/*
-#[derive(Debug)]
-pub(crate) enum User {
-    VersionNotSupported,
-    MethodNotSupported,
-    InvalidRequestUri,
-}
-*/
-
 impl Error {
     /// Returns true if this was an HTTP parse error.
     pub fn is_parse(&self) -> bool {
@@ -109,16 +108,7 @@ impl Error {
     /// Returns true if this error was caused by user code.
     pub fn is_user(&self) -> bool {
         match self.inner.kind {
-            Kind::BodyUser |
-            Kind::NewService |
-            Kind::Service |
-            Kind::Closed |
-            Kind::UnsupportedVersion |
-            Kind::UnsupportedRequestMethod |
-            Kind::UnsupportedStatusCode |
-            Kind::AbsoluteUriRequired |
-            Kind::NoUpgrade |
-            Kind::Execute => true,
+            Kind::User(_) => true,
             _ => false,
         }
     }
@@ -130,7 +120,7 @@ impl Error {
 
     /// Returns true if a sender's channel is closed.
     pub fn is_closed(&self) -> bool {
-        self.inner.kind == Kind::Closed
+        self.inner.kind == Kind::ChannelClosed
     }
 
     /// Returns true if this was an error from `Connect`.
@@ -138,6 +128,11 @@ impl Error {
         self.inner.kind == Kind::Connect
     }
 
+    /// Returns true if the connection closed before a message could complete.
+    pub fn is_incomplete_message(&self) -> bool {
+        self.inner.kind == Kind::IncompleteMessage
+    }
+
     #[doc(hidden)]
     #[cfg_attr(error_source, deprecated(note = "use Error::source instead"))]
     pub fn cause2(&self) -> Option<&(StdError + 'static + Sync + Send)> {
@@ -149,15 +144,20 @@ impl Error {
         self.inner.cause
     }
 
-    pub(crate) fn new(kind: Kind, cause: Option<Cause>) -> Error {
+    pub(crate) fn new(kind: Kind) -> Error {
         Error {
             inner: Box::new(ErrorImpl {
                 kind,
-                cause,
+                cause: None,
             }),
         }
     }
 
+    pub(crate) fn with<C: Into<Cause>>(mut self, cause: C) -> Error {
+        self.inner.cause = Some(cause.into());
+        self
+    }
+
     pub(crate) fn kind(&self) -> &Kind {
         &self.inner.kind
     }
@@ -201,114 +201,122 @@ impl Error {
         h2::Reason::INTERNAL_ERROR
     }
 
-    pub(crate) fn new_canceled<E: Into<Cause>>(cause: Option<E>) -> Error {
-        Error::new(Kind::Canceled, cause.map(Into::into))
+    pub(crate) fn new_canceled() -> Error {
+        Error::new(Kind::Canceled)
     }
 
     pub(crate) fn new_incomplete() -> Error {
-        Error::new(Kind::Incomplete, None)
+        Error::new(Kind::IncompleteMessage)
     }
 
     pub(crate) fn new_too_large() -> Error {
-        Error::new(Kind::Parse(Parse::TooLarge), None)
-    }
-
-    pub(crate) fn new_header() -> Error {
-        Error::new(Kind::Parse(Parse::Header), None)
+        Error::new(Kind::Parse(Parse::TooLarge))
     }
 
     pub(crate) fn new_version_h2() -> Error {
-        Error::new(Kind::Parse(Parse::VersionH2), None)
+        Error::new(Kind::Parse(Parse::VersionH2))
     }
 
-    pub(crate) fn new_mismatched_response() -> Error {
-        Error::new(Kind::MismatchedResponse, None)
+    pub(crate) fn new_unexpected_message() -> Error {
+        Error::new(Kind::UnexpectedMessage)
     }
 
     pub(crate) fn new_io(cause: io::Error) -> Error {
-        Error::new(Kind::Io, Some(cause.into()))
+        Error::new(Kind::Io).with(cause)
     }
 
     #[cfg(feature = "runtime")]
     pub(crate) fn new_listen<E: Into<Cause>>(cause: E) -> Error {
-        Error::new(Kind::Listen, Some(cause.into()))
+        Error::new(Kind::Listen).with(cause)
     }
 
     pub(crate) fn new_accept<E: Into<Cause>>(cause: E) -> Error {
-        Error::new(Kind::Accept, Some(cause.into()))
+        Error::new(Kind::Accept).with(cause)
     }
 
     pub(crate) fn new_connect<E: Into<Cause>>(cause: E) -> Error {
-        Error::new(Kind::Connect, Some(cause.into()))
+        Error::new(Kind::Connect).with(cause)
     }
 
     pub(crate) fn new_closed() -> Error {
-        Error::new(Kind::Closed, None)
+        Error::new(Kind::ChannelClosed)
     }
 
     pub(crate) fn new_body<E: Into<Cause>>(cause: E) -> Error {
-        Error::new(Kind::Body, Some(cause.into()))
+        Error::new(Kind::Body).with(cause)
     }
 
     pub(crate) fn new_body_write<E: Into<Cause>>(cause: E) -> Error {
-        Error::new(Kind::BodyWrite, Some(cause.into()))
+        Error::new(Kind::BodyWrite).with(cause)
+    }
+
+    fn new_user(user: User) -> Error {
+        Error::new(Kind::User(user))
+    }
+
+    pub(crate) fn new_user_header() -> Error {
+        Error::new_user(User::UnexpectedHeader)
     }
 
     pub(crate) fn new_user_unsupported_version() -> Error {
-        Error::new(Kind::UnsupportedVersion, None)
+        Error::new_user(User::UnsupportedVersion)
     }
 
     pub(crate) fn new_user_unsupported_request_method() -> Error {
-        Error::new(Kind::UnsupportedRequestMethod, None)
+        Error::new_user(User::UnsupportedRequestMethod)
     }
 
     pub(crate) fn new_user_unsupported_status_code() -> Error {
-        Error::new(Kind::UnsupportedStatusCode, None)
+        Error::new_user(User::UnsupportedStatusCode)
     }
 
     pub(crate) fn new_user_absolute_uri_required() -> Error {
-        Error::new(Kind::AbsoluteUriRequired, None)
+        Error::new_user(User::AbsoluteUriRequired)
     }
 
     pub(crate) fn new_user_no_upgrade() -> Error {
-        Error::new(Kind::NoUpgrade, None)
+        Error::new_user(User::NoUpgrade)
     }
 
     pub(crate) fn new_user_manual_upgrade() -> Error {
-        Error::new(Kind::ManualUpgrade, None)
+        Error::new_user(User::ManualUpgrade)
     }
 
-    pub(crate) fn new_user_new_service<E: Into<Cause>>(cause: E) -> Error {
-        Error::new(Kind::NewService, Some(cause.into()))
+    pub(crate) fn new_user_make_service<E: Into<Cause>>(cause: E) -> Error {
+        Error::new_user(User::MakeService).with(cause)
     }
 
     pub(crate) fn new_user_service<E: Into<Cause>>(cause: E) -> Error {
-        Error::new(Kind::Service, Some(cause.into()))
+        Error::new_user(User::Service).with(cause)
     }
 
     pub(crate) fn new_user_body<E: Into<Cause>>(cause: E) -> Error {
-        Error::new(Kind::BodyUser, Some(cause.into()))
+        Error::new_user(User::Body).with(cause)
     }
 
     pub(crate) fn new_shutdown(cause: io::Error) -> Error {
-        Error::new(Kind::Shutdown, Some(Box::new(cause)))
+        Error::new(Kind::Shutdown).with(cause)
     }
 
     pub(crate) fn new_execute<E: Into<Cause>>(cause: E) -> Error {
-        Error::new(Kind::Execute, Some(cause.into()))
+        Error::new_user(User::Execute).with(cause)
     }
 
     pub(crate) fn new_h2(cause: ::h2::Error) -> Error {
-        Error::new(Kind::Http2, Some(Box::new(cause)))
+        if cause.is_io() {
+            Error::new_io(cause.into_io().expect("h2::Error::is_io"))
+        } else {
+            Error::new(Kind::Http2).with(cause)
+        }
     }
 }
 
 impl fmt::Debug for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let mut f = f.debug_struct("Error");
-        f.field("kind", &self.inner.kind);
+        let mut f = f.debug_tuple("Error");
+        f.field(&self.inner.kind);
         if let Some(ref cause) = self.inner.cause {
-            f.field("cause", cause);
+            f.field(cause);
         }
         f.finish()
     }
@@ -327,37 +335,38 @@ impl fmt::Display for Error {
 impl StdError for Error {
     fn description(&self) -> &str {
         match self.inner.kind {
-            Kind::Parse(Parse::Method) => "invalid Method specified",
-            Kind::Parse(Parse::Version) => "invalid HTTP version specified",
-            Kind::Parse(Parse::VersionH2) => "invalid HTTP version specified (Http2)",
+            Kind::Parse(Parse::Method) => "invalid HTTP method parsed",
+            Kind::Parse(Parse::Version) => "invalid HTTP version parsed",
+            Kind::Parse(Parse::VersionH2) => "invalid HTTP version parsed (found HTTP2 preface)",
             Kind::Parse(Parse::Uri) => "invalid URI",
-            Kind::Parse(Parse::Header) => "invalid Header provided",
+            Kind::Parse(Parse::Header) => "invalid HTTP header parsed",
             Kind::Parse(Parse::TooLarge) => "message head is too large",
-            Kind::Parse(Parse::Status) => "invalid Status provided",
-            Kind::Incomplete => "parsed HTTP message from remote is incomplete",
-            Kind::MismatchedResponse => "response received without matching request",
-            Kind::Closed => "connection closed",
-            Kind::Connect => "an error occurred trying to connect",
-            Kind::Canceled => "an operation was canceled internally before starting",
+            Kind::Parse(Parse::Status) => "invalid HTTP status-code parsed",
+            Kind::IncompleteMessage => "connection closed before message completed",
+            Kind::UnexpectedMessage => "received unexpected message from connection",
+            Kind::ChannelClosed => "channel closed",
+            Kind::Connect => "error trying to connect",
+            Kind::Canceled => "operation was canceled",
             #[cfg(feature = "runtime")]
             Kind::Listen => "error creating server listener",
             Kind::Accept => "error accepting connection",
-            Kind::NewService => "calling user's new_service failed",
-            Kind::Service => "error from user's server service",
             Kind::Body => "error reading a body from connection",
             Kind::BodyWrite => "error writing a body to connection",
-            Kind::BodyUser => "error from user's Payload stream",
             Kind::Shutdown => "error shutting down connection",
-            Kind::Http2 => "http2 general error",
-            Kind::UnsupportedVersion => "request has unsupported HTTP version",
-            Kind::UnsupportedRequestMethod => "request has unsupported HTTP method",
-            Kind::UnsupportedStatusCode => "response has 1xx status code, not supported by server",
-            Kind::AbsoluteUriRequired => "client requires absolute-form URIs",
-            Kind::NoUpgrade => "no upgrade available",
-            Kind::ManualUpgrade => "upgrade expected but low level API in use",
-            Kind::Execute => "executor failed to spawn task",
+            Kind::Http2 => "http2 error",
+            Kind::Io => "connection error",
 
-            Kind::Io => "an IO error occurred",
+            Kind::User(User::Body) => "error from user's Payload stream",
+            Kind::User(User::MakeService) => "error from user's MakeService",
+            Kind::User(User::Service) => "error from user's Service",
+            Kind::User(User::UnexpectedHeader) => "user sent unexpected header",
+            Kind::User(User::UnsupportedVersion) => "request has unsupported HTTP version",
+            Kind::User(User::UnsupportedRequestMethod) => "request has unsupported HTTP method",
+            Kind::User(User::UnsupportedStatusCode) => "response has 1xx status code, not supported by server",
+            Kind::User(User::AbsoluteUriRequired) => "client requires absolute-form URIs",
+            Kind::User(User::NoUpgrade) => "no upgrade available",
+            Kind::User(User::ManualUpgrade) => "upgrade expected but low level API in use",
+            Kind::User(User::Execute) => "executor failed to spawn task",
         }
     }
 
@@ -383,7 +392,7 @@ impl StdError for Error {
 #[doc(hidden)]
 impl From<Parse> for Error {
     fn from(err: Parse) -> Error {
-        Error::new(Kind::Parse(err), None)
+        Error::new(Kind::Parse(err))
     }
 }
 
@@ -438,8 +447,14 @@ impl AssertSendSync for Error {}
 
 #[cfg(test)]
 mod tests {
+    use std::mem;
     use super::*;
 
+    #[test]
+    fn error_size_of() {
+        assert_eq!(mem::size_of::<Error>(), mem::size_of::<usize>());
+    }
+
     #[test]
     fn h2_reason_unknown() {
         let closed = Error::new_closed();