Skip to content

Use Display instead of Debug in the default error handler #18629

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 45 additions & 39 deletions crates/bevy_ecs/src/error/bevy_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,48 +34,11 @@ impl BevyError {
pub fn downcast_ref<E: Error + 'static>(&self) -> Option<&E> {
self.inner.error.downcast_ref::<E>()
}
}

/// This type exists (rather than having a `BevyError(Box<dyn InnerBevyError)`) to make [`BevyError`] use a "thin pointer" instead of
/// a "fat pointer", which reduces the size of our Result by a usize. This does introduce an extra indirection, but error handling is a "cold path".
/// We don't need to optimize it to that degree.
/// PERF: We could probably have the best of both worlds with a "custom vtable" impl, but thats not a huge priority right now and the code simplicity
/// of the current impl is nice.
struct InnerBevyError {
error: Box<dyn Error + Send + Sync + 'static>,
#[cfg(feature = "backtrace")]
backtrace: std::backtrace::Backtrace,
}

// NOTE: writing the impl this way gives us From<&str> ... nice!
impl<E> From<E> for BevyError
where
Box<dyn Error + Send + Sync + 'static>: From<E>,
{
#[cold]
fn from(error: E) -> Self {
BevyError {
inner: Box::new(InnerBevyError {
error: error.into(),
#[cfg(feature = "backtrace")]
backtrace: std::backtrace::Backtrace::capture(),
}),
}
}
}

impl Display for BevyError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
writeln!(f, "{}", self.inner.error)?;
Ok(())
}
}

impl Debug for BevyError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
writeln!(f, "{:?}", self.inner.error)?;
fn format_backtrace(&self, _f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
#[cfg(feature = "backtrace")]
{
let f = _f;
let backtrace = &self.inner.backtrace;
if let std::backtrace::BacktraceStatus::Captured = backtrace.status() {
let full_backtrace = std::env::var("BEVY_BACKTRACE").is_ok_and(|val| val == "full");
Expand Down Expand Up @@ -123,7 +86,50 @@ impl Debug for BevyError {
}
}
}
Ok(())
}
}

/// This type exists (rather than having a `BevyError(Box<dyn InnerBevyError)`) to make [`BevyError`] use a "thin pointer" instead of
/// a "fat pointer", which reduces the size of our Result by a usize. This does introduce an extra indirection, but error handling is a "cold path".
/// We don't need to optimize it to that degree.
/// PERF: We could probably have the best of both worlds with a "custom vtable" impl, but thats not a huge priority right now and the code simplicity
/// of the current impl is nice.
struct InnerBevyError {
error: Box<dyn Error + Send + Sync + 'static>,
#[cfg(feature = "backtrace")]
backtrace: std::backtrace::Backtrace,
}

// NOTE: writing the impl this way gives us From<&str> ... nice!
impl<E> From<E> for BevyError
where
Box<dyn Error + Send + Sync + 'static>: From<E>,
{
#[cold]
fn from(error: E) -> Self {
BevyError {
inner: Box::new(InnerBevyError {
error: error.into(),
#[cfg(feature = "backtrace")]
backtrace: std::backtrace::Backtrace::capture(),
}),
}
}
}

impl Display for BevyError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
writeln!(f, "{}", self.inner.error)?;
self.format_backtrace(f)?;
Ok(())
}
}

impl Debug for BevyError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
writeln!(f, "{:?}", self.inner.error)?;
self.format_backtrace(f)?;
Ok(())
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/error/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub fn default_error_handler() -> fn(BevyError, ErrorContext) {
macro_rules! inner {
($call:path, $e:ident, $c:ident) => {
$call!(
"Encountered an error in {} `{}`: {:?}",
"Encountered an error in {} `{}`: {}",
$c.kind(),
$c.name(),
$e
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2959,7 +2959,7 @@ mod tests {
}

#[test]
#[should_panic = "Encountered an error in system `bevy_ecs::system::system_param::tests::missing_resource_error::res_system`: SystemParamValidationError { skipped: false, message: \"Resource does not exist\", param: \"bevy_ecs::change_detection::Res<bevy_ecs::system::system_param::tests::missing_resource_error::MissingResource>\" }"]
#[should_panic = "Encountered an error in system `bevy_ecs::system::system_param::tests::missing_resource_error::res_system`: Parameter `Res<MissingResource>` failed validation: Resource does not exist"]
fn missing_resource_error() {
#[derive(Resource)]
pub struct MissingResource;
Expand Down
2 changes: 1 addition & 1 deletion examples/ecs/error_handling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ fn main() {

// If we run the app, we'll see the following output at startup:
//
// WARN Encountered an error in system `fallible_systems::failing_system`: "Resource not initialized"
// WARN Encountered an error in system `fallible_systems::failing_system`: Resource not initialized
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this line of documentation because we'll no longer print the quotes, but the example doesn't actually output this error! The sole use of failing_system is used with pipe to handle the result.

// ERROR fallible_systems::failing_system failed: Resource not initialized
// INFO captured error: Resource not initialized
app.run();
Expand Down