From 271bba16672ff54a44e043c5cc1ae6b9345bb172 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Thu, 25 Apr 2019 15:47:38 -0700 Subject: [PATCH] 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/client/conn.rs | 6 +- src/client/dispatch.rs | 2 +- src/client/mod.rs | 4 +- src/client/pool.rs | 6 +- src/client/tests.rs | 5 +- src/error.rs | 217 +++++++++++++++++++++------------------ src/proto/h1/conn.rs | 117 +++++++++------------ src/proto/h1/dispatch.rs | 10 +- src/proto/h1/mod.rs | 17 ++- src/proto/h1/role.rs | 20 ++-- src/proto/h2/client.rs | 8 +- src/proto/h2/mod.rs | 2 +- src/server/conn.rs | 6 +- src/upgrade.rs | 2 +- tests/client.rs | 2 +- 15 files changed, 213 insertions(+), 211 deletions(-) diff --git a/src/client/conn.rs b/src/client/conn.rs index 2195622c..0d9f297d 100644 --- a/src/client/conn.rs +++ b/src/client/conn.rs @@ -235,7 +235,7 @@ where }, Err(_req) => { debug!("connection was not ready"); - let err = ::Error::new_canceled(Some("connection was not ready")); + let err = ::Error::new_canceled().with("connection was not ready"); Either::B(future::err(err)) } }; @@ -262,7 +262,7 @@ where }, Err(req) => { debug!("connection was not ready"); - let err = ::Error::new_canceled(Some("connection was not ready")); + let err = ::Error::new_canceled().with("connection was not ready"); Either::B(future::err((err, Some(req)))) } } @@ -322,7 +322,7 @@ where }, Err(req) => { debug!("connection was not ready"); - let err = ::Error::new_canceled(Some("connection was not ready")); + let err = ::Error::new_canceled().with("connection was not ready"); Either::B(future::err((err, Some(req)))) } } diff --git a/src/client/dispatch.rs b/src/client/dispatch.rs index 247d28eb..16848047 100644 --- a/src/client/dispatch.rs +++ b/src/client/dispatch.rs @@ -167,7 +167,7 @@ struct Envelope(Option<(T, Callback)>); impl Drop for Envelope { fn drop(&mut self) { if let Some((val, cb)) = self.0.take() { - let _ = cb.send(Err((::Error::new_canceled(None::<::Error>), Some(val)))); + let _ = cb.send(Err((::Error::new_canceled().with("connection closed"), Some(val)))); } } } diff --git a/src/client/mod.rs b/src/client/mod.rs index 7c89f651..80e16b2e 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -498,7 +498,7 @@ where C: Connect + Sync + 'static, let connecting = match pool.connecting(&pool_key, ver) { Some(lock) => lock, None => { - let canceled = ::Error::new_canceled(Some("HTTP/2 connection in progress")); + let canceled = ::Error::new_canceled().with("HTTP/2 connection in progress"); return Either::B(future::err(canceled)); } }; @@ -517,7 +517,7 @@ where C: Connect + Sync + 'static, None => { // Another connection has already upgraded, // the pool checkout should finish up for us. - let canceled = ::Error::new_canceled(Some("ALPN upgraded to HTTP/2")); + let canceled = ::Error::new_canceled().with("ALPN upgraded to HTTP/2"); return Either::B(future::err(canceled)); } } diff --git a/src/client/pool.rs b/src/client/pool.rs index 40bf01f7..bf7cefaa 100644 --- a/src/client/pool.rs +++ b/src/client/pool.rs @@ -577,14 +577,14 @@ impl Checkout { if value.is_open() { Ok(Async::Ready(Some(self.pool.reuse(&self.key, value)))) } else { - Err(::Error::new_canceled(Some(CANCELED))) + Err(::Error::new_canceled().with(CANCELED)) } }, Ok(Async::NotReady) => { self.waiter = Some(rx); Ok(Async::NotReady) }, - Err(_canceled) => Err(::Error::new_canceled(Some(CANCELED))), + Err(_canceled) => Err(::Error::new_canceled().with(CANCELED)), } } else { Ok(Async::Ready(None)) @@ -654,7 +654,7 @@ impl Future for Checkout { if let Some(pooled) = self.checkout() { Ok(Async::Ready(pooled)) } else if !self.pool.is_enabled() { - Err(::Error::new_canceled(Some("pool is disabled"))) + Err(::Error::new_canceled().with("pool is disabled")) } else { Ok(Async::NotReady) } diff --git a/src/client/tests.rs b/src/client/tests.rs index a9deaac5..b55dc76d 100644 --- a/src/client/tests.rs +++ b/src/client/tests.rs @@ -103,10 +103,7 @@ fn conn_reset_after_write() { Ok(Async::Ready(())) }).map_err(|e: ::std::io::Error| panic!("srv2 poll_fn error: {}", e)); let err = rt.block_on(res2.join(srv2)).expect_err("res2"); - match err.kind() { - &::error::Kind::Incomplete => (), - other => panic!("expected Incomplete, found {:?}", other) - } + assert!(err.is_incomplete_message(), "{:?}", err); } #[test] diff --git a/src/error.rs b/src/error.rs index 24d0d657..2404c582 100644 --- a/src/error.rs +++ b/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) -> Error { + pub(crate) fn new(kind: Kind) -> Error { Error { inner: Box::new(ErrorImpl { kind, - cause, + cause: None, }), } } + pub(crate) fn with>(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>(cause: Option) -> 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>(cause: E) -> Error { - Error::new(Kind::Listen, Some(cause.into())) + Error::new(Kind::Listen).with(cause) } pub(crate) fn new_accept>(cause: E) -> Error { - Error::new(Kind::Accept, Some(cause.into())) + Error::new(Kind::Accept).with(cause) } pub(crate) fn new_connect>(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>(cause: E) -> Error { - Error::new(Kind::Body, Some(cause.into())) + Error::new(Kind::Body).with(cause) } pub(crate) fn new_body_write>(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>(cause: E) -> Error { - Error::new(Kind::NewService, Some(cause.into())) + pub(crate) fn new_user_make_service>(cause: E) -> Error { + Error::new_user(User::MakeService).with(cause) } pub(crate) fn new_user_service>(cause: E) -> Error { - Error::new(Kind::Service, Some(cause.into())) + Error::new_user(User::Service).with(cause) } pub(crate) fn new_user_body>(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>(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 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::(), mem::size_of::()); + } + #[test] fn h2_reason_unknown() { let closed = Error::new_closed(); diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index a13c48a2..721ed9d0 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -98,7 +98,6 @@ where I: AsyncRead + AsyncWrite, pub fn can_read_head(&self) -> bool { match self.state.reading { - //Reading::Init => true, Reading::Init => { if T::should_read_first() { true @@ -228,13 +227,19 @@ where I: AsyncRead + AsyncWrite, ret } - pub fn read_keep_alive(&mut self) -> Result<(), ::Error> { + pub fn wants_read_again(&mut self) -> bool { + let ret = self.state.notify_read; + self.state.notify_read = false; + ret + } + + pub fn read_keep_alive(&mut self) -> Poll<(), ::Error> { debug_assert!(!self.can_read_head() && !self.can_read_body()); if self.is_mid_message() { - self.mid_message_detect_eof().map_err(::Error::new_io) + self.mid_message_detect_eof() } else { - self.require_empty_read().map_err(::Error::new_io) + self.require_empty_read() } } @@ -245,87 +250,65 @@ where I: AsyncRead + AsyncWrite, } } - pub fn wants_read_again(&mut self) -> bool { - let ret = self.state.notify_read; - self.state.notify_read = false; - ret - } - // This will check to make sure the io object read is empty. // // This should only be called for Clients wanting to enter the idle // state. - fn require_empty_read(&mut self) -> io::Result<()> { + fn require_empty_read(&mut self) -> Poll<(), ::Error> { debug_assert!(!self.can_read_head() && !self.can_read_body()); + debug_assert!(!self.is_mid_message()); + debug_assert!(T::is_client()); if !self.io.read_buf().is_empty() { debug!("received an unexpected {} bytes", self.io.read_buf().len()); - Err(io::Error::new(io::ErrorKind::InvalidData, "unexpected bytes after message ended")) - } else { - match self.try_io_read()? { - Async::Ready(0) => { - // case handled in try_io_read - Ok(()) - }, - Async::Ready(n) => { - debug!("received {} bytes on an idle connection", n); - let desc = if self.state.is_idle() { - "unexpected bytes after message ended" - } else { - "unexpected bytes before writing message" - }; - Err(io::Error::new(io::ErrorKind::InvalidData, desc)) - }, - Async::NotReady => { - trace!("read_keep_alive check: ok, nothing to read yet"); - Ok(()) - }, - } + return Err(::Error::new_unexpected_message()); } + + let num_read = try_ready!(self.force_io_read().map_err(::Error::new_io)); + + if num_read == 0 { + let ret = if self.should_error_on_eof() { + trace!("found unexpected EOF on busy connection: {:?}", self.state); + Err(::Error::new_incomplete()) + } else { + trace!("found EOF on idle connection, closing"); + Ok(Async::Ready(())) + }; + + // order is important: should_error needs state BEFORE close_read + self.state.close_read(); + return ret; + } + + debug!("received unexpected {} bytes on an idle connection", num_read); + Err(::Error::new_unexpected_message()) } - fn mid_message_detect_eof(&mut self) -> io::Result<()> { + fn mid_message_detect_eof(&mut self) -> Poll<(), ::Error> { debug_assert!(!self.can_read_head() && !self.can_read_body()); + debug_assert!(self.is_mid_message()); if self.state.allow_half_close || !self.io.read_buf().is_empty() { - Ok(()) + return Ok(Async::NotReady); + } + + let num_read = try_ready!(self.force_io_read().map_err(::Error::new_io)); + + if num_read == 0 { + trace!("found unexpected EOF on busy connection: {:?}", self.state); + self.state.close_read(); + Err(::Error::new_incomplete()) } else { - self.try_io_read().map(|_| ()) + Ok(Async::Ready(())) } } - fn try_io_read(&mut self) -> Poll { - match self.io.read_from_io() { - Ok(Async::Ready(0)) => { - trace!("try_io_read; found EOF on connection: {:?}", self.state); - let must_error = !self.state.is_idle(); - let ret = if must_error { - let desc = if self.is_mid_message() { - "unexpected EOF waiting for response" - } else { - "unexpected EOF before writing message" - }; - Err(io::Error::new(io::ErrorKind::UnexpectedEof, desc)) - } else { - Ok(Async::Ready(0)) - }; - - // order is important: must_error needs state BEFORE close_read - self.state.close_read(); - ret - }, - Ok(Async::Ready(n)) => { - Ok(Async::Ready(n)) - }, - Ok(Async::NotReady) => { - Ok(Async::NotReady) - }, - Err(e) => { - trace!("try_io_read; error = {}", e); - self.state.close(); - Err(e) - } - } + fn force_io_read(&mut self) -> Poll { + self.io.read_from_io().map_err(|e| { + trace!("force_io_read; io error = {:?}", e); + self.state.close(); + e + }) } diff --git a/src/proto/h1/dispatch.rs b/src/proto/h1/dispatch.rs index 2857b905..df4d0137 100644 --- a/src/proto/h1/dispatch.rs +++ b/src/proto/h1/dispatch.rs @@ -162,7 +162,6 @@ where self.conn.close_read(); } } - } }, Ok(Async::Ready(None)) => { @@ -180,7 +179,7 @@ where // just drop, the body will close automatically } } else { - return self.conn.read_keep_alive().map(Async::Ready); + return self.conn.read_keep_alive(); } } } @@ -486,7 +485,10 @@ where let _ = cb.send(Ok(res)); Ok(()) } else { - Err(::Error::new_mismatched_response()) + // Getting here is likely a bug! An error should have happened + // in Conn::require_empty_read() before ever parsing a + // full message! + Err(::Error::new_unexpected_message()) } }, Err(err) => { @@ -497,7 +499,7 @@ where trace!("canceling queued request with connection error: {}", err); // in this case, the message was never even started, so it's safe to tell // the user that the request was completely canceled - let _ = cb.send(Err((::Error::new_canceled(Some(err)), Some(req)))); + let _ = cb.send(Err((::Error::new_canceled().with(err), Some(req)))); Ok(()) } else { Err(err) diff --git a/src/proto/h1/mod.rs b/src/proto/h1/mod.rs index 5facd135..5ecbb635 100644 --- a/src/proto/h1/mod.rs +++ b/src/proto/h1/mod.rs @@ -31,8 +31,21 @@ pub(crate) trait Http1Transaction { fn on_error(err: &::Error) -> Option>; - fn should_error_on_parse_eof() -> bool; - fn should_read_first() -> bool; + fn is_client() -> bool { + !Self::is_server() + } + + fn is_server() -> bool { + !Self::is_client() + } + + fn should_error_on_parse_eof() -> bool { + Self::is_client() + } + + fn should_read_first() -> bool { + Self::is_server() + } fn update_date() {} } diff --git a/src/proto/h1/role.rs b/src/proto/h1/role.rs index 2f703e65..65693848 100644 --- a/src/proto/h1/role.rs +++ b/src/proto/h1/role.rs @@ -294,7 +294,7 @@ impl Http1Transaction for Server { if wrote_len { warn!("unexpected content-length found, canceling"); rewind(dst); - return Err(::Error::new_header()); + return Err(::Error::new_user_header()); } match msg.body { Some(BodyLength::Known(known_len)) => { @@ -354,7 +354,7 @@ impl Http1Transaction for Server { if fold.0 != len { warn!("multiple Content-Length values found: [{}, {}]", fold.0, len); rewind(dst); - return Err(::Error::new_header()); + return Err(::Error::new_user_header()); } folded = Some(fold); } else { @@ -363,7 +363,7 @@ impl Http1Transaction for Server { } else { warn!("illegal Content-Length value: {:?}", value); rewind(dst); - return Err(::Error::new_header()); + return Err(::Error::new_user_header()); } } if let Some((len, value)) = folded { @@ -403,7 +403,7 @@ impl Http1Transaction for Server { if wrote_len { warn!("unexpected transfer-encoding found, canceling"); rewind(dst); - return Err(::Error::new_header()); + return Err(::Error::new_user_header()); } // check that we actually can send a chunked body... if msg.head.version == Version::HTTP_10 || !Server::can_chunked(msg.req_method, msg.head.subject) { @@ -531,11 +531,7 @@ impl Http1Transaction for Server { Some(msg) } - fn should_error_on_parse_eof() -> bool { - false - } - - fn should_read_first() -> bool { + fn is_server() -> bool { true } @@ -692,13 +688,9 @@ impl Http1Transaction for Client { None } - fn should_error_on_parse_eof() -> bool { + fn is_client() -> bool { true } - - fn should_read_first() -> bool { - false - } } impl Client { diff --git a/src/proto/h2/client.rs b/src/proto/h2/client.rs index be18998d..efcd2af4 100644 --- a/src/proto/h2/client.rs +++ b/src/proto/h2/client.rs @@ -100,7 +100,7 @@ where Ok(Async::Ready(Some((req, cb)))) => { // check that future hasn't been canceled already if cb.is_canceled() { - trace!("request canceled"); + trace!("request callback is canceled"); continue; } let (head, body) = req.into_parts(); @@ -159,11 +159,11 @@ where Ok(Async::NotReady) => return Ok(Async::NotReady), - Ok(Async::Ready(None)) | - Err(_) => { + Ok(Async::Ready(None)) => { trace!("client::dispatch::Sender dropped"); return Ok(Async::Ready(Dispatched::Shutdown)); - } + }, + Err(never) => match never {}, } }, }; diff --git a/src/proto/h2/mod.rs b/src/proto/h2/mod.rs index ff702561..3fe88389 100644 --- a/src/proto/h2/mod.rs +++ b/src/proto/h2/mod.rs @@ -126,7 +126,7 @@ where match try_ready!(self.body_tx.poll_capacity().map_err(::Error::new_body_write)) { Some(0) => {} Some(_) => break, - None => return Err(::Error::new_canceled(None::<::Error>)), + None => return Err(::Error::new_canceled()), } } } else { diff --git a/src/server/conn.rs b/src/server/conn.rs index 333a9689..5132ac8a 100644 --- a/src/server/conn.rs +++ b/src/server/conn.rs @@ -708,7 +708,7 @@ where Ok(Async::NotReady) => return Ok(Async::NotReady), Err(e) => { trace!("make_service closed"); - return Err(::Error::new_user_new_service(e)); + return Err(::Error::new_user_make_service(e)); } } @@ -876,8 +876,8 @@ pub(crate) mod spawn_all { let conn = try_ready!(connecting .poll() .map_err(|err| { - let err = ::Error::new_user_new_service(err); - debug!("connection error: {}", err); + let err = ::Error::new_user_make_service(err); + debug!("connecting error: {}", err); })); let connected = watcher.watch(conn.with_upgrades()); State::Connected(connected) diff --git a/src/upgrade.rs b/src/upgrade.rs index 4cac8557..d649d251 100644 --- a/src/upgrade.rs +++ b/src/upgrade.rs @@ -209,7 +209,7 @@ impl Future for OnUpgrade { Ok(Async::Ready(Ok(upgraded))) => Ok(Async::Ready(upgraded)), Ok(Async::Ready(Err(err))) => Err(err), Err(_oneshot_canceled) => Err( - ::Error::new_canceled(Some(UpgradeExpected(()))) + ::Error::new_canceled().with(UpgradeExpected(())) ), }, None => Err(::Error::new_user_no_upgrade()), diff --git a/tests/client.rs b/tests/client.rs index a435a862..bbc094ad 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -549,7 +549,7 @@ test! { method: GET, url: "http://{addr}/err", }, - error: |err| err.to_string() == "parsed HTTP message from remote is incomplete", + error: |err| err.is_incomplete_message(), } test! {