Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: optimize error type #3596

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/body/incoming.rs
Expand Up @@ -253,7 +253,7 @@ impl Body for Incoming {
Some(h2::Reason::NO_ERROR) | Some(h2::Reason::CANCEL) => {
Poll::Ready(None)
}
_ => Poll::Ready(Some(Err(crate::Error::new_body(e)))),
_ => Poll::Ready(Some(Err(crate::Error::new_body_h2(e)))),
};
}
None => {
Expand Down
67 changes: 52 additions & 15 deletions src/error.rs
@@ -1,11 +1,28 @@
//! Error and Result module.
use std::error::Error as StdError;
use std::fmt;
use std::fmt::{Debug, Formatter};

/// Result type often returned from methods that can have hyper `Error`s.
pub type Result<T> = std::result::Result<T, Error>;

type Cause = Box<dyn StdError + Send + Sync>;
enum Cause {
#[cfg(all(any(feature = "client", feature = "server"), feature = "http2"))]
H2(h2::Error),
Other(GenericError),
}

impl Debug for Cause {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
Cause::Other(e) => e.fmt(f),
#[cfg(all(any(feature = "client", feature = "server"), feature = "http2"))]
Cause::H2(e) => e.fmt(f),
}
}
}

type GenericError = Box<dyn StdError + Send + Sync>;

/// Represents errors that can occur handling HTTP streams.
///
Expand All @@ -29,7 +46,7 @@ type Cause = Box<dyn StdError + Send + Sync>;
/// on**. They may come from private internal dependencies, and are subject to
/// change at any moment.
pub struct Error {
inner: Box<ErrorImpl>,
inner: ErrorImpl,
}

struct ErrorImpl {
Expand Down Expand Up @@ -244,12 +261,18 @@ impl Error {

pub(super) fn new(kind: Kind) -> Error {
Error {
inner: Box::new(ErrorImpl { kind, cause: None }),
inner: ErrorImpl { kind, cause: None },
}
}

pub(super) fn with<C: Into<Cause>>(mut self, cause: C) -> Error {
self.inner.cause = Some(cause.into());
pub(super) fn with<C: Into<GenericError>>(mut self, cause: C) -> Error {
self.inner.cause = Some(Cause::Other(cause.into()));
self
}

#[cfg(all(any(feature = "client", feature = "server"), feature = "http2"))]
pub(super) fn with_h2(mut self, cause: h2::Error) -> Error {
self.inner.cause = Some(Cause::H2(cause));
self
}

Expand Down Expand Up @@ -324,15 +347,21 @@ impl Error {
any(feature = "client", feature = "server"),
any(feature = "http1", feature = "http2")
))]
pub(super) fn new_body<E: Into<Cause>>(cause: E) -> Error {
#[allow(unused)]
pub(super) fn new_body<E: Into<GenericError>>(cause: E) -> Error {
Error::new(Kind::Body).with(cause)
}

#[cfg(all(any(feature = "client", feature = "server"), feature = "http2"))]
pub(super) fn new_body_h2(cause: h2::Error) -> Error {
Error::new(Kind::Body).with_h2(cause)
}

#[cfg(all(
any(feature = "client", feature = "server"),
any(feature = "http1", feature = "http2")
))]
pub(super) fn new_body_write<E: Into<Cause>>(cause: E) -> Error {
pub(super) fn new_body_write<E: Into<GenericError>>(cause: E) -> Error {
Error::new(Kind::BodyWrite).with(cause)
}

Expand Down Expand Up @@ -378,15 +407,15 @@ impl Error {
all(any(feature = "client", feature = "server"), feature = "http1"),
all(feature = "server", feature = "http2")
))]
pub(super) fn new_user_service<E: Into<Cause>>(cause: E) -> Error {
pub(super) fn new_user_service<E: Into<GenericError>>(cause: E) -> Error {
Error::new_user(User::Service).with(cause)
}

#[cfg(all(
any(feature = "client", feature = "server"),
any(feature = "http1", feature = "http2")
))]
pub(super) fn new_user_body<E: Into<Cause>>(cause: E) -> Error {
pub(super) fn new_user_body<E: Into<GenericError>>(cause: E) -> Error {
Error::new_user(User::Body).with(cause)
}

Expand All @@ -410,7 +439,7 @@ impl Error {
if cause.is_io() {
Error::new_io(cause.into_io().expect("h2::Error::is_io"))
} else {
Error::new(Kind::Http2).with(cause)
Error::new(Kind::Http2).with_h2(cause)
}
}

Expand Down Expand Up @@ -530,10 +559,11 @@ impl fmt::Display for Error {

impl StdError for Error {
fn source(&self) -> Option<&(dyn StdError + 'static)> {
self.inner
.cause
.as_ref()
.map(|cause| &**cause as &(dyn StdError + 'static))
self.inner.cause.as_ref().map(|cause| match cause {
Cause::Other(c) => &**c as &(dyn StdError + 'static),
#[cfg(all(any(feature = "client", feature = "server"), feature = "http2"))]
Cause::H2(c) => &*c as &(dyn StdError + 'static),
})
}
}

Expand Down Expand Up @@ -625,7 +655,14 @@ mod tests {

#[test]
fn error_size_of() {
assert_eq!(mem::size_of::<Error>(), mem::size_of::<usize>());
const PREFERRED: usize = mem::size_of::<usize>() * 8;

assert!(
mem::size_of::<Error>() <= PREFERRED,
"Size of error was {}, we prefer <= {}",
mem::size_of::<Error>(),
PREFERRED
);
}

#[cfg(feature = "http2")]
Expand Down