From 990aba576ccb4c8bdef3df7090a9892b73e3e852 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 2 Jun 2022 20:55:46 +0200 Subject: [PATCH 01/26] Add `provide` method --- packages/engine/lib/error/src/future.rs | 64 +++++++++++++++++++ packages/engine/lib/error/src/lib.rs | 2 + packages/engine/lib/error/src/report.rs | 47 ++++++++++++++ packages/engine/lib/error/src/result.rs | 56 ++++++++++++++++ .../engine/lib/error/src/single_provider.rs | 24 +++++++ 5 files changed, 193 insertions(+) create mode 100644 packages/engine/lib/error/src/single_provider.rs diff --git a/packages/engine/lib/error/src/future.rs b/packages/engine/lib/error/src/future.rs index 9edf5fd4725..312a477f2ea 100644 --- a/packages/engine/lib/error/src/future.rs +++ b/packages/engine/lib/error/src/future.rs @@ -123,6 +123,20 @@ implement_lazy_future_adaptor!( Fut::Output ); +implement_future_adaptor!( + FutureWithProvided, + provide, + Display + Debug + Send + Sync + 'static, + Fut::Output +); + +implement_lazy_future_adaptor!( + FutureWithLazyProvided, + provide_lazy, + Display + Debug + Send + Sync + 'static, + Fut::Output +); + implement_future_adaptor!( FutureWithContext, change_context, @@ -260,6 +274,35 @@ pub trait FutureExt: Future + Sized { P: Provider + Display + Debug + Send + Sync + 'static, F: FnOnce() -> P; + /// Adds the provided object to the [`Frame`] stack when [`poll`]ing the [`Future`]. + /// + /// The object can later be retrieved by calling [`request_value()`]. + /// + /// The function is only executed in the `Err` arm. + /// + /// [`request_ref()`]: crate::Report::request_ref + /// [`Frame`]: crate::Frame + /// [`poll`]: Future::poll + #[cfg(nightly)] + fn provide

(self, provided: P) -> FutureWithProvided + where + P: Display + Debug + Send + Sync + 'static; + + /// Lazily adds the provided object to the [`Frame`] stack when [`poll`]ing the [`Future`]. + /// + /// The object can later be retrieved by calling [`request_value()`]. + /// + /// The function is only executed in the `Err` arm. + /// + /// [`request_ref()`]: crate::Report::request_ref + /// [`Frame`]: crate::Frame + /// [`poll`]: Future::poll + #[cfg(nightly)] + fn provide_lazy(self, provided: F) -> FutureWithLazyProvided + where + P: Display + Debug + Send + Sync + 'static, + F: FnOnce() -> P; + /// Changes the [`Context`] of a [`Report`] when [`poll`]ing the [`Future`] returning /// [`Result`]. /// @@ -342,6 +385,27 @@ where } } + fn provide

(self, provided: P) -> FutureWithProvided + where + P: Display + Debug + Send + Sync + 'static, + { + FutureWithProvided { + future: self, + inner: Some(provided), + } + } + + fn provide_lazy(self, provided: F) -> FutureWithLazyProvided + where + P: Display + Debug + Send + Sync + 'static, + F: FnOnce() -> P, + { + FutureWithLazyProvided { + future: self, + inner: Some(provided), + } + } + #[track_caller] fn change_context(self, context: C) -> FutureWithContext where diff --git a/packages/engine/lib/error/src/lib.rs b/packages/engine/lib/error/src/lib.rs index 80aab87e890..04bda5b52d6 100644 --- a/packages/engine/lib/error/src/lib.rs +++ b/packages/engine/lib/error/src/lib.rs @@ -200,6 +200,8 @@ pub mod iter; mod macros; mod report; mod result; +#[cfg(nightly)] +mod single_provider; #[cfg(feature = "std")] mod error; diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index 2583ca5d256..3e7a940d0c2 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -258,6 +258,53 @@ impl Report { ) } + /// Adds the provided object to the [`Frame`] stack. + /// + /// The object can later be retrieved by calling [`request_value()`]. + /// + /// [`request_ref()`]: Self::request_ref + /// [`Frame`]: crate::Frame + /// + /// ## Example + /// + /// ```rust + /// # #![cfg_attr(not(feature = "std"), allow(unused_imports))] + /// use std::{fmt, fs}; + /// + /// use error::{IntoReport, ResultExt}; + /// + /// #[derive(Debug)] + /// pub struct Suggestion(&'static str); + /// + /// impl fmt::Display for Suggestion { + /// fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + /// fmt.write_str(self.0) + /// } + /// } + /// + /// # #[derive(Debug)] struct NoStdError; + /// # impl core::fmt::Display for NoStdError { fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> fmt::Result { Ok(()) }} + /// # impl error::Context for NoStdError {} + /// # #[cfg(not(feature = "std"))] + /// # let error: Result<(), _> = Err(error::report!(NoStdError).provide(Suggestion("Better use a file which exists next time!"))); + /// # #[cfg(feature = "std")] + /// let error = fs::read_to_string("config.txt") + /// .report() + /// .provide(Suggestion("Better use a file which exists next time!")); + /// let report = error.unwrap_err(); + /// let suggestion = report.request_ref::().next().unwrap(); + /// + /// assert_eq!(suggestion.0, "Better use a file which exists next time!"); + /// ``` + #[track_caller] + #[cfg(nightly)] + pub fn provide

(self, provided: P) -> Self + where + P: fmt::Debug + fmt::Display + Send + Sync + 'static, + { + self.attach_provider(crate::single_provider::SingleProvider(provided)) + } + /// Adds context information to the [`Frame`] stack enforcing a typed `Report`. /// /// Please see the [`Context`] documentation for more information. diff --git a/packages/engine/lib/error/src/result.rs b/packages/engine/lib/error/src/result.rs index 1c87438a515..58433f22d8d 100644 --- a/packages/engine/lib/error/src/result.rs +++ b/packages/engine/lib/error/src/result.rs @@ -137,6 +137,35 @@ pub trait ResultExt { P: Provider + fmt::Display + fmt::Debug + Send + Sync + 'static, F: FnOnce() -> P; + /// Adds the provided object to the [`Frame`] stack. + /// + /// The object can later be retrieved by calling [`request_value()`]. + /// + /// The function is only executed in the `Err` arm. + /// + /// [`request_ref()`]: crate::Report::request_ref + /// [`Frame`]: crate::Frame + #[cfg(nightly)] + #[must_use] + fn provide

(self, provided: P) -> Self + where + P: fmt::Display + fmt::Debug + Send + Sync + 'static; + + /// Lazily adds the provided object to the [`Frame`] stack. + /// + /// The object can later be retrieved by calling [`request_value()`]. + /// + /// The function is only executed in the `Err` arm. + /// + /// [`request_ref()`]: crate::Report::request_ref + /// [`Frame`]: crate::Frame + #[cfg(nightly)] + #[must_use] + fn provide_lazy(self, provided: F) -> Self + where + P: fmt::Display + fmt::Debug + Send + Sync + 'static, + F: FnOnce() -> P; + /// Changes the [`Context`] of a [`Report`] returning [`Result`]. /// /// Please see the [`Context`] documentation for more information. @@ -195,6 +224,33 @@ impl ResultExt for Result { } } + #[cfg(nightly)] + #[track_caller] + fn provide

(self, provided: P) -> Self + where + P: fmt::Display + fmt::Debug + Send + Sync + 'static, + { + // Can't use `map_err` as `#[track_caller]` is unstable on closures + match self { + Ok(ok) => Ok(ok), + Err(report) => Err(report.provide(provided)), + } + } + + #[cfg(nightly)] + #[track_caller] + fn provide_lazy(self, provided: F) -> Self + where + P: fmt::Display + fmt::Debug + Send + Sync + 'static, + F: FnOnce() -> P, + { + // Can't use `map_err` as `#[track_caller]` is unstable on closures + match self { + Ok(ok) => Ok(ok), + Err(report) => Err(report.provide(provided())), + } + } + #[cfg(nightly)] #[track_caller] fn attach_provider

(self, provider: P) -> Self diff --git a/packages/engine/lib/error/src/single_provider.rs b/packages/engine/lib/error/src/single_provider.rs new file mode 100644 index 00000000000..093294301fd --- /dev/null +++ b/packages/engine/lib/error/src/single_provider.rs @@ -0,0 +1,24 @@ +use core::fmt; + +use crate::provider::{Demand, Provider}; + +/// Wrapper-structure to provide `T`. +pub struct SingleProvider(pub T); + +impl fmt::Debug for SingleProvider { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&self.0, fmt) + } +} + +impl fmt::Display for SingleProvider { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0, fmt) + } +} + +impl Provider for SingleProvider { + fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + demand.provide_ref(&self.0); + } +} From 1559d7435ab8c53666f79508654885f6d76494b9 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 14:50:23 +0200 Subject: [PATCH 02/26] Fix intra-doc links --- packages/engine/lib/error/src/future.rs | 4 ++-- packages/engine/lib/error/src/report.rs | 2 +- packages/engine/lib/error/src/result.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/engine/lib/error/src/future.rs b/packages/engine/lib/error/src/future.rs index 312a477f2ea..6ff51c96e94 100644 --- a/packages/engine/lib/error/src/future.rs +++ b/packages/engine/lib/error/src/future.rs @@ -276,7 +276,7 @@ pub trait FutureExt: Future + Sized { /// Adds the provided object to the [`Frame`] stack when [`poll`]ing the [`Future`]. /// - /// The object can later be retrieved by calling [`request_value()`]. + /// The object can later be retrieved by calling [`request_ref()`]. /// /// The function is only executed in the `Err` arm. /// @@ -290,7 +290,7 @@ pub trait FutureExt: Future + Sized { /// Lazily adds the provided object to the [`Frame`] stack when [`poll`]ing the [`Future`]. /// - /// The object can later be retrieved by calling [`request_value()`]. + /// The object can later be retrieved by calling [`request_ref()`]. /// /// The function is only executed in the `Err` arm. /// diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index 3e7a940d0c2..ee806b0f267 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -260,7 +260,7 @@ impl Report { /// Adds the provided object to the [`Frame`] stack. /// - /// The object can later be retrieved by calling [`request_value()`]. + /// The object can later be retrieved by calling [`request_ref()`]. /// /// [`request_ref()`]: Self::request_ref /// [`Frame`]: crate::Frame diff --git a/packages/engine/lib/error/src/result.rs b/packages/engine/lib/error/src/result.rs index 58433f22d8d..4c0d16198d2 100644 --- a/packages/engine/lib/error/src/result.rs +++ b/packages/engine/lib/error/src/result.rs @@ -139,7 +139,7 @@ pub trait ResultExt { /// Adds the provided object to the [`Frame`] stack. /// - /// The object can later be retrieved by calling [`request_value()`]. + /// The object can later be retrieved by calling [`request_ref()`]. /// /// The function is only executed in the `Err` arm. /// @@ -153,7 +153,7 @@ pub trait ResultExt { /// Lazily adds the provided object to the [`Frame`] stack. /// - /// The object can later be retrieved by calling [`request_value()`]. + /// The object can later be retrieved by calling [`request_ref()`]. /// /// The function is only executed in the `Err` arm. /// From ad30fe49868c54cbcd99c0ccebc525bf31510a62 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 14:51:41 +0200 Subject: [PATCH 03/26] Fix miri --- packages/engine/lib/error/src/report.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index ee806b0f267..1042823bb63 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -285,9 +285,9 @@ impl Report { /// # #[derive(Debug)] struct NoStdError; /// # impl core::fmt::Display for NoStdError { fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> fmt::Result { Ok(()) }} /// # impl error::Context for NoStdError {} - /// # #[cfg(not(feature = "std"))] + /// # #[cfg(any(not(feature = "std"), miri))] /// # let error: Result<(), _> = Err(error::report!(NoStdError).provide(Suggestion("Better use a file which exists next time!"))); - /// # #[cfg(feature = "std")] + /// # #[cfg(all(feature = "std", not(miri)))] /// let error = fs::read_to_string("config.txt") /// .report() /// .provide(Suggestion("Better use a file which exists next time!")); From 4fb53b98979ab216b7094b1e67899bd4fe70866c Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 15:16:54 +0200 Subject: [PATCH 04/26] Rename `FutureWithProvided` to `FutureWithProvidedRef` --- packages/engine/lib/error/src/future.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/engine/lib/error/src/future.rs b/packages/engine/lib/error/src/future.rs index 6ff51c96e94..f1fc55d5bad 100644 --- a/packages/engine/lib/error/src/future.rs +++ b/packages/engine/lib/error/src/future.rs @@ -124,14 +124,14 @@ implement_lazy_future_adaptor!( ); implement_future_adaptor!( - FutureWithProvided, + FutureWithProvidedRef, provide, Display + Debug + Send + Sync + 'static, Fut::Output ); implement_lazy_future_adaptor!( - FutureWithLazyProvided, + FutureWithLazyProvidedRef, provide_lazy, Display + Debug + Send + Sync + 'static, Fut::Output @@ -284,7 +284,7 @@ pub trait FutureExt: Future + Sized { /// [`Frame`]: crate::Frame /// [`poll`]: Future::poll #[cfg(nightly)] - fn provide

(self, provided: P) -> FutureWithProvided + fn provide

(self, provided: P) -> FutureWithProvidedRef where P: Display + Debug + Send + Sync + 'static; @@ -298,7 +298,7 @@ pub trait FutureExt: Future + Sized { /// [`Frame`]: crate::Frame /// [`poll`]: Future::poll #[cfg(nightly)] - fn provide_lazy(self, provided: F) -> FutureWithLazyProvided + fn provide_lazy(self, provided: F) -> FutureWithLazyProvidedRef where P: Display + Debug + Send + Sync + 'static, F: FnOnce() -> P; @@ -385,22 +385,22 @@ where } } - fn provide

(self, provided: P) -> FutureWithProvided + fn provide

(self, provided: P) -> FutureWithProvidedRef where P: Display + Debug + Send + Sync + 'static, { - FutureWithProvided { + FutureWithProvidedRef { future: self, inner: Some(provided), } } - fn provide_lazy(self, provided: F) -> FutureWithLazyProvided + fn provide_lazy(self, provided: F) -> FutureWithLazyProvidedRef where P: Display + Debug + Send + Sync + 'static, F: FnOnce() -> P, { - FutureWithLazyProvided { + FutureWithLazyProvidedRef { future: self, inner: Some(provided), } From ff6374971f2cd981eee4a2e28045b97e23bf2e84 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 15:42:12 +0200 Subject: [PATCH 05/26] Combine `attach_message` and `provide` --- .../lib/error/examples/contextual_messages.rs | 5 +- .../engine/lib/error/examples/json_output.rs | 4 +- packages/engine/lib/error/src/frame.rs | 22 ++-- packages/engine/lib/error/src/future.rs | 110 ++++-------------- packages/engine/lib/error/src/lib.rs | 4 +- packages/engine/lib/error/src/macros.rs | 12 +- packages/engine/lib/error/src/report.rs | 98 +++++++--------- packages/engine/lib/error/src/result.rs | 82 +++---------- 8 files changed, 98 insertions(+), 239 deletions(-) diff --git a/packages/engine/lib/error/examples/contextual_messages.rs b/packages/engine/lib/error/examples/contextual_messages.rs index bf4d8156e1f..87fe40d5552 100644 --- a/packages/engine/lib/error/examples/contextual_messages.rs +++ b/packages/engine/lib/error/examples/contextual_messages.rs @@ -35,15 +35,14 @@ fn parse_config(config: &HashMap<&str, u64>) -> Result { let key = "abcd-efgh"; // `ResultExt` provides different methods for adding additional information to the `Report` - let value = - lookup_key(config, key).attach_message_lazy(|| format!("Could not lookup key {key:?}"))?; + let value = lookup_key(config, key).attach_lazy(|| format!("Could not lookup key {key:?}"))?; Ok(value) } fn main() -> Result<(), LookupError> { let config = HashMap::default(); - let _config_value = parse_config(&config).attach_message("Unable to parse config")?; + let _config_value = parse_config(&config).attach("Unable to parse config")?; Ok(()) } diff --git a/packages/engine/lib/error/examples/json_output.rs b/packages/engine/lib/error/examples/json_output.rs index 07305f40b8f..77d3d0fd2fe 100644 --- a/packages/engine/lib/error/examples/json_output.rs +++ b/packages/engine/lib/error/examples/json_output.rs @@ -62,11 +62,11 @@ fn main() -> Result<(), MapError> { let mut config = HashMap::default(); // Create an entry with "foo" as key - create_new_entry(&mut config, "foo", 1).attach_message("Could not create new entry")?; + create_new_entry(&mut config, "foo", 1).attach("Could not create new entry")?; // Purposefully cause an error by attempting to create another entry with "foo" as key let creation_result = - create_new_entry(&mut config, "foo", 2).attach_message("Could not create new entry"); + create_new_entry(&mut config, "foo", 2).attach("Could not create new entry"); assert_eq!( format!("{:?}", creation_result.unwrap_err()), diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index 201dbb4a0a8..fdf189a7367 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -93,15 +93,15 @@ impl Frame { Self::from_unerased(ContextRepr(context), location, source) } - pub(crate) fn from_message( - message: M, + pub(crate) fn from_object( + object: O, location: &'static Location<'static>, source: Option>, ) -> Self where - M: fmt::Display + fmt::Debug + Send + Sync + 'static, + O: fmt::Display + fmt::Debug + Send + Sync + 'static, { - Self::from_unerased(MessageRepr(message), location, source) + Self::from_unerased(FrameObject(object), location, source) } #[cfg(feature = "std")] @@ -215,26 +215,26 @@ struct VTable { /// /// A message does not necessarily implement [`Provider`], an empty implementation is provided. /// If a [`Provider`] is required attach it directly rather than attaching a message. -struct MessageRepr(M); +struct FrameObject(T); -impl fmt::Display for MessageRepr { +impl fmt::Display for FrameObject { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.0, fmt) } } -impl fmt::Debug for MessageRepr { +impl fmt::Debug for FrameObject { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(&self.0, fmt) } } -impl Context for MessageRepr {} +impl Context for FrameObject {} #[cfg(nightly)] -impl Provider for MessageRepr { - fn provide<'a>(&'a self, _: &mut Demand<'a>) { - // Empty definition as a contextual message does not convey provider information +impl Provider for FrameObject { + fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + demand.provide_ref(&self.0); } } diff --git a/packages/engine/lib/error/src/future.rs b/packages/engine/lib/error/src/future.rs index f1fc55d5bad..1dd53b29c3e 100644 --- a/packages/engine/lib/error/src/future.rs +++ b/packages/engine/lib/error/src/future.rs @@ -96,15 +96,15 @@ macro_rules! implement_lazy_future_adaptor { } implement_future_adaptor!( - FutureWithMessage, - attach_message, + FutureWithObject, + attach, Display + Debug + Send + Sync + 'static, Fut::Output ); implement_lazy_future_adaptor!( - FutureWithLazyMessage, - attach_message_lazy, + FutureWithLazyObject, + attach_lazy, Display + Debug + Send + Sync + 'static, Fut::Output ); @@ -123,20 +123,6 @@ implement_lazy_future_adaptor!( Fut::Output ); -implement_future_adaptor!( - FutureWithProvidedRef, - provide, - Display + Debug + Send + Sync + 'static, - Fut::Output -); - -implement_lazy_future_adaptor!( - FutureWithLazyProvidedRef, - provide_lazy, - Display + Debug + Send + Sync + 'static, - Fut::Output -); - implement_future_adaptor!( FutureWithContext, change_context, @@ -155,7 +141,7 @@ implement_lazy_future_adaptor!( /// /// [`Report`]: crate::Report pub trait FutureExt: Future + Sized { - /// Adds new contextual message to the [`Frame`] stack of a [`Report`] when [`poll`]ing the + /// Adds new contextual information to the [`Frame`] stack of a [`Report`] when [`poll`]ing the /// [`Future`]. /// /// [`Frame`]: crate::Frame @@ -184,19 +170,19 @@ pub trait FutureExt: Future + Sized { /// # let user = User; /// # let resource = Resource; /// // A contextual message can be provided before polling the `Future` - /// load_resource(&user, &resource).attach_message("Could not load resource").await + /// load_resource(&user, &resource).attach("Could not load resource").await /// # }; /// # #[cfg(not(miri))] /// # assert_eq!(futures::executor::block_on(fut).unwrap_err().frames().count(), 2); /// # Result::<_, ResourceError>::Ok(()) /// ``` #[track_caller] - fn attach_message(self, message: M) -> FutureWithMessage + fn attach(self, object: O) -> FutureWithObject where - M: Display + Debug + Send + Sync + 'static; + O: Display + Debug + Send + Sync + 'static; - /// Lazily adds new contextual message to the [`Frame`] stack of a [`Report`] when [`poll`]ing - /// the [`Future`]. + /// Lazily adds new contextual information to the [`Frame`] stack of a [`Report`] when + /// [`poll`]ing the [`Future`]. /// /// The function is only executed in the `Err` arm. /// @@ -227,17 +213,17 @@ pub trait FutureExt: Future + Sized { /// # let user = User; /// # let resource = Resource; /// // A contextual message can be provided before polling the `Future` - /// load_resource(&user, &resource).attach_message_lazy(|| format!("Could not load resource {resource}")).await + /// load_resource(&user, &resource).attach_lazy(|| format!("Could not load resource {resource}")).await /// # }; /// # #[cfg(not(miri))] /// # assert_eq!(futures::executor::block_on(fut).unwrap_err().frames().count(), 2); /// # Result::<_, ResourceError>::Ok(()) /// ``` #[track_caller] - fn attach_message_lazy(self, message: F) -> FutureWithLazyMessage + fn attach_lazy(self, object: F) -> FutureWithLazyObject where - M: Display + Debug + Send + Sync + 'static, - F: FnOnce() -> M; + O: Display + Debug + Send + Sync + 'static, + F: FnOnce() -> O; /// Adds a [`Provider`] to the [`Frame`] stack when [`poll`]ing the [`Future`]. /// @@ -274,35 +260,6 @@ pub trait FutureExt: Future + Sized { P: Provider + Display + Debug + Send + Sync + 'static, F: FnOnce() -> P; - /// Adds the provided object to the [`Frame`] stack when [`poll`]ing the [`Future`]. - /// - /// The object can later be retrieved by calling [`request_ref()`]. - /// - /// The function is only executed in the `Err` arm. - /// - /// [`request_ref()`]: crate::Report::request_ref - /// [`Frame`]: crate::Frame - /// [`poll`]: Future::poll - #[cfg(nightly)] - fn provide

(self, provided: P) -> FutureWithProvidedRef - where - P: Display + Debug + Send + Sync + 'static; - - /// Lazily adds the provided object to the [`Frame`] stack when [`poll`]ing the [`Future`]. - /// - /// The object can later be retrieved by calling [`request_ref()`]. - /// - /// The function is only executed in the `Err` arm. - /// - /// [`request_ref()`]: crate::Report::request_ref - /// [`Frame`]: crate::Frame - /// [`poll`]: Future::poll - #[cfg(nightly)] - fn provide_lazy(self, provided: F) -> FutureWithLazyProvidedRef - where - P: Display + Debug + Send + Sync + 'static, - F: FnOnce() -> P; - /// Changes the [`Context`] of a [`Report`] when [`poll`]ing the [`Future`] returning /// [`Result`]. /// @@ -338,25 +295,25 @@ impl FutureExt for Fut where Fut::Output: ResultExt, { - fn attach_message(self, message: M) -> FutureWithMessage + fn attach(self, object: O) -> FutureWithObject where - M: Display + Debug + Send + Sync + 'static, + O: Display + Debug + Send + Sync + 'static, { - FutureWithMessage { + FutureWithObject { future: self, - inner: Some(message), + inner: Some(object), } } #[track_caller] - fn attach_message_lazy(self, message: F) -> FutureWithLazyMessage + fn attach_lazy(self, object: F) -> FutureWithLazyObject where - M: Display + Debug + Send + Sync + 'static, - F: FnOnce() -> M, + O: Display + Debug + Send + Sync + 'static, + F: FnOnce() -> O, { - FutureWithLazyMessage { + FutureWithLazyObject { future: self, - inner: Some(message), + inner: Some(object), } } @@ -385,27 +342,6 @@ where } } - fn provide

(self, provided: P) -> FutureWithProvidedRef - where - P: Display + Debug + Send + Sync + 'static, - { - FutureWithProvidedRef { - future: self, - inner: Some(provided), - } - } - - fn provide_lazy(self, provided: F) -> FutureWithLazyProvidedRef - where - P: Display + Debug + Send + Sync + 'static, - F: FnOnce() -> P, - { - FutureWithLazyProvidedRef { - future: self, - inner: Some(provided), - } - } - #[track_caller] fn change_context(self, context: C) -> FutureWithContext where diff --git a/packages/engine/lib/error/src/lib.rs b/packages/engine/lib/error/src/lib.rs index 04bda5b52d6..4abcdfa6660 100644 --- a/packages/engine/lib/error/src/lib.rs +++ b/packages/engine/lib/error/src/lib.rs @@ -115,7 +115,7 @@ //! //! // `ResultExt` provides different methods for adding additional information to the `Report` //! let value = -//! lookup_key(config, key).attach_message_lazy(|| format!("Could not lookup key {key:?}"))?; +//! lookup_key(config, key).attach_lazy(|| format!("Could not lookup key {key:?}"))?; //! //! Ok(value) //! } @@ -124,7 +124,7 @@ //! # fn fake_main() -> Result<(), LookupError> { // We want to assert on the result //! let config = HashMap::default(); //! # #[allow(unused_variables)] -//! let config_value = parse_config(&config).attach_message("Unable to parse config")?; +//! let config_value = parse_config(&config).attach("Unable to parse config")?; //! //! # const _: &str = stringify! { //! ... diff --git a/packages/engine/lib/error/src/macros.rs b/packages/engine/lib/error/src/macros.rs index 3909dfd1ba0..7f440e70e33 100644 --- a/packages/engine/lib/error/src/macros.rs +++ b/packages/engine/lib/error/src/macros.rs @@ -290,7 +290,7 @@ mod tests { #[test] fn report() { let err = capture_error(|| Err(report!(ContextA))); - let err = err.attach_message("additional message"); + let err = err.attach("additional message"); assert!(err.contains::()); assert_eq!(err.frames().count(), 2); assert_eq!(messages(&err), ["additional message", "Context A"]); @@ -305,7 +305,7 @@ mod tests { #[cfg(feature = "std")] { let err = capture_error(|| Err(report!(ContextB))); - let err = err.attach_message("additional message"); + let err = err.attach("additional message"); assert!(err.contains::()); assert_eq!(err.frames().count(), 2); assert_eq!(messages(&err), ["additional message", "Context B"]); @@ -316,7 +316,7 @@ mod tests { #[test] fn bail() { let err = capture_error(|| bail!(ContextA)); - let err = err.attach_message("additional message"); + let err = err.attach("additional message"); assert!(err.contains::()); assert_eq!(err.frames().count(), 2); assert_eq!(messages(&err), ["additional message", "Context A"]); @@ -331,7 +331,7 @@ mod tests { #[cfg(feature = "std")] { let err = capture_error(|| bail!(ContextB)); - let err = err.attach_message("additional message"); + let err = err.attach("additional message"); assert!(err.contains::()); assert_eq!(err.frames().count(), 2); assert_eq!(messages(&err), ["additional message", "Context B"]); @@ -345,7 +345,7 @@ mod tests { ensure!(false, ContextA); Ok(()) }); - let err = err.attach_message("additional message"); + let err = err.attach("additional message"); assert!(err.contains::()); assert_eq!(err.frames().count(), 2); assert_eq!(messages(&err), ["additional message", "Context A"]); @@ -366,7 +366,7 @@ mod tests { ensure!(false, ContextB); Ok(()) }); - let err = err.attach_message("additional message"); + let err = err.attach("additional message"); assert!(err.contains::()); assert_eq!(err.frames().count(), 2); assert_eq!(messages(&err), ["additional message", "Context B"]); diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index 1042823bb63..b1179f8a020 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -21,7 +21,7 @@ use crate::{ /// the [`Backtrace` documentation][`Backtrace`]. To enable the span trace, [`ErrorLayer`] has to /// be enabled. /// -/// Context information can be added by using [`attach_message()`] or [`ResultExt`]. The [`Frame`] +/// Context information can be added by using [`attach()`] or [`ResultExt`]. The [`Frame`] /// stack can be iterated by using [`frames()`]. /// /// To enforce context information generation, a context [`Provider`] needs to be used. When @@ -33,7 +33,7 @@ use crate::{ /// [`Backtrace`]: std::backtrace::Backtrace /// [`SpanTrace`]: tracing_error::SpanTrace /// [`ErrorLayer`]: tracing_error::ErrorLayer -/// [`attach_message()`]: Self::attach_message +/// [`attach()`]: Self::attach /// [`from_error()`]: Self::from_error /// [`from_context()`]: Self::from_context /// [`frames()`]: Self::frames @@ -59,7 +59,7 @@ use crate::{ /// # #[allow(unused_variables)] /// let content = std::fs::read_to_string(config_path) /// .report() -/// .attach_message_lazy(|| format!("Failed to read config file {config_path:?}"))?; +/// .attach_lazy(|| format!("Failed to read config file {config_path:?}"))?; /// /// # const _: &str = stringify! { /// ... @@ -116,7 +116,7 @@ use crate::{ /// # #[allow(unused_variables)] /// fn read_config(path: impl AsRef) -> Result> { /// # #[cfg(any(miri, not(feature = "std")))] -/// # return Err(error::report!(ConfigError::IoError).attach_message("Not supported")); +/// # return Err(error::report!(ConfigError::IoError).attach("Not supported")); /// # #[cfg(all(not(miri), feature = "std"))] /// std::fs::read_to_string(path.as_ref()).report().change_context(ConfigError::IoError) /// } @@ -210,18 +210,45 @@ impl Report { ) } - /// Adds a contextual message to the [`Frame`] stack. + /// Adds a contextual information to the [`Frame`] stack. + /// + /// ## Example + /// + /// ```rust + /// # #![cfg_attr(not(feature = "std"), allow(unused_imports))] + /// use std::{fmt, fs}; + /// + /// use error::{IntoReport, ResultExt}; + /// + /// #[derive(Debug)] + /// pub struct Suggestion(&'static str); + /// + /// impl fmt::Display for Suggestion { + /// fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + /// fmt.write_str(self.0) + /// } + /// } + /// + /// # #[derive(Debug)] struct NoStdError; + /// # impl core::fmt::Display for NoStdError { fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> fmt::Result { Ok(()) }} + /// # impl error::Context for NoStdError {} + /// # #[cfg(any(not(feature = "std"), miri))] + /// # let error: Result<(), _> = Err(error::report!(NoStdError).attach(Suggestion("Better use a file which exists next time!"))); + /// # #[cfg(all(feature = "std", not(miri)))] + /// let error = fs::read_to_string("config.txt") + /// .report() + /// .attach(Suggestion("Better use a file which exists next time!")); + /// let report = error.unwrap_err(); + /// let suggestion = report.request_ref::().next().unwrap(); + /// + /// assert_eq!(suggestion.0, "Better use a file which exists next time!"); #[track_caller] - pub fn attach_message(self, message: M) -> Self + pub fn attach(self, object: O) -> Self where - M: fmt::Display + fmt::Debug + Send + Sync + 'static, + O: fmt::Display + fmt::Debug + Send + Sync + 'static, { Self::from_frame( - Frame::from_message( - message, - Location::caller(), - Some(Box::new(self.inner.frame)), - ), + Frame::from_object(object, Location::caller(), Some(Box::new(self.inner.frame))), #[cfg(all(nightly, feature = "std"))] self.inner.backtrace, #[cfg(feature = "spantrace")] @@ -258,53 +285,6 @@ impl Report { ) } - /// Adds the provided object to the [`Frame`] stack. - /// - /// The object can later be retrieved by calling [`request_ref()`]. - /// - /// [`request_ref()`]: Self::request_ref - /// [`Frame`]: crate::Frame - /// - /// ## Example - /// - /// ```rust - /// # #![cfg_attr(not(feature = "std"), allow(unused_imports))] - /// use std::{fmt, fs}; - /// - /// use error::{IntoReport, ResultExt}; - /// - /// #[derive(Debug)] - /// pub struct Suggestion(&'static str); - /// - /// impl fmt::Display for Suggestion { - /// fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - /// fmt.write_str(self.0) - /// } - /// } - /// - /// # #[derive(Debug)] struct NoStdError; - /// # impl core::fmt::Display for NoStdError { fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> fmt::Result { Ok(()) }} - /// # impl error::Context for NoStdError {} - /// # #[cfg(any(not(feature = "std"), miri))] - /// # let error: Result<(), _> = Err(error::report!(NoStdError).provide(Suggestion("Better use a file which exists next time!"))); - /// # #[cfg(all(feature = "std", not(miri)))] - /// let error = fs::read_to_string("config.txt") - /// .report() - /// .provide(Suggestion("Better use a file which exists next time!")); - /// let report = error.unwrap_err(); - /// let suggestion = report.request_ref::().next().unwrap(); - /// - /// assert_eq!(suggestion.0, "Better use a file which exists next time!"); - /// ``` - #[track_caller] - #[cfg(nightly)] - pub fn provide

(self, provided: P) -> Self - where - P: fmt::Debug + fmt::Display + Send + Sync + 'static, - { - self.attach_provider(crate::single_provider::SingleProvider(provided)) - } - /// Adds context information to the [`Frame`] stack enforcing a typed `Report`. /// /// Please see the [`Context`] documentation for more information. diff --git a/packages/engine/lib/error/src/result.rs b/packages/engine/lib/error/src/result.rs index 4c0d16198d2..f3410d719d0 100644 --- a/packages/engine/lib/error/src/result.rs +++ b/packages/engine/lib/error/src/result.rs @@ -48,7 +48,7 @@ pub trait ResultExt { /// Type of the [`Ok`] value in the [`Result`] type Ok; - /// Adds new contextual message to the [`Frame`] stack of a [`Report`]. + /// Adds new contextual information to the [`Frame`] stack of a [`Report`]. /// /// [`Frame`]: crate::Frame /// @@ -64,15 +64,15 @@ pub trait ResultExt { /// # let user = User; /// # let resource = Resource; /// # #[allow(unused_variables)] - /// let resource = load_resource(&user, &resource).attach_message("Could not load resource")?; + /// let resource = load_resource(&user, &resource).attach("Could not load resource")?; /// # Result::Ok(()) /// ``` #[must_use] - fn attach_message(self, message: M) -> Self + fn attach(self, object: O) -> Self where - M: fmt::Display + fmt::Debug + Send + Sync + 'static; + O: fmt::Display + fmt::Debug + Send + Sync + 'static; - /// Lazily adds new contextual message to the [`Frame`] stack of a [`Report`]. + /// Lazily adds new information message to the [`Frame`] stack of a [`Report`]. /// /// The function is only executed in the `Err` arm. /// @@ -93,14 +93,14 @@ pub trait ResultExt { /// # let resource = Resource; /// # #[allow(unused_variables)] /// let resource = load_resource(&user, &resource) - /// .attach_message_lazy(|| format!("Could not load resource {resource}"))?; + /// .attach_lazy(|| format!("Could not load resource {resource}"))?; /// # Result::Ok(()) /// ``` #[must_use] - fn attach_message_lazy(self, op: F) -> Self + fn attach_lazy(self, object: F) -> Self where - M: fmt::Display + fmt::Debug + Send + Sync + 'static, - F: FnOnce() -> M; + O: fmt::Display + fmt::Debug + Send + Sync + 'static, + F: FnOnce() -> O; /// Adds a [`Provider`] to the [`Frame`] stack. /// @@ -137,35 +137,6 @@ pub trait ResultExt { P: Provider + fmt::Display + fmt::Debug + Send + Sync + 'static, F: FnOnce() -> P; - /// Adds the provided object to the [`Frame`] stack. - /// - /// The object can later be retrieved by calling [`request_ref()`]. - /// - /// The function is only executed in the `Err` arm. - /// - /// [`request_ref()`]: crate::Report::request_ref - /// [`Frame`]: crate::Frame - #[cfg(nightly)] - #[must_use] - fn provide

(self, provided: P) -> Self - where - P: fmt::Display + fmt::Debug + Send + Sync + 'static; - - /// Lazily adds the provided object to the [`Frame`] stack. - /// - /// The object can later be retrieved by calling [`request_ref()`]. - /// - /// The function is only executed in the `Err` arm. - /// - /// [`request_ref()`]: crate::Report::request_ref - /// [`Frame`]: crate::Frame - #[cfg(nightly)] - #[must_use] - fn provide_lazy(self, provided: F) -> Self - where - P: fmt::Display + fmt::Debug + Send + Sync + 'static, - F: FnOnce() -> P; - /// Changes the [`Context`] of a [`Report`] returning [`Result`]. /// /// Please see the [`Context`] documentation for more information. @@ -200,19 +171,19 @@ impl ResultExt for Result { type Ok = T; #[track_caller] - fn attach_message(self, message: M) -> Self + fn attach(self, message: M) -> Self where M: fmt::Display + fmt::Debug + Send + Sync + 'static, { // Can't use `map_err` as `#[track_caller]` is unstable on closures match self { Ok(ok) => Ok(ok), - Err(report) => Err(report.attach_message(message)), + Err(report) => Err(report.attach(message)), } } #[track_caller] - fn attach_message_lazy(self, message: F) -> Self + fn attach_lazy(self, message: F) -> Self where M: fmt::Display + fmt::Debug + Send + Sync + 'static, F: FnOnce() -> M, @@ -220,34 +191,7 @@ impl ResultExt for Result { // Can't use `map_err` as `#[track_caller]` is unstable on closures match self { Ok(ok) => Ok(ok), - Err(report) => Err(report.attach_message(message())), - } - } - - #[cfg(nightly)] - #[track_caller] - fn provide

(self, provided: P) -> Self - where - P: fmt::Display + fmt::Debug + Send + Sync + 'static, - { - // Can't use `map_err` as `#[track_caller]` is unstable on closures - match self { - Ok(ok) => Ok(ok), - Err(report) => Err(report.provide(provided)), - } - } - - #[cfg(nightly)] - #[track_caller] - fn provide_lazy(self, provided: F) -> Self - where - P: fmt::Display + fmt::Debug + Send + Sync + 'static, - F: FnOnce() -> P, - { - // Can't use `map_err` as `#[track_caller]` is unstable on closures - match self { - Ok(ok) => Ok(ok), - Err(report) => Err(report.provide(provided())), + Err(report) => Err(report.attach(message())), } } From a9a4b1518e1cdcdc2e14d7d983e74e12377b284e Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 15:42:23 +0200 Subject: [PATCH 06/26] Apply changes to hEngine --- packages/engine/bin/cli/src/main.rs | 8 ++--- packages/engine/bin/hash_engine/src/main.rs | 8 ++--- packages/engine/lib/nano/src/client.rs | 22 +++++++------- packages/engine/lib/nano/src/server.rs | 12 ++++---- packages/engine/lib/orchestrator/src/error.rs | 2 +- .../engine/lib/orchestrator/src/experiment.rs | 28 ++++++++--------- .../lib/orchestrator/src/experiment_server.rs | 7 +++-- .../engine/lib/orchestrator/src/manifest.rs | 30 +++++++++---------- packages/engine/src/env.rs | 2 +- packages/engine/tests/experiment/mod.rs | 14 ++++----- 10 files changed, 66 insertions(+), 67 deletions(-) diff --git a/packages/engine/bin/cli/src/main.rs b/packages/engine/bin/cli/src/main.rs index 99aef7b6a2f..8bcfa88b80f 100644 --- a/packages/engine/bin/cli/src/main.rs +++ b/packages/engine/bin/cli/src/main.rs @@ -90,7 +90,7 @@ async fn main() -> Result<(), ()> { &format!("cli-{now}-texray"), ) .report() - .attach_message("Failed to initialize the logger") + .attach("Failed to initialize the logger") .generalize()?; let nng_listen_url = format!("ipc://hash-orchestrator-{now}"); @@ -102,14 +102,14 @@ async fn main() -> Result<(), ()> { .project .canonicalize() .report() - .attach_message_lazy(|| format!("Could not canonicalize project path: {:?}", args.project)) + .attach_lazy(|| format!("Could not canonicalize project path: {:?}", args.project)) .generalize()?; let manifest = Manifest::from_local(&absolute_project_path) - .attach_message_lazy(|| format!("Could not read local project {absolute_project_path:?}")) + .attach_lazy(|| format!("Could not read local project {absolute_project_path:?}")) .generalize()?; let experiment_run = manifest .read(args.r#type.into()) - .attach_message("Could not read manifest") + .attach("Could not read manifest") .generalize()?; let experiment = Experiment::new(args.experiment_config); diff --git a/packages/engine/bin/hash_engine/src/main.rs b/packages/engine/bin/hash_engine/src/main.rs index caf55851482..2073c647e7f 100644 --- a/packages/engine/bin/hash_engine/src/main.rs +++ b/packages/engine/bin/hash_engine/src/main.rs @@ -20,20 +20,20 @@ async fn main() -> Result<(), ()> { &format!("experiment-{}-texray", args.experiment_id), ) .report() - .attach_message("Failed to initialize the logger") + .attach("Failed to initialize the logger") .generalize()?; let mut env = env::(&args) .await .report() - .attach_message("Could not create environment for experiment") + .attach("Could not create environment for experiment") .generalize()?; // Fetch all dependencies of the experiment run such as datasets env.experiment .fetch_deps() .await .report() - .attach_message("Could not fetch dependencies for experiment") + .attach("Could not fetch dependencies for experiment") .generalize()?; // Generate the configuration for packages from the environment let config = experiment_config(&args, &env).await.report().generalize()?; @@ -46,7 +46,7 @@ async fn main() -> Result<(), ()> { let experiment_result = run_experiment(config, env) .await .report() - .attach_message("Could not run experiment") + .attach("Could not run experiment") .generalize(); cleanup_experiment(args.experiment_id); diff --git a/packages/engine/lib/nano/src/client.rs b/packages/engine/lib/nano/src/client.rs index c4d4ee8fe3c..429eb91b7bc 100644 --- a/packages/engine/lib/nano/src/client.rs +++ b/packages/engine/lib/nano/src/client.rs @@ -44,35 +44,35 @@ impl Worker { fn new(url: &str, request_rx: spmc::Receiver) -> Result { let socket = nng::Socket::new(nng::Protocol::Req0) .report() - .attach_message("Could not create nng socket")?; + .attach("Could not create nng socket")?; let builder = nng::DialerBuilder::new(&socket, url) .report() - .attach_message("Could not create nng dialer")?; + .attach("Could not create nng dialer")?; builder .set_opt::(Some(RECONNECT_MAX_TIME)) .report() - .attach_message_lazy(|| { + .attach_lazy(|| { format!("Could not set maximum reconnection time to {RECONNECT_MAX_TIME:?}") })?; builder .set_opt::(Some(RECONNECT_MIN_TIME)) .report() - .attach_message_lazy(|| { + .attach_lazy(|| { format!("Could not set minimum reconnection time to {RECONNECT_MIN_TIME:?}") })?; let dialer = builder .start(false) .map_err(|(_, error)| error) .report() - .attach_message("Could not start nng dialer")?; + .attach("Could not start nng dialer")?; let ctx = nng::Context::new(&socket) .report() - .attach_message("Could not create nng context")?; + .attach("Could not create nng context")?; ctx.set_opt::(Some(RESEND_TIME)) .report() - .attach_message_lazy(|| format!("Could not set resend time to {RESEND_TIME:?}"))?; + .attach_lazy(|| format!("Could not set resend time to {RESEND_TIME:?}"))?; // There will only ever be one message in the channel. But, it needs to be // unbounded because we can't .await inside an nng::Aio. @@ -102,7 +102,7 @@ impl Worker { } }) .report() - .attach_message("Could not create asynchronous I/O context")?; + .attach("Could not create asynchronous I/O context")?; Ok(Self { _socket: socket, @@ -156,7 +156,7 @@ impl Client { let workers = (0..num_workers) .map(|_| { let mut worker = Worker::new(url, receiver.clone()) - .attach_message("Could not create nng worker") + .attach("Could not create nng worker") .change_context(ErrorKind::ClientCreation)?; // let (stop_tx, stop_rx) = mpsc::channel(1); let (stop_tx, stop_rx) = mpsc::unbounded_channel(); @@ -193,7 +193,7 @@ impl Client { let mut nng_msg = nng::Message::new(); serde_json::to_writer(&mut nng_msg, msg) .report() - .attach_message("Could not serialize message") + .attach("Could not serialize message") .change_context(ErrorKind::Send)?; let (tx, rx) = oneshot::channel(); @@ -203,7 +203,7 @@ impl Client { .expect(SEND_EXPECT_MESSAGE); rx.await .expect(RECV_EXPECT_MESSAGE) - .attach_message("Could not receive response") + .attach("Could not receive response") .change_context(ErrorKind::Send) } } diff --git a/packages/engine/lib/nano/src/server.rs b/packages/engine/lib/nano/src/server.rs index d86ff7ee7d8..efbbb94e3cd 100644 --- a/packages/engine/lib/nano/src/server.rs +++ b/packages/engine/lib/nano/src/server.rs @@ -57,7 +57,7 @@ impl Worker { fn new(socket: &nng::Socket, sender: MsgSender, url: &str) -> Result { let ctx_orig = nng::Context::new(socket) .report() - .attach_message("Could not create context")?; + .attach("Could not create context")?; let ctx = ctx_orig.clone(); let socket_url = url.to_owned(); @@ -90,7 +90,7 @@ impl Worker { ctx_orig .recv(&aio) .report() - .attach_message("Could not receive message from context")?; + .attach("Could not receive message from context")?; Ok(Self { _aio: aio }) } @@ -108,12 +108,12 @@ impl Server { pub fn new(url: &str) -> Result { let socket = nng::Socket::new(nng::Protocol::Rep0) .report() - .attach_message("Could not create socket") + .attach("Could not create socket") .change_context(ErrorKind::ServerCreation)?; socket .listen(url) .report() - .attach_message("Could not listen on socket") + .attach("Could not listen on socket") .change_context(ErrorKind::ServerCreation)?; let (sender, receiver) = mpsc::unbounded_channel(); @@ -121,7 +121,7 @@ impl Server { let workers = (0..NUM_WORKERS) .map(|_| { Worker::new(&socket, sender.clone(), url) - .attach_message("Could not create worker") + .attach("Could not create worker") .change_context(ErrorKind::ServerCreation) }) .collect::>>()?; @@ -153,7 +153,7 @@ impl Server { let msg = self.receiver.recv().await.expect(RECV_EXPECT_MESSAGE); serde_json::from_slice::(msg.as_slice()) .report() - .attach_message("Could not convert message from JSON") + .attach("Could not convert message from JSON") .change_context(ErrorKind::Receive) } } diff --git a/packages/engine/lib/orchestrator/src/error.rs b/packages/engine/lib/orchestrator/src/error.rs index 3a89e1aa61b..f96dc08c50e 100644 --- a/packages/engine/lib/orchestrator/src/error.rs +++ b/packages/engine/lib/orchestrator/src/error.rs @@ -5,7 +5,7 @@ pub type Result = error::Result; // TODO: Use proper context type // Currently, we capture every error message inside of a generic error message. We want a context // object similar to the one we using in the integration test suite, e.g. -// `OrchestratorError::InvalidManifest` with additional information provided by `attach_message`. +// `OrchestratorError::InvalidManifest` with additional information provided by `attach`. #[derive(Debug)] pub enum OrchestratorError { UniqueOwned(Box), diff --git a/packages/engine/lib/orchestrator/src/experiment.rs b/packages/engine/lib/orchestrator/src/experiment.rs index b76a6d13987..07c114359cd 100644 --- a/packages/engine/lib/orchestrator/src/experiment.rs +++ b/packages/engine/lib/orchestrator/src/experiment.rs @@ -161,7 +161,7 @@ impl ExperimentType { )), ExperimentType::Simple { name } => Ok(ExperimentPackageConfig::Simple( get_simple_experiment_config(base, name) - .attach_message("Could not read simple experiment config")?, + .attach("Could not read simple experiment config")?, )), } } @@ -225,9 +225,7 @@ impl Experiment { let mut engine_handle = handler .register_experiment(experiment_run.base.id) .await - .attach_message_lazy(|| { - format!("Could not register experiment \"{experiment_name}\"") - })?; + .attach_lazy(|| format!("Could not register experiment \"{experiment_name}\""))?; // Create and start the experiment run let cmd = self.create_engine_command( @@ -237,7 +235,7 @@ impl Experiment { self.config.js_runner_initial_heap_constraint, self.config.js_runner_max_heap_size, ); - let mut engine_process = cmd.run().await.attach_message("Could not run experiment")?; + let mut engine_process = cmd.run().await.attach("Could not run experiment")?; // Wait to receive a message that the experiment has started before sending the init // message. @@ -261,7 +259,7 @@ impl Experiment { engine_process .exit_and_cleanup(experiment_run.base.id) .await - .attach_message("Failed to cleanup after failed start")?; + .attach("Failed to cleanup after failed start")?; bail!(e.change_context(OrchestratorError::from(format!( "Engine start timeout for experiment \"{experiment_name}\"" )))); @@ -284,7 +282,7 @@ impl Experiment { if let Err(err) = engine_process .send(&proto::EngineMsg::Init(init_message)) .await - .attach_message("Could not send `Init` message") + .attach("Could not send `Init` message") { // TODO: Wait for threads to finish before starting a forced cleanup warn!("Engine didn't exit gracefully, waiting for subprocesses to finish."); @@ -293,7 +291,7 @@ impl Experiment { if let Err(cleanup_err) = engine_process .exit_and_cleanup(experiment_run.base.id) .await - .attach_message("Failed to cleanup after failed start") + .attach("Failed to cleanup after failed start") { warn!("{cleanup_err}"); } @@ -436,7 +434,7 @@ impl Experiment { engine_process .exit_and_cleanup(experiment_run.base.id) .await - .attach_message("Could not cleanup after finish")?; + .attach("Could not cleanup after finish")?; ensure!( graceful_finish, @@ -464,7 +462,7 @@ fn get_simple_experiment_config( "Could not parse experiment manifest", ))?; let plan = create_experiment_plan(&parsed, &experiment_name) - .attach_message("Could not read experiment plan")?; + .attach("Could not read experiment plan")?; let max_sims_in_parallel = parsed .get("max_sims_in_parallel") @@ -524,7 +522,7 @@ fn create_experiment_plan( "Not implemented for optimization experiment types" )), _ => create_basic_variant(selected_experiment, experiment_type) - .attach_message("Could not parse basic variant"), + .attach("Could not parse basic variant"), } } @@ -556,11 +554,11 @@ fn create_multiparameter_variant( "Experiment plan does not define the specified experiment: {run_name}" )) }) - .attach_message("Could not parse experiment file")?; - create_basic_variant(selected, run_name).attach_message("Could not parse basic variant") + .attach("Could not parse experiment file")?; + create_basic_variant(selected, run_name).attach("Could not parse basic variant") }) .collect::>>() - .attach_message("Unable to create sub plans")?; + .attach("Unable to create sub plans")?; let mut variant_list: Vec = vec![]; for (i, subplan) in subplans.into_iter().enumerate() { @@ -605,7 +603,7 @@ fn create_group_variant( SimpleExperimentPlan::new(var.steps as usize), |mut acc, name| { let variants = create_experiment_plan(experiments, name) - .attach_message("Could not read experiment plan")?; + .attach("Could not read experiment plan")?; variants.inner.into_iter().for_each(|v| { acc.push(v); }); diff --git a/packages/engine/lib/orchestrator/src/experiment_server.rs b/packages/engine/lib/orchestrator/src/experiment_server.rs index 11ea3d6a2b1..2a555900606 100644 --- a/packages/engine/lib/orchestrator/src/experiment_server.rs +++ b/packages/engine/lib/orchestrator/src/experiment_server.rs @@ -222,9 +222,10 @@ impl Server { // completes before sending de-registering the experiment. Ok(()) } - Some(sender) => sender.send(msg.body).report().attach_message_lazy(|| { - format!("Routing message for experiment {}", msg.experiment_id) - }), + Some(sender) => sender + .send(msg.body) + .report() + .attach_lazy(|| format!("Routing message for experiment {}", msg.experiment_id)), } } diff --git a/packages/engine/lib/orchestrator/src/manifest.rs b/packages/engine/lib/orchestrator/src/manifest.rs index f0923e865f0..b16f36420f7 100644 --- a/packages/engine/lib/orchestrator/src/manifest.rs +++ b/packages/engine/lib/orchestrator/src/manifest.rs @@ -188,7 +188,7 @@ impl Manifest { // TODO: How to handle versions for dependency_name in self.dependencies.keys() { match get_dependency_type_from_name(dependency_name) - .attach_message_lazy(|| format!("Could not read dependency: {dependency_name}"))? + .attach_lazy(|| format!("Could not read dependency: {dependency_name}"))? { DependencyType::Behavior(extension) => { let behavior = if &extension == ".rs" { @@ -201,7 +201,7 @@ impl Manifest { } } else { get_behavior_from_dependency_projects(dependency_name, &dependency_projects) - .attach_message_lazy(|| { + .attach_lazy(|| { format!("Could not get behavior from dependency: {dependency_name}") })? }; @@ -270,10 +270,10 @@ impl Manifest { id: file_name.clone(), name: file_name, shortnames: vec![], // if this is a dependency, then these will be updated later - behavior_src: file_contents_opt(&path).attach_message("Could not read behavior")?, + behavior_src: file_contents_opt(&path).attach("Could not read behavior")?, // this may not return anything if file doesn't exist behavior_keys_src: file_contents_opt(&key_path) - .attach_message("Could not read behavior keys")?, + .attach("Could not read behavior keys")?, }); Ok(()) } @@ -304,7 +304,7 @@ impl Manifest { // Filter for `.json` files for behavior keys if file_extension(&path)? != "json" { self.add_behavior_from_file(path) - .attach_message("Could not add behavior")?; + .attach("Could not add behavior")?; } } Err(err) => { @@ -339,7 +339,7 @@ impl Manifest { OrchestratorError::from(format!("Not a valid dataset extension: {path:?}")) ); - let mut data = file_contents(path).attach_message("Could not read dataset")?; + let mut data = file_contents(path).attach("Could not read dataset")?; if file_extension == "csv" { data = parse_raw_csv_into_json(data) @@ -383,7 +383,7 @@ impl Manifest { match entry { Ok(entry) => { self.add_dataset_from_file(entry.path()) - .attach_message("Could not add dataset")?; + .attach("Could not add dataset")?; } Err(err) => { warn!("Could not ready directory entry: {err}"); @@ -459,39 +459,39 @@ impl Manifest { project .set_initial_state_from_directory(src_folder) - .attach_message("Could not read initial state")?; + .attach("Could not read initial state")?; if globals_json.exists() { project .set_globals_from_file(globals_json) - .attach_message("Could not read globals")?; + .attach("Could not read globals")?; } if analysis_json.exists() { project .set_analysis_from_file(analysis_json) - .attach_message("Could not read analysis view")?; + .attach("Could not read analysis view")?; } if experiments_json.exists() { project .set_experiments_from_file(experiments_json) - .attach_message("Could not read experiments")?; + .attach("Could not read experiments")?; } } if dependencies_json.exists() { project .set_dependencies_from_file(dependencies_json) - .attach_message("Could not read experiments")?; + .attach("Could not read experiments")?; } if behaviors_folder.exists() { project .add_behaviors_from_directory(behaviors_folder) - .attach_message("Could not read local behaviors")?; + .attach("Could not read local behaviors")?; } if data_folder.exists() { project .add_datasets_from_directory(data_folder) - .attach_message("Could not read local datasets")?; + .attach("Could not read local datasets")?; } let behaviors_deps_folders = local_dependencies_folders(dependencies_folder); @@ -502,7 +502,7 @@ impl Manifest { Err(err) => Err(err), }) .collect::>>() - .attach_message("Could not read dependencies")?; + .attach("Could not read dependencies")?; project.add_dependency_projects(dep_projects)?; Ok(project) diff --git a/packages/engine/src/env.rs b/packages/engine/src/env.rs index b08e8d1070e..aaf8efd17d5 100644 --- a/packages/engine/src/env.rs +++ b/packages/engine/src/env.rs @@ -57,7 +57,7 @@ pub struct OrchClient { impl OrchClient { pub fn new(url: &str, experiment_id: ExperimentId) -> Result { let client = nano::Client::new(url, 1) - .attach_message_lazy(|| format!("Could not create orchestrator client for {url:?}"))?; + .attach_lazy(|| format!("Could not create orchestrator client for {url:?}"))?; Ok(OrchClient { url: url.into(), experiment_id, diff --git a/packages/engine/tests/experiment/mod.rs b/packages/engine/tests/experiment/mod.rs index 25c8ba19c5c..16d9b5bdacd 100644 --- a/packages/engine/tests/experiment/mod.rs +++ b/packages/engine/tests/experiment/mod.rs @@ -46,7 +46,7 @@ fn load_manifest>(project_path: P, language: Option) -> // We read the behaviors and datasets like loading a dependency let mut manifest = Manifest::from_dependency(project_path) - .attach_message_lazy(|| format!("Could not load manifest from {project_path:?}")) + .attach_lazy(|| format!("Could not load manifest from {project_path:?}")) .change_context(TestContext::ExperimentSetup)?; // Now load globals and experiments as specified in the documentation of `Manifest` @@ -328,10 +328,10 @@ pub async fn run_test>( tokio::spawn(async move { experiment_server.run().await }); let manifest = load_manifest(project_path, language) - .attach_message_lazy(|| format!("Could not read project {project_path:?}"))?; + .attach_lazy(|| format!("Could not read project {project_path:?}"))?; let experiment_run = manifest .read(experiment_type) - .attach_message("Could not read manifest") + .attach("Could not read manifest") .change_context(TestContext::ExperimentSetup)?; let experiment = orchestrator::Experiment::new(experiment_config); @@ -355,11 +355,11 @@ pub async fn run_test>( .take_while(|output_dir| output_dir.exists()) .map(|output_dir| { let json_state = parse_file(Path::new(&output_dir).join("json_state.json")) - .attach_message("Could not read JSON state")?; + .attach("Could not read JSON state")?; let globals = parse_file(Path::new(&output_dir).join("globals.json")) - .attach_message("Could not read globals")?; + .attach("Could not read globals")?; let analysis_outputs = parse_file(Path::new(&output_dir).join("analysis_outputs.json")) - .attach_message("Could not read analysis outputs`")?; + .attach("Could not read analysis outputs`")?; Ok((json_state, globals, analysis_outputs)) }) @@ -397,7 +397,7 @@ pub fn read_config>(path: P) -> Result, P>(path) - .attach_message("Could not read integration test configuration") + .attach("Could not read integration test configuration") .change_context(TestContext::TestSetup)? .into_iter() .map(|config_value| match config_value { From 55a8db8ce370a2e0fb3e317eded604e067aef7f7 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 15:50:47 +0200 Subject: [PATCH 07/26] Test the new feature --- packages/engine/lib/error/src/lib.rs | 2 -- packages/engine/lib/error/src/macros.rs | 12 ++++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/engine/lib/error/src/lib.rs b/packages/engine/lib/error/src/lib.rs index 4abcdfa6660..7ca15044d58 100644 --- a/packages/engine/lib/error/src/lib.rs +++ b/packages/engine/lib/error/src/lib.rs @@ -312,10 +312,8 @@ pub(crate) mod test_helper { impl Context for ContextA {} #[derive(Debug)] - #[cfg(feature = "std")] pub struct ContextB; - #[cfg(feature = "std")] impl fmt::Display for ContextB { fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { fmt.write_str("Context B") diff --git a/packages/engine/lib/error/src/macros.rs b/packages/engine/lib/error/src/macros.rs index 7f440e70e33..e0ff11af7b1 100644 --- a/packages/engine/lib/error/src/macros.rs +++ b/packages/engine/lib/error/src/macros.rs @@ -297,9 +297,17 @@ mod tests { // TODO: check `err.frames().next().kind() == FrameKind::Context` let err = capture_error(|| Err(report!(err))); + let err = err.attach(ContextB); assert!(err.contains::()); - assert_eq!(err.frames().count(), 2); - assert_eq!(messages(&err), ["additional message", "Context A"]); + assert!(err.contains::()); + #[cfg(nightly)] + assert!(err.request_ref::().next().is_some()); + assert_eq!(err.frames().count(), 3); + assert_eq!(messages(&err), [ + "Context B", + "additional message", + "Context A" + ]); // TODO: check `err.frames().next().kind() == FrameKind::Context` #[cfg(feature = "std")] From 610c0cf59f91a822ccbae142ac33ce6e9e3992f4 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 15:51:05 +0200 Subject: [PATCH 08/26] Improve documentation on `FrameObject` --- packages/engine/lib/error/src/frame.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index fdf189a7367..b99905f4c44 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -211,10 +211,11 @@ struct VTable { object_downcast: unsafe fn(&FrameRepr, target: TypeId) -> Option>, } -/// Wrapper around a contextual message. +/// Wrapper around a contextual information living in a [`Frame`]. /// -/// A message does not necessarily implement [`Provider`], an empty implementation is provided. -/// If a [`Provider`] is required attach it directly rather than attaching a message. +/// An information can be requested by calling [`Report::request_ref`]. It's used for the +/// [`Display`] and [`Debug`] implementation for a [`Frame`]. If the information is a [`Provider`], +/// use [`Report::attach_provider`] instead. struct FrameObject(T); impl fmt::Display for FrameObject { From 4d1813903d7b818b462cb9b2cb0f5d55897591d8 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 15:51:15 +0200 Subject: [PATCH 09/26] Fix downcasting --- packages/engine/lib/error/src/frame.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index b99905f4c44..52d3713f32d 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -412,6 +412,11 @@ impl FrameRepr { return Some(addr.cast()); } + if let Some(addr) = (self.vtable.object_downcast)(self, TypeId::of::>()) + { + return Some(addr.cast()); + } + #[cfg(feature = "std")] if let Some(addr) = (self.vtable.object_downcast)(self, TypeId::of::>()) { return Some(addr.cast()); From b72ba37db543eaa06b7c89eedf63503b6e4f3074 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 15:52:37 +0200 Subject: [PATCH 10/26] Remove `SingleProvider` --- packages/engine/lib/error/src/lib.rs | 2 -- .../engine/lib/error/src/single_provider.rs | 24 ------------------- 2 files changed, 26 deletions(-) delete mode 100644 packages/engine/lib/error/src/single_provider.rs diff --git a/packages/engine/lib/error/src/lib.rs b/packages/engine/lib/error/src/lib.rs index 7ca15044d58..3346e295e0d 100644 --- a/packages/engine/lib/error/src/lib.rs +++ b/packages/engine/lib/error/src/lib.rs @@ -200,8 +200,6 @@ pub mod iter; mod macros; mod report; mod result; -#[cfg(nightly)] -mod single_provider; #[cfg(feature = "std")] mod error; diff --git a/packages/engine/lib/error/src/single_provider.rs b/packages/engine/lib/error/src/single_provider.rs deleted file mode 100644 index 093294301fd..00000000000 --- a/packages/engine/lib/error/src/single_provider.rs +++ /dev/null @@ -1,24 +0,0 @@ -use core::fmt; - -use crate::provider::{Demand, Provider}; - -/// Wrapper-structure to provide `T`. -pub struct SingleProvider(pub T); - -impl fmt::Debug for SingleProvider { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(&self.0, fmt) - } -} - -impl fmt::Display for SingleProvider { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.0, fmt) - } -} - -impl Provider for SingleProvider { - fn provide<'a>(&'a self, demand: &mut Demand<'a>) { - demand.provide_ref(&self.0); - } -} From 8bdd0643ab0361cbce4faa023db674dd942a14f7 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 17:15:23 +0200 Subject: [PATCH 11/26] Rework attaching and context --- packages/engine/lib/error/src/context.rs | 109 +++++++++++++++++++++++ packages/engine/lib/error/src/error.rs | 15 ---- packages/engine/lib/error/src/frame.rs | 77 +--------------- packages/engine/lib/error/src/future.rs | 76 ---------------- packages/engine/lib/error/src/hook.rs | 4 +- packages/engine/lib/error/src/lib.rs | 70 +-------------- packages/engine/lib/error/src/report.rs | 82 +++++------------ packages/engine/lib/error/src/result.rs | 64 ------------- 8 files changed, 139 insertions(+), 358 deletions(-) create mode 100644 packages/engine/lib/error/src/context.rs delete mode 100644 packages/engine/lib/error/src/error.rs diff --git a/packages/engine/lib/error/src/context.rs b/packages/engine/lib/error/src/context.rs new file mode 100644 index 00000000000..8dcb496321a --- /dev/null +++ b/packages/engine/lib/error/src/context.rs @@ -0,0 +1,109 @@ +use core::fmt; + +#[cfg(nightly)] +use crate::provider::{Demand, Provider}; +use crate::Report; + +/// Defines the current context of a [`Report`]. +/// +/// When in a `std` environment, every [`Error`] is a valid `Context`. This trait is not limited to +/// [`Error`]s and can also be manually implemented for custom objects. +/// +/// [`Error`]: std::error::Error +/// +/// ## Example +/// +/// Used for creating a [`Report`] or for switching the [`Report`]'s context: +/// +/// ```rust +/// # #![cfg_attr(any(not(feature = "std"), miri), allow(unused_imports))] +/// use std::{fmt, fs, io}; +/// +/// use error::{Context, IntoReport, Result, ResultExt}; +/// +/// # type Config = (); +/// #[derive(Debug)] +/// pub enum ConfigError { +/// ParseError, +/// } +/// +/// impl fmt::Display for ConfigError { +/// # #[allow(unused_variables)] +/// fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { +/// # const _: &str = stringify! { +/// ... +/// # }; Ok(()) +/// } +/// } +/// +/// // In this scenario, `Error` is not implemented for `ConfigError` for some reason, so implement +/// // `Context` manually. +/// impl Context for ConfigError {} +/// +/// # #[cfg(any(not(feature = "std"), miri))] +/// # pub fn read_file(_: &str) -> Result { error::bail!(ConfigError::ParseError) } +/// # #[cfg(all(feature = "std", not(miri)))] +/// pub fn read_file(path: &str) -> Result { +/// // Creates a `Report` from `io::Error`, the current context is `io::Error` +/// fs::read_to_string(path).report() +/// } +/// +/// pub fn parse_config(path: &str) -> Result { +/// // The return type of `parse_config` requires another context. By calling `change_context` +/// // the context may be changed. +/// read_file(path).change_context(ConfigError::ParseError)?; +/// +/// # const _: &str = stringify! { +/// ... +/// # }; Ok(()) +/// } +/// # let err = parse_config("invalid-path").unwrap_err(); +/// # #[cfg(all(feature = "std", not(miri)))] +/// # assert!(err.contains::()); +/// # assert!(err.contains::()); +/// # assert_eq!(err.frames().count(), 2); +/// ``` +pub trait Context: fmt::Display + fmt::Debug + Send + Sync + 'static { + /// Provide values which can then be requested by [`Report`]. + /// + /// [`Report`]: crate::Report + #[cfg(nightly)] + #[allow(unused_variables)] + fn provide<'a>(&'a self, demand: &mut Demand<'a>) {} +} + +impl From for Report +where + C: Context, +{ + #[track_caller] + #[inline] + fn from(error: C) -> Self { + Self::from_context(error) + } +} + +/// Turns a [`Context`] into a temporary [`Provider`]. +// We can't implement `Provider` on Context as `Error` will implement `Provider` and `Context` will +// be implemented on `Error`. For `request`ing a type from `Context`, we need a `Provider` +// implementation however. +#[cfg(nightly)] +pub fn temporary_provider(context: &impl Context) -> impl Provider + '_ { + struct ProviderImpl<'a, C>(&'a C); + impl Provider for ProviderImpl<'_, C> { + fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + self.0.provide(demand); + } + } + ProviderImpl(context) +} + +#[cfg(feature = "std")] +impl Context for C { + #[cfg(nightly)] + fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + if let Some(backtrace) = self.backtrace() { + demand.provide_ref(backtrace); + } + } +} diff --git a/packages/engine/lib/error/src/error.rs b/packages/engine/lib/error/src/error.rs deleted file mode 100644 index 098f4a86723..00000000000 --- a/packages/engine/lib/error/src/error.rs +++ /dev/null @@ -1,15 +0,0 @@ -use std::error::Error; - -use crate::Report; - -#[cfg(feature = "std")] -impl From for Report -where - E: Error + Send + Sync + 'static, -{ - #[track_caller] - #[inline] - fn from(error: E) -> Self { - Self::from_error(error) - } -} diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index 52d3713f32d..6da88c3e74d 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -9,8 +9,6 @@ use core::{ panic::Location, ptr::{addr_of, NonNull}, }; -#[cfg(feature = "std")] -use std::error::Error; #[cfg(nightly)] use crate::provider::{self, Demand, Provider}; @@ -70,18 +68,6 @@ impl Frame { } } - #[cfg(nightly)] - pub(crate) fn from_provider

( - provider: P, - location: &'static Location<'static>, - source: Option>, - ) -> Self - where - P: Provider + fmt::Debug + fmt::Display + Send + Sync + 'static, - { - Self::from_unerased(provider, location, source) - } - pub(crate) fn from_context( context: C, location: &'static Location<'static>, @@ -104,19 +90,6 @@ impl Frame { Self::from_unerased(FrameObject(object), location, source) } - #[cfg(feature = "std")] - pub(crate) fn from_error( - error: E, - location: &'static Location<'static>, - source: Option>, - ) -> Self - where - E: Error + Send + Sync + 'static, - { - // TODO: Pass error directly when Provider is implemented on errors - Self::from_unerased(ErrorRepr(error), location, source) - } - /// Returns the location where this `Frame` was created. #[must_use] pub const fn location(&self) -> &'static Location<'static> { @@ -259,48 +232,10 @@ impl fmt::Debug for ContextRepr { } } -impl Context for ContextRepr {} - #[cfg(nightly)] -impl Provider for ContextRepr { - fn provide<'a>(&'a self, _: &mut Demand<'a>) { - // Empty definition as `Context` does not convey provider information - } -} - -/// Temporary wrapper around [`Error`] to implement Provider. -/// -/// As [`Error`] does not necessarily implement [`Provider`], an implementation is provided. As soon -/// as [`Provider`] is implemented on [`Error`], this struct will be removed and used directly -/// instead. -// TODO: Remove when Provider is implemented on errors -#[cfg(feature = "std")] -#[repr(transparent)] -struct ErrorRepr(E); - -#[cfg(feature = "std")] -impl fmt::Display for ErrorRepr { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.0, fmt) - } -} - -#[cfg(feature = "std")] -impl fmt::Debug for ErrorRepr { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(&self.0, fmt) - } -} - -#[cfg(feature = "std")] -impl Context for ErrorRepr {} - -#[cfg(all(nightly, feature = "std"))] -impl Provider for ErrorRepr { +impl Provider for ContextRepr { fn provide<'a>(&'a self, demand: &mut Demand<'a>) { - if let Some(backtrace) = self.0.backtrace() { - demand.provide_ref(backtrace); - } + self.0.provide(demand); } } @@ -403,10 +338,6 @@ impl FrameRepr { // between `T` and `ContextRepr`/`ErrorRepr` is safe as those structs are // `repr(transparent)`. unsafe { - if let Some(addr) = (self.vtable.object_downcast)(self, TypeId::of::()) { - return Some(addr.cast()); - } - if let Some(addr) = (self.vtable.object_downcast)(self, TypeId::of::>()) { return Some(addr.cast()); @@ -417,10 +348,6 @@ impl FrameRepr { return Some(addr.cast()); } - #[cfg(feature = "std")] - if let Some(addr) = (self.vtable.object_downcast)(self, TypeId::of::>()) { - return Some(addr.cast()); - } None } } diff --git a/packages/engine/lib/error/src/future.rs b/packages/engine/lib/error/src/future.rs index 1dd53b29c3e..4d47bc7ce25 100644 --- a/packages/engine/lib/error/src/future.rs +++ b/packages/engine/lib/error/src/future.rs @@ -14,8 +14,6 @@ use core::{ use pin_project::pin_project; -#[cfg(nightly)] -use crate::provider::Provider; use crate::{Context, Result, ResultExt}; macro_rules! implement_future_adaptor { @@ -109,20 +107,6 @@ implement_lazy_future_adaptor!( Fut::Output ); -implement_future_adaptor!( - FutureWithProvider, - attach_provider, - Provider + Display + Debug + Send + Sync + 'static, - Fut::Output -); - -implement_lazy_future_adaptor!( - FutureWithLazyProvider, - attach_provider_lazy, - Provider + Display + Debug + Send + Sync + 'static, - Fut::Output -); - implement_future_adaptor!( FutureWithContext, change_context, @@ -225,41 +209,6 @@ pub trait FutureExt: Future + Sized { O: Display + Debug + Send + Sync + 'static, F: FnOnce() -> O; - /// Adds a [`Provider`] to the [`Frame`] stack when [`poll`]ing the [`Future`]. - /// - /// The provider is used to [`provide`] values either by calling - /// [`request_ref()`]/[`request_value()`] to return an iterator over all specified values, or by - /// using the [`Provider`] implementation on a [`Frame`]. - /// - /// [`provide`]: Provider::provide - /// [`request_ref()`]: crate::Report::request_ref - /// [`request_value()`]: crate::Report::request_value - /// [`Frame`]: crate::Frame - /// [`poll`]: Future::poll - #[cfg(nightly)] - fn attach_provider

(self, provider: P) -> FutureWithProvider - where - P: Provider + Display + Debug + Send + Sync + 'static; - - /// Lazily adds a [`Provider`] to the [`Frame`] stack when [`poll`]ing the [`Future`]. - /// - /// The provider is used to [`provide`] values either by calling - /// [`request_ref()`]/[`request_value()`] to return an iterator over all specified values, or by - /// using the [`Provider`] implementation on a [`Frame`]. - /// - /// The function is only executed in the `Err` arm. - /// - /// [`provide`]: Provider::provide - /// [`request_ref()`]: crate::Report::request_ref - /// [`request_value()`]: crate::Report::request_value - /// [`Frame`]: crate::Frame - /// [`poll`]: Future::poll - #[cfg(nightly)] - fn attach_provider_lazy(self, provider: F) -> FutureWithLazyProvider - where - P: Provider + Display + Debug + Send + Sync + 'static, - F: FnOnce() -> P; - /// Changes the [`Context`] of a [`Report`] when [`poll`]ing the [`Future`] returning /// [`Result`]. /// @@ -317,31 +266,6 @@ where } } - #[cfg(nightly)] - #[track_caller] - fn attach_provider

(self, provider: P) -> FutureWithProvider - where - P: Provider + Display + Debug + Send + Sync + 'static, - { - FutureWithProvider { - future: self, - inner: Some(provider), - } - } - - #[cfg(nightly)] - #[track_caller] - fn attach_provider_lazy(self, provider: F) -> FutureWithLazyProvider - where - P: Provider + Display + Debug + Send + Sync + 'static, - F: FnOnce() -> P, - { - FutureWithLazyProvider { - future: self, - inner: Some(provider), - } - } - #[track_caller] fn change_context(self, context: C) -> FutureWithContext where diff --git a/packages/engine/lib/error/src/hook.rs b/packages/engine/lib/error/src/hook.rs index d4a461eb676..5a2bde45833 100644 --- a/packages/engine/lib/error/src/hook.rs +++ b/packages/engine/lib/error/src/hook.rs @@ -66,7 +66,7 @@ impl Report<()> { { DEBUG_HOOK .set(Box::new(hook)) - .map_err(|_| Report::from_error(HookAlreadySet)) + .map_err(|_| Report::from_context(HookAlreadySet)) } /// Returns the hook that was previously set by [`set_debug_hook`], if any. @@ -115,7 +115,7 @@ impl Report<()> { { DISPLAY_HOOK .set(Box::new(hook)) - .map_err(|_| Report::from_error(HookAlreadySet)) + .map_err(|_| Report::from_context(HookAlreadySet)) } /// Returns the hook that was previously set by [`set_display_hook`], if any. diff --git a/packages/engine/lib/error/src/lib.rs b/packages/engine/lib/error/src/lib.rs index dd4ecaf687f..4f64a6097e8 100644 --- a/packages/engine/lib/error/src/lib.rs +++ b/packages/engine/lib/error/src/lib.rs @@ -201,8 +201,7 @@ mod macros; mod report; mod result; -#[cfg(feature = "std")] -mod error; +mod context; #[cfg(feature = "futures")] pub mod future; #[cfg(feature = "hooks")] @@ -210,84 +209,19 @@ mod hook; #[cfg(nightly)] pub mod provider; -use core::fmt; - #[cfg(feature = "futures")] #[doc(inline)] pub use self::future::FutureExt; #[cfg(feature = "hooks")] pub use self::hook::HookAlreadySet; pub use self::{ + context::Context, frame::Frame, macros::*, report::Report, result::{IntoReport, Result, ResultExt}, }; -/// Defines the current context of a [`Report`]. -/// -/// When in a `std` environment, every [`Error`] is a valid `Context`. This trait is not limited to -/// [`Error`]s and can also be manually implemented for custom objects. -/// -/// [`Error`]: std::error::Error -/// -/// ## Example -/// -/// Used for creating a [`Report`] or for switching the [`Report`]'s context: -/// -/// ```rust -/// # #![cfg_attr(any(not(feature = "std"), miri), allow(unused_imports))] -/// use std::{fmt, fs, io}; -/// -/// use error::{Context, IntoReport, Result, ResultExt}; -/// -/// # type Config = (); -/// #[derive(Debug)] -/// pub enum ConfigError { -/// ParseError, -/// } -/// -/// impl fmt::Display for ConfigError { -/// # #[allow(unused_variables)] -/// fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { -/// # const _: &str = stringify! { -/// ... -/// # }; Ok(()) -/// } -/// } -/// -/// // In this scenario, `Error` is not implemented for `ConfigError` for some reason, so implement -/// // `Context` manually. -/// impl Context for ConfigError {} -/// -/// # #[cfg(any(not(feature = "std"), miri))] -/// # pub fn read_file(_: &str) -> Result { error::bail!(ConfigError::ParseError) } -/// # #[cfg(all(feature = "std", not(miri)))] -/// pub fn read_file(path: &str) -> Result { -/// // Creates a `Report` from `io::Error`, the current context is `io::Error` -/// fs::read_to_string(path).report() -/// } -/// -/// pub fn parse_config(path: &str) -> Result { -/// // The return type of `parse_config` requires another context. By calling `change_context` -/// // the context may be changed. -/// read_file(path).change_context(ConfigError::ParseError)?; -/// -/// # const _: &str = stringify! { -/// ... -/// # }; Ok(()) -/// } -/// # let err = parse_config("invalid-path").unwrap_err(); -/// # #[cfg(all(feature = "std", not(miri)))] -/// # assert!(err.contains::()); -/// # assert!(err.contains::()); -/// # assert_eq!(err.frames().count(), 2); -/// ``` -pub trait Context: fmt::Display + fmt::Debug + Send + Sync + 'static {} - -#[cfg(feature = "std")] -impl Context for C {} - #[cfg(test)] pub(crate) mod test_helper { pub use alloc::{ diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index ff8b2a4e5fe..865307b0d45 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -7,12 +7,13 @@ use std::backtrace::{Backtrace, BacktraceStatus}; use tracing_error::{SpanTrace, SpanTraceStatus}; use super::Frame; -use crate::{iter::Frames, Context}; #[cfg(nightly)] use crate::{ + context::temporary_provider, iter::{RequestRef, RequestValue}, - provider::Provider, + provider::request_ref, }; +use crate::{iter::Frames, Context}; /// Contains a [`Frame`] stack consisting of an original error, context information, and optionally /// a [`Backtrace`] and a [`SpanTrace`]. @@ -170,12 +171,31 @@ impl Report { where T: Context, { + #[cfg(all(nightly, any(feature = "std", feature = "spantrace")))] + let provider = temporary_provider(&context); + #[cfg(all(nightly, feature = "std"))] - let backtrace = Some(Backtrace::capture()); + let backtrace = if request_ref::(&provider).is_some() { + None + } else { + Some(Backtrace::capture()) + }; + #[cfg(not(any(nightly, feature = "std")))] + let backtrace = Some(SpanTrace::capture()); - #[cfg(feature = "spantrace")] + #[cfg(all(nightly, feature = "spantrace"))] + let span_trace = if request_ref::(&provider).is_some() { + None + } else { + Some(SpanTrace::capture()) + }; + #[cfg(not(any(nightly, feature = "spantrace")))] let span_trace = Some(SpanTrace::capture()); + // Context will be moved in the next statement, so we need to drop the temporary provider + // first. + drop(provider); + Self::from_frame( Frame::from_context(context, Location::caller(), None), #[cfg(all(nightly, feature = "std"))] @@ -185,31 +205,6 @@ impl Report { ) } - /// Creates a new `Report` from the provided [`Error`]. - /// - /// [`Error`]: std::error::Error - #[track_caller] - #[cfg(feature = "std")] - pub fn from_error(error: T) -> Self - where - T: std::error::Error + Send + Sync + 'static, - { - #[cfg(nightly)] - let backtrace = if error.backtrace().is_some() { - None - } else { - Some(std::backtrace::Backtrace::capture()) - }; - - Self::from_frame( - Frame::from_error(error, Location::caller(), None), - #[cfg(nightly)] - backtrace, - #[cfg(feature = "spantrace")] - Some(tracing_error::SpanTrace::capture()), - ) - } - /// Adds a contextual information to the [`Frame`] stack. /// /// ## Example @@ -256,35 +251,6 @@ impl Report { ) } - /// Adds a [`Provider`] to the [`Frame`] stack. - /// - /// The provider is used to [`provide`] values either by calling - /// [`request_ref()`]/[`request_value()`] to return an iterator over all specified values, or by - /// using the [`Provider`] implementation on a [`Frame`]. - /// - /// [`provide`]: Provider::provide - /// [`request_ref()`]: Self::request_ref - /// [`request_value()`]: Self::request_value - /// [`Frame`]: crate::Frame - #[track_caller] - #[cfg(nightly)] - pub fn attach_provider

(self, provider: P) -> Self - where - P: Provider + fmt::Debug + fmt::Display + Send + Sync + 'static, - { - Self::from_frame( - Frame::from_provider( - provider, - Location::caller(), - Some(Box::new(self.inner.frame)), - ), - #[cfg(all(nightly, feature = "std"))] - self.inner.backtrace, - #[cfg(feature = "spantrace")] - self.inner.span_trace, - ) - } - /// Adds context information to the [`Frame`] stack enforcing a typed `Report`. /// /// Please see the [`Context`] documentation for more information. diff --git a/packages/engine/lib/error/src/result.rs b/packages/engine/lib/error/src/result.rs index c1cfca89314..5b27ba7528c 100644 --- a/packages/engine/lib/error/src/result.rs +++ b/packages/engine/lib/error/src/result.rs @@ -1,7 +1,5 @@ use core::fmt; -#[cfg(nightly)] -use crate::provider::Provider; use crate::{Context, Report}; /// [`Result`](std::result::Result)``](Report)`>` @@ -102,41 +100,6 @@ pub trait ResultExt { O: fmt::Display + fmt::Debug + Send + Sync + 'static, F: FnOnce() -> O; - /// Adds a [`Provider`] to the [`Frame`] stack. - /// - /// The provider is used to [`provide`] values either by calling - /// [`request_ref()`]/[`request_value()`] to return an iterator over all specified values, or by - /// using the [`Provider`] implementation on a [`Frame`]. - /// - /// [`provide`]: Provider::provide - /// [`request_ref()`]: crate::Report::request_ref - /// [`request_value()`]: crate::Report::request_value - /// [`Frame`]: crate::Frame - #[cfg(nightly)] - #[must_use] - fn attach_provider

(self, provider: P) -> Self - where - P: Provider + fmt::Display + fmt::Debug + Send + Sync + 'static; - - /// Lazily adds a [`Provider`] to the [`Frame`] stack. - /// - /// The provider is used to [`provide`] values either by calling - /// [`request_ref()`]/[`request_value()`] to return an iterator over all specified values, or by - /// using the [`Provider`] implementation on a [`Frame`]. - /// - /// The function is only executed in the `Err` arm. - /// - /// [`provide`]: Provider::provide - /// [`request_ref()`]: crate::Report::request_ref - /// [`request_value()`]: crate::Report::request_value - /// [`Frame`]: crate::Frame - #[cfg(nightly)] - #[must_use] - fn attach_provider_lazy(self, provider: F) -> Self - where - P: Provider + fmt::Display + fmt::Debug + Send + Sync + 'static, - F: FnOnce() -> P; - /// Changes the [`Context`] of a [`Report`] returning [`Result`]. /// /// Please see the [`Context`] documentation for more information. @@ -189,33 +152,6 @@ impl ResultExt for Result { } } - #[cfg(nightly)] - #[track_caller] - fn attach_provider

(self, provider: P) -> Self - where - P: Provider + fmt::Display + fmt::Debug + Send + Sync + 'static, - { - // Can't use `map_err` as `#[track_caller]` is unstable on closures - match self { - Ok(ok) => Ok(ok), - Err(report) => Err(report.attach_provider(provider)), - } - } - - #[cfg(nightly)] - #[track_caller] - fn attach_provider_lazy(self, provider: F) -> Self - where - P: Provider + fmt::Display + fmt::Debug + Send + Sync + 'static, - F: FnOnce() -> P, - { - // Can't use `map_err` as `#[track_caller]` is unstable on closures - match self { - Ok(ok) => Ok(ok), - Err(report) => Err(report.attach_provider(provider())), - } - } - #[track_caller] fn change_context(self, context: C2) -> Result where From 18d2b9ce0e13638e8698958b4e831d49cbe2b090 Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Fri, 3 Jun 2022 17:42:29 +0200 Subject: [PATCH 12/26] Update packages/engine/lib/error/src/frame.rs Co-authored-by: Alfred Mountfield --- packages/engine/lib/error/src/frame.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index 6da88c3e74d..e9c73bb267c 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -184,7 +184,7 @@ struct VTable { object_downcast: unsafe fn(&FrameRepr, target: TypeId) -> Option>, } -/// Wrapper around a contextual information living in a [`Frame`]. +/// Wrapper around the contextual information stored in a [`Frame`]. /// /// An information can be requested by calling [`Report::request_ref`]. It's used for the /// [`Display`] and [`Debug`] implementation for a [`Frame`]. If the information is a [`Provider`], From e9473b5ab47559ad4c56de257b464c3007b77434 Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Fri, 3 Jun 2022 17:42:38 +0200 Subject: [PATCH 13/26] Update packages/engine/lib/error/src/frame.rs Co-authored-by: Alfred Mountfield --- packages/engine/lib/error/src/frame.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index e9c73bb267c..e00a4c53bef 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -186,7 +186,7 @@ struct VTable { /// Wrapper around the contextual information stored in a [`Frame`]. /// -/// An information can be requested by calling [`Report::request_ref`]. It's used for the +/// A piece of information can be requested by calling [`Report::request_ref`]. It's used for the /// [`Display`] and [`Debug`] implementation for a [`Frame`]. If the information is a [`Provider`], /// use [`Report::attach_provider`] instead. struct FrameObject(T); From 6fc1721b033499242f99eaa2675fdd0bd00cb967 Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Fri, 3 Jun 2022 17:43:32 +0200 Subject: [PATCH 14/26] Update packages/engine/lib/error/src/report.rs Co-authored-by: Alfred Mountfield --- packages/engine/lib/error/src/report.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index 865307b0d45..98bbc4e880a 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -205,7 +205,7 @@ impl Report { ) } - /// Adds a contextual information to the [`Frame`] stack. + /// Adds contextual information to the [`Frame`] stack. /// /// ## Example /// From 31a543b215751d62b5854e788fc118ae0bf7ac33 Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Fri, 3 Jun 2022 17:43:42 +0200 Subject: [PATCH 15/26] Update packages/engine/lib/error/src/result.rs Co-authored-by: Alfred Mountfield --- packages/engine/lib/error/src/result.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/engine/lib/error/src/result.rs b/packages/engine/lib/error/src/result.rs index 5b27ba7528c..38f4d9d22fe 100644 --- a/packages/engine/lib/error/src/result.rs +++ b/packages/engine/lib/error/src/result.rs @@ -70,7 +70,7 @@ pub trait ResultExt { where O: fmt::Display + fmt::Debug + Send + Sync + 'static; - /// Lazily adds new information message to the [`Frame`] stack of a [`Report`]. + /// Lazily adds new contextual information to the [`Frame`] stack of a [`Report`]. /// /// The function is only executed in the `Err` arm. /// From 725a303a6f94975b21dcfe1ce3cce905fa4623b2 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 17:44:12 +0200 Subject: [PATCH 16/26] Simplify `Frame` implementation by removing `Unerased` --- packages/engine/lib/error/src/frame.rs | 103 ++++++------------------- 1 file changed, 23 insertions(+), 80 deletions(-) diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index e00a4c53bef..e2c19ba26a7 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -14,26 +14,6 @@ use core::{ use crate::provider::{self, Demand, Provider}; use crate::Context; -/// Trait alias to require all required traits used on a [`Frame`]. -/// -/// In order to implement these traits on a [`Frame`], the underlying type requires these types as -/// well. After creation of the [`Frame`] it's erased. To unerase it later on to act on the actual -/// Frame implementation, this trait is used. -#[cfg(nightly)] -trait Unerased: Provider + fmt::Debug + fmt::Display + Send + Sync + 'static {} -#[cfg(nightly)] -impl Unerased for T where T: Provider + fmt::Debug + fmt::Display + Send + Sync + 'static {} - -/// Trait alias to require all required traits used on a [`Frame`]. -/// -/// In order to implement these traits on a [`Frame`], the underlying type requires these types as -/// well. After creation of the [`Frame`] it's erased. To unerase it later on to act on the actual -/// Frame implementation, this trait is used. -#[cfg(not(nightly))] -trait Unerased: fmt::Debug + fmt::Display + Send + Sync + 'static {} -#[cfg(not(nightly))] -impl Unerased for T where T: fmt::Debug + fmt::Display + Send + Sync + 'static {} - /// A single error, contextual message, or error context inside of a [`Report`]. /// /// `Frame`s are organized as a singly linked list, which can be iterated by calling @@ -54,20 +34,6 @@ pub struct Frame { } impl Frame { - fn from_unerased( - object: T, - location: &'static Location<'static>, - source: Option>, - ) -> Self { - Self { - // SAFETY: `FrameRepr` must not be dropped without using the vtable, so it's wrapped in - // `ManuallyDrop`. A custom drop implementation is provided that takes care of this. - inner: unsafe { ManuallyDrop::new(FrameRepr::new(object)) }, - location, - source, - } - } - pub(crate) fn from_context( context: C, location: &'static Location<'static>, @@ -76,7 +42,13 @@ impl Frame { where C: Context, { - Self::from_unerased(ContextRepr(context), location, source) + Self { + // SAFETY: `FrameRepr` must not be dropped without using the vtable, so it's wrapped in + // `ManuallyDrop`. A custom drop implementation is provided that takes care of this. + inner: unsafe { ManuallyDrop::new(FrameRepr::new(context)) }, + location, + source, + } } pub(crate) fn from_object( @@ -87,7 +59,7 @@ impl Frame { where O: fmt::Display + fmt::Debug + Send + Sync + 'static, { - Self::from_unerased(FrameObject(object), location, source) + Self::from_context(AttachedObject(object), location, source) } /// Returns the location where this `Frame` was created. @@ -174,13 +146,13 @@ impl Drop for Frame { /// Stores functions to act on the underlying [`Frame`] type without knowing the unerased type. /// -/// This works around the limitation of not being able to coerce from `Box` to +/// This works around the limitation of not being able to coerce from `Box` to /// `Box` to add downcasting. Also this works around dynamic dispatching, as the functions /// are stored and called directly. In addition this reduces the memory usage by one pointer, as the /// `VTable` is stored next to the object. struct VTable { object_drop: unsafe fn(Box), - object_ref: unsafe fn(&FrameRepr) -> &dyn Unerased, + object_ref: unsafe fn(&FrameRepr) -> &dyn Context, object_downcast: unsafe fn(&FrameRepr, target: TypeId) -> Option>, } @@ -189,56 +161,27 @@ struct VTable { /// A piece of information can be requested by calling [`Report::request_ref`]. It's used for the /// [`Display`] and [`Debug`] implementation for a [`Frame`]. If the information is a [`Provider`], /// use [`Report::attach_provider`] instead. -struct FrameObject(T); +struct AttachedObject(T); -impl fmt::Display for FrameObject { +impl fmt::Display for AttachedObject { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.0, fmt) } } -impl fmt::Debug for FrameObject { +impl fmt::Debug for AttachedObject { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(&self.0, fmt) } } -impl Context for FrameObject {} - -#[cfg(nightly)] -impl Provider for FrameObject { +impl Context for AttachedObject { + #[cfg(nightly)] fn provide<'a>(&'a self, demand: &mut Demand<'a>) { demand.provide_ref(&self.0); } } -/// Wrapper around [`Context`]. -/// -/// As [`Context`] does not necessarily implement [`Provider`] but [`Unerased`] requires it (on -/// nightly), an empty implementation is provided. If a [`Provider`] is required use it directly -/// instead of [`Context`]. -#[repr(transparent)] -struct ContextRepr(C); - -impl fmt::Display for ContextRepr { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.0, fmt) - } -} - -impl fmt::Debug for ContextRepr { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(&self.0, fmt) - } -} - -#[cfg(nightly)] -impl Provider for ContextRepr { - fn provide<'a>(&'a self, demand: &mut Demand<'a>) { - self.0.provide(demand); - } -} - impl VTable { /// Drops the `frame` /// @@ -255,12 +198,12 @@ impl VTable { drop(unerased); } - /// Unerase the object as `&dyn Unerased` + /// Unerase the object as `&dyn Context` /// /// # Safety /// /// - Layout of `Self` must match `FrameRepr`. - unsafe fn object_ref(frame: &FrameRepr) -> &dyn Unerased { + unsafe fn object_ref(frame: &FrameRepr) -> &dyn Context { // Attach T's native vtable onto the pointer to `self._unerased` let unerased = (frame as *const FrameRepr).cast::>(); // inside of vtable it's allowed to access `_unerased` @@ -273,7 +216,7 @@ impl VTable { /// # Safety /// /// - Layout of `Self` must match `FrameRepr`. - unsafe fn object_downcast( + unsafe fn object_downcast( frame: &FrameRepr, target: TypeId, ) -> Option> { @@ -302,7 +245,7 @@ struct FrameRepr { impl FrameRepr where - T: Unerased, + T: Context, { /// Creates a new frame from an unerased object. /// @@ -326,7 +269,7 @@ where #[allow(clippy::used_underscore_binding)] impl FrameRepr { - fn unerase(&self) -> &dyn Unerased { + fn unerase(&self) -> &dyn Context { // SAFETY: Use vtable to attach T's native vtable for the right original type T. unsafe { (self.vtable.object_ref)(self) } } @@ -338,12 +281,12 @@ impl FrameRepr { // between `T` and `ContextRepr`/`ErrorRepr` is safe as those structs are // `repr(transparent)`. unsafe { - if let Some(addr) = (self.vtable.object_downcast)(self, TypeId::of::>()) - { + if let Some(addr) = (self.vtable.object_downcast)(self, TypeId::of::()) { return Some(addr.cast()); } - if let Some(addr) = (self.vtable.object_downcast)(self, TypeId::of::>()) + if let Some(addr) = + (self.vtable.object_downcast)(self, TypeId::of::>()) { return Some(addr.cast()); } From 3277be638a08e5d638064cbf04b7834f75c32c60 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 3 Jun 2022 18:23:01 +0200 Subject: [PATCH 17/26] Clean up code --- packages/engine/lib/error/src/context.rs | 9 ++-- packages/engine/lib/error/src/frame.rs | 68 ++++++++++++------------ packages/engine/lib/error/src/future.rs | 36 ++++++------- packages/engine/lib/error/src/hook.rs | 4 +- packages/engine/lib/error/src/lib.rs | 4 +- packages/engine/lib/error/src/macros.rs | 52 ++++-------------- packages/engine/lib/error/src/report.rs | 33 ++++++------ packages/engine/lib/error/src/result.rs | 26 ++++----- 8 files changed, 102 insertions(+), 130 deletions(-) diff --git a/packages/engine/lib/error/src/context.rs b/packages/engine/lib/error/src/context.rs index 8dcb496321a..438cfd86e30 100644 --- a/packages/engine/lib/error/src/context.rs +++ b/packages/engine/lib/error/src/context.rs @@ -65,8 +65,6 @@ use crate::Report; /// ``` pub trait Context: fmt::Display + fmt::Debug + Send + Sync + 'static { /// Provide values which can then be requested by [`Report`]. - /// - /// [`Report`]: crate::Report #[cfg(nightly)] #[allow(unused_variables)] fn provide<'a>(&'a self, demand: &mut Demand<'a>) {} @@ -78,12 +76,15 @@ where { #[track_caller] #[inline] - fn from(error: C) -> Self { - Self::from_context(error) + fn from(context: C) -> Self { + Self::new(context) } } /// Turns a [`Context`] into a temporary [`Provider`]. +/// +/// To enable the usage of the [`Provider`] trait without implementing [`Provider`] for [`Context`] +/// this function wraps a reference to a [`Context`] inside of a [`Provider`] // We can't implement `Provider` on Context as `Error` will implement `Provider` and `Context` will // be implemented on `Error`. For `request`ing a type from `Context`, we need a `Provider` // implementation however. diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index e2c19ba26a7..41b3d98e877 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -14,17 +14,16 @@ use core::{ use crate::provider::{self, Demand, Provider}; use crate::Context; -/// A single error, contextual message, or error context inside of a [`Report`]. +/// A single context or attachment inside of a [`Report`]. /// /// `Frame`s are organized as a singly linked list, which can be iterated by calling -/// [`Report::frames()`]. The head is pointing to the most recent context or contextual message, -/// the tail is the root error created by [`Report::from_context()`] or [`Report::from_error()`]. -/// The next `Frame` can be accessed by requesting it by calling [`Report::request_ref()`]. +/// [`Report::frames()`]. The head is pointing to the most recent context or attachment, the tail is +/// the root context created by [`Report::new()`]. The next `Frame` can be accessed by +/// requesting it by calling [`Report::request_ref()`]. /// /// [`Report`]: crate::Report /// [`Report::frames()`]: crate::Report::frames -/// [`Report::from_error()`]: crate::Report::from_error -/// [`Report::from_context()`]: crate::Report::from_context +/// [`Report::new()`]: crate::Report::new /// [`Report::request_ref()`]: crate::Report::request_ref pub struct Frame { inner: ManuallyDrop>, @@ -51,13 +50,13 @@ impl Frame { } } - pub(crate) fn from_object( - object: O, + pub(crate) fn from_attachment( + object: A, location: &'static Location<'static>, source: Option>, ) -> Self where - O: fmt::Display + fmt::Debug + Send + Sync + 'static, + A: fmt::Display + fmt::Debug + Send + Sync + 'static, { Self::from_context(AttachedObject(object), location, source) } @@ -88,13 +87,13 @@ impl Frame { provider::request_value(self) } - /// Returns if `T` is the type held by this frame. + /// Returns if `T` is the held context or attachment by this frame. #[must_use] pub fn is(&self) -> bool { self.downcast_ref::().is_some() } - /// Downcasts this frame if the held provider object is the same as `T`. + /// Downcasts this frame if the held context or attachment is the same as `T`. #[must_use] pub fn downcast_ref(&self) -> Option<&T> { self.inner.as_ref().downcast().map(|addr| { @@ -156,11 +155,14 @@ struct VTable { object_downcast: unsafe fn(&FrameRepr, target: TypeId) -> Option>, } -/// Wrapper around the contextual information stored in a [`Frame`]. +/// Wrapper around an attachment to unify the interface for a [`Frame`]. /// -/// A piece of information can be requested by calling [`Report::request_ref`]. It's used for the -/// [`Display`] and [`Debug`] implementation for a [`Frame`]. If the information is a [`Provider`], -/// use [`Report::attach_provider`] instead. +/// A piece of information can be requested by calling [`Report::request_ref()`]. It's used for the +/// [`Display`] and [`Debug`] implementation for a [`Frame`]. +/// +/// [`Report::request_ref()`]: crate::Report::request_ref +/// [`Display`]: core::fmt::Display +/// [`Debug`]: core::fmt::Debug struct AttachedObject(T); impl fmt::Display for AttachedObject { @@ -183,18 +185,18 @@ impl Context for AttachedO } impl VTable { - /// Drops the `frame` + /// Drops the `frame`. /// /// # Safety /// /// - Layout of `*frame` must match `FrameRepr`. - unsafe fn object_drop(frame: Box) { + unsafe fn object_drop(frame: Box) { // Attach T's native vtable onto the pointer to `self._unerased` // Note: This must not use `mem::transmute` because it tries to reborrow the `Unique` // contained in `Box`, which must not be done. In practice this probably won't make any // difference by now, but technically it's unsound. // see: https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md - let unerased = Box::from_raw(Box::into_raw(frame).cast::>()); + let unerased = Box::from_raw(Box::into_raw(frame).cast::>()); drop(unerased); } @@ -202,10 +204,10 @@ impl VTable { /// /// # Safety /// - /// - Layout of `Self` must match `FrameRepr`. - unsafe fn object_ref(frame: &FrameRepr) -> &dyn Context { + /// - Layout of `frame` must match `FrameRepr`. + unsafe fn object_ref(frame: &FrameRepr) -> &dyn Context { // Attach T's native vtable onto the pointer to `self._unerased` - let unerased = (frame as *const FrameRepr).cast::>(); + let unerased = (frame as *const FrameRepr).cast::>(); // inside of vtable it's allowed to access `_unerased` #[allow(clippy::used_underscore_binding)] &(*(unerased))._unerased @@ -215,14 +217,14 @@ impl VTable { /// /// # Safety /// - /// - Layout of `Self` must match `FrameRepr`. - unsafe fn object_downcast( + /// - Layout of `frame` must match `FrameRepr`. + unsafe fn object_downcast( frame: &FrameRepr, target: TypeId, ) -> Option> { - if TypeId::of::() == target { + if TypeId::of::() == target { // Attach T's native vtable onto the pointer to `self._unerased` - let unerased = (frame as *const FrameRepr).cast::>(); + let unerased = (frame as *const FrameRepr).cast::>(); // inside of vtable it's allowed to access `_unerased` #[allow(clippy::used_underscore_binding)] let addr = addr_of!((*(unerased))._unerased) as *mut (); @@ -243,23 +245,23 @@ struct FrameRepr { _unerased: T, } -impl FrameRepr +impl FrameRepr where - T: Context, + C: Context, { - /// Creates a new frame from an unerased object. + /// Creates a new [`Frame`] from an unerased [`Context`] object. /// /// # Safety /// /// Must not be dropped without calling `vtable.object_drop` - unsafe fn new(object: T) -> Box { + unsafe fn new(context: C) -> Box { let unerased_frame = Self { vtable: &VTable { - object_drop: VTable::object_drop::, - object_ref: VTable::object_ref::, - object_downcast: VTable::object_downcast::, + object_drop: VTable::object_drop::, + object_ref: VTable::object_ref::, + object_downcast: VTable::object_downcast::, }, - _unerased: object, + _unerased: context, }; let unerased_box = Box::new(unerased_frame); // erase the frame by casting the pointer to `FrameBox<()>` diff --git a/packages/engine/lib/error/src/future.rs b/packages/engine/lib/error/src/future.rs index 4d47bc7ce25..e2edd60fc30 100644 --- a/packages/engine/lib/error/src/future.rs +++ b/packages/engine/lib/error/src/future.rs @@ -94,14 +94,14 @@ macro_rules! implement_lazy_future_adaptor { } implement_future_adaptor!( - FutureWithObject, + FutureWithAttachment, attach, Display + Debug + Send + Sync + 'static, Fut::Output ); implement_lazy_future_adaptor!( - FutureWithLazyObject, + FutureWithLazyAttachment, attach_lazy, Display + Debug + Send + Sync + 'static, Fut::Output @@ -153,7 +153,7 @@ pub trait FutureExt: Future + Sized { /// # let fut = async { /// # let user = User; /// # let resource = Resource; - /// // A contextual message can be provided before polling the `Future` + /// // An attachment can be added before polling the `Future` /// load_resource(&user, &resource).attach("Could not load resource").await /// # }; /// # #[cfg(not(miri))] @@ -161,9 +161,9 @@ pub trait FutureExt: Future + Sized { /// # Result::<_, ResourceError>::Ok(()) /// ``` #[track_caller] - fn attach(self, object: O) -> FutureWithObject + fn attach(self, attachment: A) -> FutureWithAttachment where - O: Display + Debug + Send + Sync + 'static; + A: Display + Debug + Send + Sync + 'static; /// Lazily adds new contextual information to the [`Frame`] stack of a [`Report`] when /// [`poll`]ing the [`Future`]. @@ -196,7 +196,7 @@ pub trait FutureExt: Future + Sized { /// # let fut = async { /// # let user = User; /// # let resource = Resource; - /// // A contextual message can be provided before polling the `Future` + /// // An attachment can be added before polling the `Future` /// load_resource(&user, &resource).attach_lazy(|| format!("Could not load resource {resource}")).await /// # }; /// # #[cfg(not(miri))] @@ -204,10 +204,10 @@ pub trait FutureExt: Future + Sized { /// # Result::<_, ResourceError>::Ok(()) /// ``` #[track_caller] - fn attach_lazy(self, object: F) -> FutureWithLazyObject + fn attach_lazy(self, attachment: F) -> FutureWithLazyAttachment where - O: Display + Debug + Send + Sync + 'static, - F: FnOnce() -> O; + A: Display + Debug + Send + Sync + 'static, + F: FnOnce() -> A; /// Changes the [`Context`] of a [`Report`] when [`poll`]ing the [`Future`] returning /// [`Result`]. @@ -244,25 +244,25 @@ impl FutureExt for Fut where Fut::Output: ResultExt, { - fn attach(self, object: O) -> FutureWithObject + fn attach(self, attachment: A) -> FutureWithAttachment where - O: Display + Debug + Send + Sync + 'static, + A: Display + Debug + Send + Sync + 'static, { - FutureWithObject { + FutureWithAttachment { future: self, - inner: Some(object), + inner: Some(attachment), } } #[track_caller] - fn attach_lazy(self, object: F) -> FutureWithLazyObject + fn attach_lazy(self, attachment: F) -> FutureWithLazyAttachment where - O: Display + Debug + Send + Sync + 'static, - F: FnOnce() -> O, + A: Display + Debug + Send + Sync + 'static, + F: FnOnce() -> A, { - FutureWithLazyObject { + FutureWithLazyAttachment { future: self, - inner: Some(object), + inner: Some(attachment), } } diff --git a/packages/engine/lib/error/src/hook.rs b/packages/engine/lib/error/src/hook.rs index 5a2bde45833..a3d88325803 100644 --- a/packages/engine/lib/error/src/hook.rs +++ b/packages/engine/lib/error/src/hook.rs @@ -66,7 +66,7 @@ impl Report<()> { { DEBUG_HOOK .set(Box::new(hook)) - .map_err(|_| Report::from_context(HookAlreadySet)) + .map_err(|_| Report::new(HookAlreadySet)) } /// Returns the hook that was previously set by [`set_debug_hook`], if any. @@ -115,7 +115,7 @@ impl Report<()> { { DISPLAY_HOOK .set(Box::new(hook)) - .map_err(|_| Report::from_context(HookAlreadySet)) + .map_err(|_| Report::new(HookAlreadySet)) } /// Returns the hook that was previously set by [`set_display_hook`], if any. diff --git a/packages/engine/lib/error/src/lib.rs b/packages/engine/lib/error/src/lib.rs index 4f64a6097e8..060ec2696f9 100644 --- a/packages/engine/lib/error/src/lib.rs +++ b/packages/engine/lib/error/src/lib.rs @@ -47,7 +47,7 @@ //! # Usage //! //! [`Report`] is supposed to be used as the [`Err`] variant of a `Result`. This crates provides a -//! [`Result`] type alias, which uses [`Report`] as [`Err`] variant and can be used as +//! [`Result`] type alias, which uses [`Report`] as [`Err`] variant and can be used as //! return type: //! //! ``` @@ -76,7 +76,7 @@ //! } //! ``` //! -//! A contextual message can be provided to lower level errors. +//! An attachment can be provided to lower level errors. //! //! ``` //! use std::{collections::HashMap, fmt}; diff --git a/packages/engine/lib/error/src/macros.rs b/packages/engine/lib/error/src/macros.rs index 24915d2b1d4..2d7677f47b5 100644 --- a/packages/engine/lib/error/src/macros.rs +++ b/packages/engine/lib/error/src/macros.rs @@ -12,25 +12,18 @@ pub mod __private { //! This is a stable implementation for specialization (only possible within macros, as //! there is no trait bound for these things). //! - //! The different tags [`ReportTag`], [`ErrorTag`], and [`ContextTag`] have a blanket - //! implementation returning a concrete type. This type is then used to create a [`Report`]. + //! The different tags [`ReportTag`] and [`ContextTag`] have a blanket implementation + //! returning a concrete type. This type is then used to create a [`Report`]. //! - //! [`ErrorTag`] is implemented for `T: `[`Error`]s, [`ContextTag`] is implemented for `&T: - //! `[`Context`]s. As [`ContextTag`] is implemented for references, [`ErrorTag`] has a - //! higher precedence and will be picked up first. So calling `my_error.__kind()` will - //! always return [`ErrorReporter`]. This can then be used to create a [`Report`] by calling - //! `my_error.__kind().report(my_error)`, which will then use [`Report::from_error`]. For - //! [`ContextTag`], [`Report::from_context`] will be used. - //! - //! [`ReportTag`] is a special case, which is implemented for any [`Report`], so this has - //! the highest precedence when calling `report.__kind()`. This will use an identity - //! function when creating a [`Report`] to ensure that no information will be lost. + //! [`ContextTag`] is implemented for `T: `[`Context`]s while [`ReportTag`] is implement for + //! [`Report`]s. Calling `my_report.__kind()` will always return a [`Reporter`] while + //! `my_context.__kind()` will return a [`ContextReporter`] so a [`Report`] has the highest + //! precedence when calling `.__kind()`. This will use an identity function when creating a + //! [`Report`] to ensure that no information will be lost. //! //! Note: The methods on the tags are called `__kind` instead of `kind` to avoid misleading //! suggestions from the Rust compiler, when calling `kind`. It would suggest implementing a //! tag for the type which cannot and should not be implemented. - //! - //! [`Error`]: std::error::Error pub trait ReportTag { #[inline] @@ -40,16 +33,6 @@ pub mod __private { } impl ReportTag for Report {} - #[cfg(feature = "std")] - pub trait ErrorTag { - #[inline] - fn __kind(&self) -> ErrorReporter { - ErrorReporter - } - } - #[cfg(feature = "std")] - impl ErrorTag for T where T: ?Sized + std::error::Error + Send + Sync + 'static {} - pub trait ContextTag { #[inline] fn __kind(&self) -> ContextReporter { @@ -67,39 +50,24 @@ pub mod __private { } } - #[cfg(feature = "std")] - pub struct ErrorReporter; - #[cfg(feature = "std")] - impl ErrorReporter { - #[inline] - #[track_caller] - pub fn report( - self, - error: C, - ) -> Report { - Report::from(error) - } - } pub struct ContextReporter; impl ContextReporter { #[inline] #[track_caller] pub fn report(self, context: C) -> Report { - Report::from_context(context) + Report::new(context) } } } // Import anonymously to allow calling `__kind` but forbid implementing the tag-traits. - #[cfg(feature = "std")] - pub use self::specialization::ErrorTag as _; pub use self::specialization::{ContextTag as _, ReportTag as _}; } /// Creates a [`Report`] from the given parameters. /// -/// The parameters may either be [`Context`] or [`Error`]. The returned [`Report`] will use the the -/// provided type as context. +/// The parameters may either be [`Context`] or a [`Report`]. The returned [`Report`] will use the +/// the provided type as context. /// /// [`Report`]: crate::Report /// [`Context`]: crate::Context diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index 98bbc4e880a..24b84ee16b6 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -15,34 +15,31 @@ use crate::{ }; use crate::{iter::Frames, Context}; -/// Contains a [`Frame`] stack consisting of an original error, context information, and optionally +/// Contains a [`Frame`] stack consisting of [`Context`]s, attachments, and optionally /// a [`Backtrace`] and a [`SpanTrace`]. /// /// To enable the backtrace, make sure `RUST_BACKTRACE` or `RUST_LIB_BACKTRACE` is set according to /// the [`Backtrace` documentation][`Backtrace`]. To enable the span trace, [`ErrorLayer`] has to /// be enabled. /// -/// Context information can be added by using [`attach()`] or [`ResultExt`]. The [`Frame`] -/// stack can be iterated by using [`frames()`]. +/// Attachments can be added by using [`attach()`]. The [`Frame`] stack can be iterated by using +/// [`frames()`]. /// -/// To enforce context information generation, a context [`Provider`] needs to be used. When -/// creating a `Report` by using [`from_error()`] or [`from_context()`], the parameter is used as -/// context in the `Report`. To provide a new one, use [`change_context()`] or [`ResultExt`] to add -/// it, which may also be used to provide more context information than only a display message. This -/// information can then be retrieved by calling [`request_ref()`] or [`request_value()`]. +/// To enforce context information generation, a [`Context`] needs to be used. When creating a +/// `Report` by using [`new()`], the passed [`Context`] is used to set the _current +/// context_ on the `Report`. To provide a new one, use [`change_context()`], which may also be used +/// to provide more context information than only a display message. This information can then be +/// retrieved by calling [`request_ref()`] or [`request_value()`]. /// /// [`Backtrace`]: std::backtrace::Backtrace /// [`SpanTrace`]: tracing_error::SpanTrace /// [`ErrorLayer`]: tracing_error::ErrorLayer /// [`attach()`]: Self::attach -/// [`from_error()`]: Self::from_error -/// [`from_context()`]: Self::from_context +/// [`new()`]: Self::new /// [`frames()`]: Self::frames /// [`change_context()`]: Self::change_context /// [`request_ref()`]: Self::request_ref /// [`request_value()`]: Self::request_value -/// [`ResultExt`]: crate::ResultExt -/// [`Provider`]: crate::provider::Provider /// /// # Examples /// @@ -167,7 +164,7 @@ impl Report { /// Creates a new `Report` from a provided scope. #[track_caller] - pub fn from_context(context: T) -> Self + pub fn new(context: T) -> Self where T: Context, { @@ -238,12 +235,16 @@ impl Report { /// /// assert_eq!(suggestion.0, "Better use a file which exists next time!"); #[track_caller] - pub fn attach(self, object: O) -> Self + pub fn attach(self, attachment: A) -> Self where - O: fmt::Display + fmt::Debug + Send + Sync + 'static, + A: fmt::Display + fmt::Debug + Send + Sync + 'static, { Self::from_frame( - Frame::from_object(object, Location::caller(), Some(Box::new(self.inner.frame))), + Frame::from_attachment( + attachment, + Location::caller(), + Some(Box::new(self.inner.frame)), + ), #[cfg(all(nightly, feature = "std"))] self.inner.backtrace, #[cfg(feature = "spantrace")] diff --git a/packages/engine/lib/error/src/result.rs b/packages/engine/lib/error/src/result.rs index 38f4d9d22fe..6f18cf8b20c 100644 --- a/packages/engine/lib/error/src/result.rs +++ b/packages/engine/lib/error/src/result.rs @@ -66,9 +66,9 @@ pub trait ResultExt { /// # Result::Ok(()) /// ``` #[must_use] - fn attach(self, object: O) -> Self + fn attach(self, attachment: A) -> Self where - O: fmt::Display + fmt::Debug + Send + Sync + 'static; + A: fmt::Display + fmt::Debug + Send + Sync + 'static; /// Lazily adds new contextual information to the [`Frame`] stack of a [`Report`]. /// @@ -95,10 +95,10 @@ pub trait ResultExt { /// # Result::Ok(()) /// ``` #[must_use] - fn attach_lazy(self, object: F) -> Self + fn attach_lazy(self, attachment: F) -> Self where - O: fmt::Display + fmt::Debug + Send + Sync + 'static, - F: FnOnce() -> O; + A: fmt::Display + fmt::Debug + Send + Sync + 'static, + F: FnOnce() -> A; /// Changes the [`Context`] of a [`Report`] returning [`Result`]. /// @@ -118,7 +118,7 @@ pub trait ResultExt { /// /// [`Frame`]: crate::Frame // TODO: come up with a decent example - fn change_context_lazy(self, op: F) -> Result + fn change_context_lazy(self, context: F) -> Result where C: Context, F: FnOnce() -> C; @@ -128,27 +128,27 @@ impl ResultExt for Result { type Ok = T; #[track_caller] - fn attach(self, message: M) -> Self + fn attach(self, attachment: A) -> Self where - M: fmt::Display + fmt::Debug + Send + Sync + 'static, + A: fmt::Display + fmt::Debug + Send + Sync + 'static, { // Can't use `map_err` as `#[track_caller]` is unstable on closures match self { Ok(ok) => Ok(ok), - Err(report) => Err(report.attach(message)), + Err(report) => Err(report.attach(attachment)), } } #[track_caller] - fn attach_lazy(self, message: F) -> Self + fn attach_lazy(self, attachment: F) -> Self where - M: fmt::Display + fmt::Debug + Send + Sync + 'static, - F: FnOnce() -> M, + A: fmt::Display + fmt::Debug + Send + Sync + 'static, + F: FnOnce() -> A, { // Can't use `map_err` as `#[track_caller]` is unstable on closures match self { Ok(ok) => Ok(ok), - Err(report) => Err(report.attach(message())), + Err(report) => Err(report.attach(attachment())), } } From 35ecbb18150e064fc20dd7e2a10bd092437ec68e Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 6 Jun 2022 10:21:58 +0200 Subject: [PATCH 18/26] Clarify documentation on `Frame` --- packages/engine/lib/error/src/frame.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index 41b3d98e877..214fae9502c 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -17,9 +17,9 @@ use crate::Context; /// A single context or attachment inside of a [`Report`]. /// /// `Frame`s are organized as a singly linked list, which can be iterated by calling -/// [`Report::frames()`]. The head is pointing to the most recent context or attachment, the tail is -/// the root context created by [`Report::new()`]. The next `Frame` can be accessed by -/// requesting it by calling [`Report::request_ref()`]. +/// [`Report::frames()`]. The head is pointing to the current context or attachment, the tail is the +/// root context created by [`Report::new()`]. The next `Frame` can be accessed by requesting it by +/// calling [`Report::request_ref()`]. /// /// [`Report`]: crate::Report /// [`Report::frames()`]: crate::Report::frames From 028ccfe72fcf2837343d64711514c5e15c77b9dc Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 6 Jun 2022 10:27:54 +0200 Subject: [PATCH 19/26] Clarify documentation on trace capturing --- packages/engine/lib/error/src/report.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index 24b84ee16b6..07c0e314c5e 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -18,9 +18,10 @@ use crate::{iter::Frames, Context}; /// Contains a [`Frame`] stack consisting of [`Context`]s, attachments, and optionally /// a [`Backtrace`] and a [`SpanTrace`]. /// -/// To enable the backtrace, make sure `RUST_BACKTRACE` or `RUST_LIB_BACKTRACE` is set according to -/// the [`Backtrace` documentation][`Backtrace`]. To enable the span trace, [`ErrorLayer`] has to -/// be enabled. +/// If the root [`Frame`] contains a [`Backtrace`]/[`SpanTrace`], these are used, otherwise they +/// are eventually captured. To enable capturing of the backtrace, make sure `RUST_BACKTRACE` or +/// `RUST_LIB_BACKTRACE` is set according to the [`Backtrace` documentation][`Backtrace`]. To enable +/// capturing of the span trace, an [`ErrorLayer`] has to be enabled. /// /// Attachments can be added by using [`attach()`]. The [`Frame`] stack can be iterated by using /// [`frames()`]. From b2bc61ed60de3f764e03687f9417bc3ee3007d30 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 6 Jun 2022 10:32:33 +0200 Subject: [PATCH 20/26] Mention the feature flags section on [`Report`] --- packages/engine/lib/error/src/report.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index 07c0e314c5e..b093460adad 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -21,7 +21,8 @@ use crate::{iter::Frames, Context}; /// If the root [`Frame`] contains a [`Backtrace`]/[`SpanTrace`], these are used, otherwise they /// are eventually captured. To enable capturing of the backtrace, make sure `RUST_BACKTRACE` or /// `RUST_LIB_BACKTRACE` is set according to the [`Backtrace` documentation][`Backtrace`]. To enable -/// capturing of the span trace, an [`ErrorLayer`] has to be enabled. +/// capturing of the span trace, an [`ErrorLayer`] has to be enabled. Please also look at the +/// [Feature Flags] section. /// /// Attachments can be added by using [`attach()`]. The [`Frame`] stack can be iterated by using /// [`frames()`]. @@ -41,6 +42,7 @@ use crate::{iter::Frames, Context}; /// [`change_context()`]: Self::change_context /// [`request_ref()`]: Self::request_ref /// [`request_value()`]: Self::request_value +/// [Feature Flags]: index.html#feature-flags /// /// # Examples /// From 7900847a7e04a2c36805caa4c1dc8d00fe2ede85 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 6 Jun 2022 10:43:24 +0200 Subject: [PATCH 21/26] Fix tests --- packages/engine/lib/error/src/report.rs | 1 + packages/engine/lib/orchestrator/src/process/local.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index b093460adad..a33d79dcc69 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -194,6 +194,7 @@ impl Report { // Context will be moved in the next statement, so we need to drop the temporary provider // first. + #[cfg(all(nightly, any(feature = "std", feature = "spantrace")))] drop(provider); Self::from_frame( diff --git a/packages/engine/lib/orchestrator/src/process/local.rs b/packages/engine/lib/orchestrator/src/process/local.rs index 18e985c60a6..a8784dd1343 100644 --- a/packages/engine/lib/orchestrator/src/process/local.rs +++ b/packages/engine/lib/orchestrator/src/process/local.rs @@ -42,7 +42,7 @@ impl process::Process for LocalProcess { // From `Child::kill` docs: Forces the child process to exit. If the child has // already exited, an InvalidInput error is returned std::io::ErrorKind::InvalidInput => Ok(()), - _ => Err(Report::from_error(e)), + _ => Err(Report::new(e)), }) .change_context(OrchestratorError::from("Could not kill the process")); From f0280b92988d0841209d5c02ffa56e3e90ce54fe Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 6 Jun 2022 11:27:54 +0200 Subject: [PATCH 22/26] Clarified `Frame` docs more --- packages/engine/lib/error/src/frame.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index 214fae9502c..10087efa825 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -17,8 +17,8 @@ use crate::Context; /// A single context or attachment inside of a [`Report`]. /// /// `Frame`s are organized as a singly linked list, which can be iterated by calling -/// [`Report::frames()`]. The head is pointing to the current context or attachment, the tail is the -/// root context created by [`Report::new()`]. The next `Frame` can be accessed by requesting it by +/// [`Report::frames()`]. The head the current context or attachment, the tail is the root context +/// created by [`Report::new()`]. The next `Frame` can be accessed by requesting it by /// calling [`Report::request_ref()`]. /// /// [`Report`]: crate::Report From e8dbd74fd1940ee96b8087fdbc0786a544e1feec Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 6 Jun 2022 11:29:15 +0200 Subject: [PATCH 23/26] Clarified `Frame` docs more --- packages/engine/lib/error/src/frame.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/engine/lib/error/src/frame.rs b/packages/engine/lib/error/src/frame.rs index 10087efa825..2224a6a5554 100644 --- a/packages/engine/lib/error/src/frame.rs +++ b/packages/engine/lib/error/src/frame.rs @@ -17,9 +17,9 @@ use crate::Context; /// A single context or attachment inside of a [`Report`]. /// /// `Frame`s are organized as a singly linked list, which can be iterated by calling -/// [`Report::frames()`]. The head the current context or attachment, the tail is the root context -/// created by [`Report::new()`]. The next `Frame` can be accessed by requesting it by -/// calling [`Report::request_ref()`]. +/// [`Report::frames()`]. The head contains the current context or attachment, and the tail contains +/// the root context created by [`Report::new()`]. The next `Frame` can be accessed by requesting it +/// by calling [`Report::request_ref()`]. /// /// [`Report`]: crate::Report /// [`Report::frames()`]: crate::Report::frames From ea43ccc7140f7c54c6fa57cac1e86f0bd27c12c2 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 6 Jun 2022 11:34:03 +0200 Subject: [PATCH 24/26] Fix clippy on no-default-features --- packages/engine/lib/error/src/context.rs | 6 ++++-- packages/engine/lib/error/src/report.rs | 8 +++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/engine/lib/error/src/context.rs b/packages/engine/lib/error/src/context.rs index 438cfd86e30..d5a061770b2 100644 --- a/packages/engine/lib/error/src/context.rs +++ b/packages/engine/lib/error/src/context.rs @@ -1,7 +1,9 @@ use core::fmt; #[cfg(nightly)] -use crate::provider::{Demand, Provider}; +use crate::provider::Demand; +#[cfg(all(nightly, any(feature = "std", feature = "spantrace")))] +use crate::provider::Provider; use crate::Report; /// Defines the current context of a [`Report`]. @@ -88,7 +90,7 @@ where // We can't implement `Provider` on Context as `Error` will implement `Provider` and `Context` will // be implemented on `Error`. For `request`ing a type from `Context`, we need a `Provider` // implementation however. -#[cfg(nightly)] +#[cfg(all(nightly, any(feature = "std", feature = "spantrace")))] pub fn temporary_provider(context: &impl Context) -> impl Provider + '_ { struct ProviderImpl<'a, C>(&'a C); impl Provider for ProviderImpl<'_, C> { diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index a33d79dcc69..c75d049daf7 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -8,11 +8,9 @@ use tracing_error::{SpanTrace, SpanTraceStatus}; use super::Frame; #[cfg(nightly)] -use crate::{ - context::temporary_provider, - iter::{RequestRef, RequestValue}, - provider::request_ref, -}; +use crate::iter::{RequestRef, RequestValue}; +#[cfg(all(nightly, any(feature = "std", feature = "spantrace")))] +use crate::{context::temporary_provider, provider::request_ref}; use crate::{iter::Frames, Context}; /// Contains a [`Frame`] stack consisting of [`Context`]s, attachments, and optionally From 069bb525a5cdc435827732ae3bc1337b201ff87d Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 6 Jun 2022 11:40:46 +0200 Subject: [PATCH 25/26] Clarify retrieving of the backtrace/spantrace --- packages/engine/lib/error/src/report.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index c75d049daf7..97cbbff2201 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -13,14 +13,7 @@ use crate::iter::{RequestRef, RequestValue}; use crate::{context::temporary_provider, provider::request_ref}; use crate::{iter::Frames, Context}; -/// Contains a [`Frame`] stack consisting of [`Context`]s, attachments, and optionally -/// a [`Backtrace`] and a [`SpanTrace`]. -/// -/// If the root [`Frame`] contains a [`Backtrace`]/[`SpanTrace`], these are used, otherwise they -/// are eventually captured. To enable capturing of the backtrace, make sure `RUST_BACKTRACE` or -/// `RUST_LIB_BACKTRACE` is set according to the [`Backtrace` documentation][`Backtrace`]. To enable -/// capturing of the span trace, an [`ErrorLayer`] has to be enabled. Please also look at the -/// [Feature Flags] section. +/// Contains a [`Frame`] stack consisting of [`Context`]s and attachments. /// /// Attachments can be added by using [`attach()`]. The [`Frame`] stack can be iterated by using /// [`frames()`]. @@ -31,6 +24,14 @@ use crate::{iter::Frames, Context}; /// to provide more context information than only a display message. This information can then be /// retrieved by calling [`request_ref()`] or [`request_value()`]. /// +/// `Report` is able to provide a [`Backtrace`] and a [`SpanTrace`], which can be retrieved by +/// calling [`backtrace()`] or [`span_trace()`] respectively. If the root context provides a +/// [`Backtrace`] or a [`SpanTrace`], those are returned, otherwise, if configured, they are tried +/// to be captured when creating a `Report`. To enable capturing of the backtrace, make sure +/// `RUST_BACKTRACE` or `RUST_LIB_BACKTRACE` is set according to the +/// [`Backtrace` documentation][`Backtrace`]. To enable capturing of the span trace, an +/// [`ErrorLayer`] has to be enabled. Please also see the [Feature Flags] section. +/// /// [`Backtrace`]: std::backtrace::Backtrace /// [`SpanTrace`]: tracing_error::SpanTrace /// [`ErrorLayer`]: tracing_error::ErrorLayer @@ -40,6 +41,8 @@ use crate::{iter::Frames, Context}; /// [`change_context()`]: Self::change_context /// [`request_ref()`]: Self::request_ref /// [`request_value()`]: Self::request_value +/// [`backtrace()`]: Self::backtrace +/// [`span_trace()`]: Self::span_trace /// [Feature Flags]: index.html#feature-flags /// /// # Examples From 7e5d08c9d55b3729e7f61e17c2ce69ac55fe330c Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 6 Jun 2022 11:46:55 +0200 Subject: [PATCH 26/26] Add note on backtrace and spantrace to `Report::new` --- packages/engine/lib/error/src/report.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/engine/lib/error/src/report.rs b/packages/engine/lib/error/src/report.rs index 97cbbff2201..8200e1e2a11 100644 --- a/packages/engine/lib/error/src/report.rs +++ b/packages/engine/lib/error/src/report.rs @@ -24,6 +24,8 @@ use crate::{iter::Frames, Context}; /// to provide more context information than only a display message. This information can then be /// retrieved by calling [`request_ref()`] or [`request_value()`]. /// +/// ## `Backtrace` and `SpanTrace` +/// /// `Report` is able to provide a [`Backtrace`] and a [`SpanTrace`], which can be retrieved by /// calling [`backtrace()`] or [`span_trace()`] respectively. If the root context provides a /// [`Backtrace`] or a [`SpanTrace`], those are returned, otherwise, if configured, they are tried @@ -32,8 +34,6 @@ use crate::{iter::Frames, Context}; /// [`Backtrace` documentation][`Backtrace`]. To enable capturing of the span trace, an /// [`ErrorLayer`] has to be enabled. Please also see the [Feature Flags] section. /// -/// [`Backtrace`]: std::backtrace::Backtrace -/// [`SpanTrace`]: tracing_error::SpanTrace /// [`ErrorLayer`]: tracing_error::ErrorLayer /// [`attach()`]: Self::attach /// [`new()`]: Self::new @@ -167,6 +167,12 @@ impl Report { } /// Creates a new `Report` from a provided scope. + /// + /// If `context` does not provide [`Backtrace`]/[`SpanTrace`] thy are attempted to be + /// captured. Please see the [`Backtrace` and `SpanTrace` section] of the `Report` + /// documentation for more information. + /// + /// [`Backtrace` and `SpanTrace` section]: #backtrace-and-spantrace #[track_caller] pub fn new(context: T) -> Self where