From b986c862ade173d84013831d363263a5cf0586f6 Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Tue, 4 Aug 2020 16:07:32 -0700 Subject: [PATCH 1/6] enable error backtraces --- build.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/error.rs | 6 +++--- src/lib.rs | 1 + 3 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 build.rs diff --git a/build.rs b/build.rs new file mode 100644 index 00000000..b4207faf --- /dev/null +++ b/build.rs @@ -0,0 +1,60 @@ +use std::env; +use std::fs; +use std::path::Path; +use std::process::{Command, ExitStatus, Stdio}; + +// This code exercises the surface area that we expect of the std Backtrace +// type. If the current toolchain is able to compile it, we go ahead and use +// backtrace in anyhow. +const PROBE: &str = r#" + #![feature(backtrace)] + #![allow(dead_code)] + + use std::backtrace::{Backtrace, BacktraceStatus}; + use std::error::Error; + use std::fmt::{self, Display}; + + #[derive(Debug)] + struct E; + + impl Display for E { + fn fmt(&self, _formatter: &mut fmt::Formatter) -> fmt::Result { + unimplemented!() + } + } + + impl Error for E { + fn backtrace(&self) -> Option<&Backtrace> { + let backtrace = Backtrace::capture(); + match backtrace.status() { + BacktraceStatus::Captured | BacktraceStatus::Disabled | _ => {} + } + unimplemented!() + } + } +"#; + +fn main() { + match compile_probe() { + Some(status) if status.success() => println!("cargo:rustc-cfg=backtrace"), + _ => {} + } +} + +fn compile_probe() -> Option { + let rustc = env::var_os("RUSTC")?; + let out_dir = env::var_os("OUT_DIR")?; + let probefile = Path::new(&out_dir).join("probe.rs"); + fs::write(&probefile, PROBE).ok()?; + Command::new(rustc) + .stderr(Stdio::null()) + .arg("--edition=2018") + .arg("--crate-name=http_types_build") + .arg("--crate-type=lib") + .arg("--emit=metadata") + .arg("--out-dir") + .arg(out_dir) + .arg(probefile) + .status() + .ok() +} diff --git a/src/error.rs b/src/error.rs index 8b887dce..7e2375f5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -72,7 +72,7 @@ impl Error { /// [tracking]: https://github.com/rust-lang/rust/issues/53487 #[cfg(backtrace)] pub fn backtrace(&self) -> &std::backtrace::Backtrace { - self.error.downcast_ref::() + self.error.backtrace() } /// Attempt to downcast the error object to a concrete type. @@ -106,13 +106,13 @@ impl Error { impl Display for Error { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(formatter, "{}", self.error) + Display::fmt(&self.error, formatter) } } impl Debug for Error { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(formatter, "{}", self.error) + Debug::fmt(&self.error, formatter) } } diff --git a/src/lib.rs b/src/lib.rs index 213cf885..1e482127 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -96,6 +96,7 @@ #![deny(missing_debug_implementations, nonstandard_style)] #![warn(missing_docs, unreachable_pub)] #![allow(clippy::new_without_default)] +#![cfg_attr(backtrace, feature(backtrace))] #![cfg_attr(test, deny(warnings))] #![cfg_attr(feature = "docs", feature(doc_cfg))] #![doc(html_favicon_url = "https://yoshuawuyts.com/assets/http-rs/favicon.ico")] From 9446815f9c24eec65c7172197553908e7891c863 Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Tue, 4 Aug 2020 17:34:27 -0700 Subject: [PATCH 2/6] make it easier to conditionally use a backtrace --- src/error.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/error.rs b/src/error.rs index 7e2375f5..6a230742 100644 --- a/src/error.rs +++ b/src/error.rs @@ -71,8 +71,19 @@ impl Error { /// /// [tracking]: https://github.com/rust-lang/rust/issues/53487 #[cfg(backtrace)] - pub fn backtrace(&self) -> &std::backtrace::Backtrace { - self.error.backtrace() + pub fn backtrace(&self) -> Option<&std::backtrace::Backtrace> { + let backtrace = self.error.backtrace(); + if let std::backtrace::BacktraceStatus::Captured = backtrace.status() { + Some(backtrace) + } else { + None + } + } + + #[cfg(not(backtrace))] + #[allow(missing_docs)] + pub fn backtrace(&self) -> Option<()> { + None } /// Attempt to downcast the error object to a concrete type. From 963b1ca272a2a8d60024bf9585c6be427eb16848 Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Wed, 5 Aug 2020 10:43:38 -0700 Subject: [PATCH 3/6] add explanatory comment --- build.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/build.rs b/build.rs index b4207faf..b43a9787 100644 --- a/build.rs +++ b/build.rs @@ -3,9 +3,18 @@ use std::fs; use std::path::Path; use std::process::{Command, ExitStatus, Stdio}; -// This code exercises the surface area that we expect of the std Backtrace -// type. If the current toolchain is able to compile it, we go ahead and use -// backtrace in anyhow. +// This build script is copied from +// [anyhow](https://github.com/dtolnay/anyhow/blob/master/build.rs), +// and is a type of feature detection to determine if the current rust +// toolchain has backtraces available. +// +// It exercises the surface area that we expect of the std Backtrace +// type. If the current toolchain is able to compile it, we enable a +// backtrace compiler configuration flag in http-types. We then +// conditionally require the compiler feature in src/lib.rs with +// `#![cfg_attr(backtrace, feature(backtrace))]` +// and gate our backtrace code behind `#[cfg(backtrace)]` + const PROBE: &str = r#" #![feature(backtrace)] #![allow(dead_code)] From 4a8fb79a8d60861991b43b8fdcebe37e11af6a0e Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Wed, 5 Aug 2020 10:49:29 -0700 Subject: [PATCH 4/6] explains that Error::backtrace can always be called --- src/error.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/error.rs b/src/error.rs index 6a230742..edbb8a8f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -70,6 +70,12 @@ impl Error { /// capturing them all over the place all the time. /// /// [tracking]: https://github.com/rust-lang/rust/issues/53487 + /// + /// Note: This function can be called whether or not backtraces + /// are enabled and available. It will return a `None` variant if + /// compiled on a toolchain that does not support backtraces, or + /// if executed without backtraces enabled with + /// `RUST_LIB_BACKTRACE=1`. #[cfg(backtrace)] pub fn backtrace(&self) -> Option<&std::backtrace::Backtrace> { let backtrace = self.error.backtrace(); From 46d2a1cedb26054aa343653c3ec2ef06dc8791e5 Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Wed, 5 Aug 2020 22:28:11 -0700 Subject: [PATCH 5/6] use eyre --- Cargo.toml | 4 ++-- src/error.rs | 32 +++++++++++++++----------------- src/lib.rs | 2 ++ tests/error.rs | 13 ++++++------- 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c58b339d..cc2fcb12 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,8 +30,6 @@ async-std = { version = "1.6.0", features = ["unstable"] } # features: hyperium/http http = { version = "0.2.0", optional = true } - -anyhow = "1.0.26" cookie = { version = "0.14.0", features = ["percent-encode"] } infer = "0.1.2" pin-project-lite = "0.1.0" @@ -41,6 +39,8 @@ serde = { version = "1.0.106", features = ["derive"] } serde_urlencoded = "0.6.1" rand = "0.7.3" serde_qs = "0.6.0" +backtrace = "0.3.50" +stable-eyre = "0.2.1" [dev-dependencies] http = "0.2.0" diff --git a/src/error.rs b/src/error.rs index edbb8a8f..a8ef9313 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,9 +1,12 @@ //! HTTP error types -use std::error::Error as StdError; -use std::fmt::{self, Debug, Display}; - use crate::StatusCode; +use stable_eyre::{eyre, BacktraceExt}; +use std::error::Error as StdError; +use std::{ + fmt::{self, Debug, Display}, + sync::Once, +}; /// A specialized `Result` type for HTTP operations. /// @@ -13,17 +16,20 @@ pub type Result = std::result::Result; /// The error type for HTTP operations. pub struct Error { - error: anyhow::Error, + error: eyre::Report, status: crate::StatusCode, } +static INSTALL_EYRE_HANDLER: Once = Once::new(); + impl Error { /// Create a new error object from any error type. /// /// The error type must be threadsafe and 'static, so that the Error will be /// as well. If the error type does not provide a backtrace, a backtrace will /// be created here to ensure that a backtrace exists. - pub fn new(status: StatusCode, error: impl Into) -> Self { + pub fn new(status: StatusCode, error: impl Into) -> Self { + INSTALL_EYRE_HANDLER.call_once(|| stable_eyre::install().unwrap()); Self { status, error: error.into(), @@ -35,10 +41,7 @@ impl Error { where M: Display + Debug + Send + Sync + 'static, { - Self { - status, - error: anyhow::Error::msg(msg), - } + Self::new(status, eyre::Error::msg(msg)) } /// Create a new error from a message. @@ -77,13 +80,8 @@ impl Error { /// if executed without backtraces enabled with /// `RUST_LIB_BACKTRACE=1`. #[cfg(backtrace)] - pub fn backtrace(&self) -> Option<&std::backtrace::Backtrace> { - let backtrace = self.error.backtrace(); - if let std::backtrace::BacktraceStatus::Captured = backtrace.status() { - Some(backtrace) - } else { - None - } + pub fn backtrace(&self) -> Option<&backtrace::Backtrace> { + self.error.backtrace() } #[cfg(not(backtrace))] @@ -133,7 +131,7 @@ impl Debug for Error { } } -impl> From for Error { +impl> From for Error { fn from(error: E) -> Self { Self::new(StatusCode::InternalServerError, error) } diff --git a/src/lib.rs b/src/lib.rs index 1e482127..e5195032 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -153,6 +153,8 @@ pub use status::Status; pub use status_code::StatusCode; pub use version::Version; +pub use backtrace; + #[doc(inline)] pub use trailers::Trailers; diff --git a/tests/error.rs b/tests/error.rs index 06a59776..c63b3682 100644 --- a/tests/error.rs +++ b/tests/error.rs @@ -72,15 +72,14 @@ fn option_ext() { } #[test] -fn anyhow_error_into_http_types_error() { - let anyhow_error = - anyhow::Error::new(std::io::Error::new(std::io::ErrorKind::Other, "irrelevant")); - let http_types_error: Error = anyhow_error.into(); +fn eyre_error_into_http_types_error() { + use stable_eyre::eyre; + let eyre_error = eyre::Error::new(std::io::Error::new(std::io::ErrorKind::Other, "irrelevant")); + let http_types_error: Error = eyre_error.into(); assert_eq!(http_types_error.status(), StatusCode::InternalServerError); - let anyhow_error = - anyhow::Error::new(std::io::Error::new(std::io::ErrorKind::Other, "irrelevant")); - let http_types_error: Error = Error::new(StatusCode::ImATeapot, anyhow_error); + let eyre_error = eyre::Error::new(std::io::Error::new(std::io::ErrorKind::Other, "irrelevant")); + let http_types_error: Error = Error::new(StatusCode::ImATeapot, eyre_error); assert_eq!(http_types_error.status(), StatusCode::ImATeapot); } From 209f21c6e0906532380b09f87859099630de5ff2 Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Wed, 5 Aug 2020 22:39:55 -0700 Subject: [PATCH 6/6] cheap fix: don't panic, accept that the Once is maybe called more than once --- src/error.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index a8ef9313..1185bd9b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -29,7 +29,9 @@ impl Error { /// as well. If the error type does not provide a backtrace, a backtrace will /// be created here to ensure that a backtrace exists. pub fn new(status: StatusCode, error: impl Into) -> Self { - INSTALL_EYRE_HANDLER.call_once(|| stable_eyre::install().unwrap()); + INSTALL_EYRE_HANDLER.call_once(|| { + stable_eyre::install().ok(); + }); Self { status, error: error.into(),