diff --git a/_release-content/release-notes/bevy_error_context.md b/_release-content/release-notes/bevy_error_context.md new file mode 100644 index 0000000000000..2959b25ad3177 --- /dev/null +++ b/_release-content/release-notes/bevy_error_context.md @@ -0,0 +1,50 @@ +--- +title: Bevy Error Context Messages +authors: ["@cookie1170"] +pull_requests: [24528] +--- + +Similar to the popular `anyhow` crate, `BevyError` now provides an ergonomic way to attach extra context to an error using the `context` method, +which also allows creating a `Result` from an `Option`. + +This makes it easier to trace back errors with human-readable messages without looking at verbose backtraces. + +```rs +fn fallible() -> Result<(), BevyError> { + // This produces the error message `Failed to parse number: invalid digit found in string` + let parsed: usize = "I am not a number" + .parse() + .context("Failed to parse number")?; + + Ok(()) +} +``` + +`with_context` may be used to produce the error string with a closure instead. + +If multiple `context`s were used on the same `BevyError`, they're enumerated below: + +```rs +fn fallible() -> Result { + let path = "package.json"; + let package = std::fs::read_to_string(path) + .with_context(|| format!("Failed to read {path}"))?; + + serde_json::parse(&package)? +} + +fn uses_fallible() -> Result<(), BevyError> { + let package = fallible().context("Failed to parse package.json")?; + // Use `package`... +} +``` + +Will produce the following error if `package.json` is missing: + +```rs +Failed to parse package.json + +Caused by: + Failed to read package.json + No such file or directory (os error 2) +``` diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index c9f06796beaca..0ac6044d4c1f9 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -1,4 +1,4 @@ -use alloc::boxed::Box; +use alloc::{boxed::Box, string::ToString}; use core::{ error::Error, fmt::{Debug, Display}, @@ -106,6 +106,7 @@ impl BevyError { inner: Box::new(InnerBevyError { error: error.into(), severity, + context: alloc::vec![], #[cfg(feature = "backtrace")] backtrace, }), @@ -201,7 +202,7 @@ impl BevyError { // TODO: Cache let full_backtrace = std::env::var("BEVY_BACKTRACE").is_ok_and(|val| val == "full"); - let backtrace_str = alloc::string::ToString::to_string(backtrace); + let backtrace_str = backtrace.to_string(); let mut skip_next_location_line = false; for line in backtrace_str.split('\n') { if !full_backtrace { @@ -255,6 +256,7 @@ impl BevyError { /// of the current impl is nice. struct InnerBevyError { error: Box, + context: alloc::vec::Vec, severity: Severity, #[cfg(feature = "backtrace")] backtrace: std::backtrace::Backtrace, @@ -311,7 +313,7 @@ impl BevyError { } /// Extension methods for annotating errors with a [`Severity`]. -pub trait ResultSeverityExt { +pub trait ResultSeverityExt: Sized { /// Overrides the [`Severity`] of the error if this result is `Err`. /// This does not change control flow; it only annotates the error. /// @@ -362,6 +364,48 @@ pub trait ResultSeverityExt { /// /// If you don't need to inspect the error, use [`Result::with_severity`](ResultSeverityExt::with_severity) fn map_severity(self, f: impl FnOnce(&E) -> Severity) -> Result; + + /// Overrides the severity of the error with [`Severity::Ignore`]. See [`Result::with_severity`] + /// + /// This is shorthand for `self.with_severity(Severity::Ignore)` + fn ignore(self) -> Result { + self.with_severity(Severity::Ignore) + } + + /// Overrides the severity of the error with [`Severity::Trace`]. See [`Result::with_severity`] + /// + /// This is shorthand for `self.with_severity(Severity::Trace)` + fn trace(self) -> Result { + self.with_severity(Severity::Trace) + } + + /// Overrides the severity of the error with [`Severity::Info`]. See [`Result::with_severity`] + /// + /// This is shorthand for `self.with_severity(Severity::Info)` + fn info(self) -> Result { + self.with_severity(Severity::Info) + } + + /// Overrides the severity of the error with [`Severity::Warning`]. See [`Result::with_severity`] + /// + /// This is shorthand for `self.with_severity(Severity::Warning)` + fn warn(self) -> Result { + self.with_severity(Severity::Warning) + } + + /// Overrides the severity of the error with [`Severity::Error`]. See [`Result::with_severity`] + /// + /// This is shorthand for `self.with_severity(Severity::Error)` + fn error(self) -> Result { + self.with_severity(Severity::Error) + } + + /// Overrides the severity of the error with [`Severity::Panic`]. See [`Result::with_severity`] + /// + /// This is shorthand for `self.with_severity(Severity::Panic)` + fn panic(self) -> Result { + self.with_severity(Severity::Panic) + } } impl ResultSeverityExt for Result @@ -380,6 +424,84 @@ where } } +/// Extension methods for adding additional context messages to a [`BevyError`] +pub trait ContextExt: Sized { + /// Annotate the error with a context message. + /// + /// # Example + /// ``` + /// # use bevy_ecs::error::{BevyError, ContextExt}; + /// fn fallible() -> Result<(), BevyError> { + /// // Produces a `BevyError` with the message + /// // "failed to parse number: invalid digit found in string" + /// let _parsed: usize = "I am not a number" + /// .parse() + /// .context("failed to parse number")?; + /// + /// Ok(()) + /// } + /// ``` + fn context(self, context: C) -> Result + where + C: Display, + { + self.with_context(move || context) + } + + /// Annotate the error with a context message from a closure + /// + /// # Example + /// ``` + /// # use bevy_ecs::error::{BevyError, ContextExt}; + /// # use std::fs; + /// fn fallible() -> Result<(), BevyError> { + /// let path = "some_file.txt"; + /// let _message = fs::read_to_string(path) + /// .with_context(|| format!("failed to read {path}"))?; + /// + /// Ok(()) + /// } + /// ``` + fn with_context(self, context: impl FnOnce() -> C) -> Result + where + C: Display; +} +impl ContextExt for Result +where + E: Into, +{ + fn with_context(self, context: impl FnOnce() -> C) -> Result + where + C: Display, + { + match self { + Ok(v) => Ok(v), + Err(error) => { + let mut error = error.into(); + let message = context().to_string(); + error.inner.context.push(message); + Err(error) + } + } + } +} + +impl ContextExt for Option { + fn with_context(self, context: impl FnOnce() -> C) -> Result + where + C: Display, + { + match self { + Some(v) => Ok(v), + None => { + let message = context().to_string(); + + Err(message.into()) + } + } + } +} + // NOTE: writing the impl this way gives us From<&str> ... nice! impl From for BevyError where @@ -391,6 +513,7 @@ where inner: Box::new(InnerBevyError { error: error.into(), severity: Severity::Panic, + context: alloc::vec![], #[cfg(feature = "backtrace")] backtrace: std::backtrace::Backtrace::capture(), }), @@ -400,7 +523,25 @@ where impl Display for BevyError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - writeln!(f, "{}", self.inner.error)?; + match &self.inner.context { + context if context.is_empty() => writeln!(f, "{}", self.inner.error)?, + context if context.len() == 1 => { + writeln!(f, "{}: {}", context[0].trim(), self.inner.error)?; + } + context => { + // The most recent message is the last one in the `Vec` + // so we need to reverse the iterator + let error = self.inner.error.to_string(); + let mut reversed = context.iter().rev().chain(core::iter::once(&error)); + let first = reversed.next().unwrap().trim(); + + writeln!(f, "{first}\n\nCaused by:")?; + for message in reversed { + let message = message.trim(); + writeln!(f, "\t{message}")?; + } + } + } self.format_backtrace(f)?; Ok(()) } @@ -510,6 +651,8 @@ macro_rules! bail { #[cfg(test)] mod tests { use crate::error::BevyError; + use crate::error::ContextExt; + use alloc::string::ToString; #[test] #[cfg(not(miri))] // miri backtraces are weird @@ -649,4 +792,64 @@ mod tests { }); t(|| bail!("Format string {}", 1 + 2)); } + + #[test] + fn context() { + let empty = None::; + let as_result = empty.context("Didn't have anything!"); + assert!(as_result + .unwrap_err() + .to_string() + .starts_with("Didn't have anything!\n")); + + let err: Result = + Err(BevyError::new(crate::error::Severity::Debug, "Oh no!")); + let mut with_context = err.context("Failed"); + + assert!(with_context + .as_ref() + .unwrap_err() + .to_string() + .starts_with("Failed: Oh no!\n")); + + with_context = with_context.context("Something went wrong"); + assert!(with_context.unwrap_err().to_string().starts_with( + "Something went wrong + +Caused by: +\tFailed +\tOh no! +" + )); + } + + #[test] + fn context_downcasting() { + #[derive(Debug, PartialEq)] + struct Fun(i32); + + impl core::fmt::Display for Fun { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + core::fmt::Debug::fmt(&self, f) + } + } + impl core::error::Error for Fun {} + + let fun: Result = Err(Fun(1)); + let new_error = fun.context("Hello world!"); + + assert!(new_error.as_ref().unwrap_err().is::()); + assert_eq!( + new_error.as_ref().unwrap_err().downcast_ref::(), + Some(&Fun(1)) + ); + + let new_new_error = new_error.context("Hey there!"); + + assert!(new_new_error.as_ref().unwrap_err().is::()); + assert_eq!( + new_new_error.as_ref().unwrap_err().downcast_ref::(), + Some(&Fun(1)) + ); + } } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index dee0fc0aa0f24..4a050a7087d8c 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -74,7 +74,7 @@ pub mod prelude { children, component::Component, entity::{ContainsEntity, Entity, EntityMapper}, - error::{BevyError, Result, ResultSeverityExt, Severity}, + error::{BevyError, ContextExt, Result, ResultSeverityExt, Severity}, event::{EntityEvent, Event}, hierarchy::{ChildOf, ChildSpawner, ChildSpawnerCommands, Children}, lifecycle::{Add, Despawn, Discard, Insert, Remove, RemovedComponents},