feat(bevy_ecs): add Severity metadata to BevyError

[syszery]

Objective

As outlined in #20561, Bevy's default error handler currently panics on most errors. In some cases, this is overly strict—e.g., inserting a component into a despawned entity is harmless and could be handled more gracefully in library code.

This PR implements the idea suggested by @Freyja-moth: each BevyError now carries a Severity, and the ResultSeverityExt trait allows overriding it. These changes lay the foundation for error handlers that can log, ignore, or panic based on severity.

Happy to adjust based on feedback.

Solution

• Added Severity enum.
• Updated InnerBevyError to store a Severity, defaulting to Severity::Critical to preserve existing panic-on-error behavior.
• BevyError exposes severity() and with_severity() methods.
• ResultSeverityExt provides ergonomic overriding of error severity on Result<T, E>.

Testing

• Tests in the crate bevy_ecs pass (with the debug feature enabled); existing "should panic" tests continue to behave as expected.

Future TODOs

• This primarily sets up the API; proper handling of severity in the default handler is left for future work.
• Feedback on the API design and suggestions for integrating severity into the default handler are welcome.

[Freyja-moth]

To add to the future work section, I do feel like it's a good idea to also add the ErrorSeverity trait as @alice-i-cecile outlined, as otherwise we've only solved the issue for error types that user interact with.

With just the error_severity field implemented errors that occur in commands (such as inserting an component into a despawned entity as you noted) are still just as strict unless the unwieldy queue_handled method is used.

[NthTensor]

I would prefer to have Debug here as well, for errors that are lower severity than warnings but may still need to be surfaced during debugging.

[NthTensor]

This is pretty much what I was envisioning, nice work! Obviously there's more to do, but I'm happy with this.

[syszery]

Thanks for the feedback!

@NthTensor: I’ll add Debug to the severity levels in a follow-up commit.

@Freyja-moth: I understand your concerns—thanks for pointing this out! The preferred default for error severity should be None, right? And then the idea is to add the option to override errors not directly controlled by the user with the trait suggested by @alice-i-cecile?

[Freyja-moth]

The idea is to have the trait be the default with the option to override it

trait ErrorSeverity: Error {
    fn severity(&self) -> SeverityLevel;
}

pub struct BevyError {
    error: Box<dyn ErrorSeverity>,
    severity: Option<SeverityLevel>
}
impl BevyError {
    pub fn new(error: impl ErrorSeverity) -> Self {
        Self {
            error: Box::new(error),
            severity: None,
        }
    }
    pub fn with_severity(&mut self, severity: SeverityLevel) {
        self.severity.replace(severity);
    }
    pub fn severity(&self) -> SeverityLevel {
        self.severity.unwrap_or(self.error.severity())
    }
}

[syszery]

I didn’t modify the BevyError directly because of the comment above InnerBevyError. I understand that if we use a dyn Trait or add the severity field directly to BevyError, we would lose the "thin pointer" benefit.

When thinking more about using Option<Severity>, I‘m in doubt if this really solves the issue. If the severity is None, it leaves the handler in an ambiguous state.

The problem seems to be: What is the right default? With Critical as the default, we ensure that errors are always visible and addressed right away—nothing is left unhandled. This is why I thought that a clear, strong default might be safe. But it is also extreme and the other (equally extreme) option would be Ignore.

[Freyja-moth]

Yes sorry, I'm not familiar with the internals of BevyError. In this case it would be updating InnerBevyError to look like the BevyError shown above.

When thinking more about using Option, I‘m in doubt if this really solves the issue. If the severity is None, it leaves the handler in an ambiguous state.

It wouldn't, as the error severity would default to the error severity as specified by the trait.

To give an example

pub struct EntityDoesNotMatchError;
impl ErrorSeverity for EntityDoesNotMatchError {
    fn severity() -> SeverityLevel {
        SeverityLevel::Critical
    }
}

fn tester() {
    let mut result = Err(BevyError::new(EntityDoesNotMatchError)); // `BevyError::severity` currently returns SeverityLevel::Critical since `EntityDoesNotMatchError::severity` returns `SeverityLevel::Critical` and `InnerBevyError.severity` is `None`

    result.with_severity(SeverityLevel::Ignore); // `BevyError::severity` currently returns SeverityLevel::Ignore, since `InnerBevyError.severity` is now no longer `None` and is used instead of `EntityDoesNotMatchError::severity`
}

[Freyja-moth]

Looking into it further doing this would introduce issues with non bevy errors as they wouldn't be able to be stored.

There's currently no way to do what I'd suggested while allowing non bevy errors, but once specialization is added we should be able to add a From<E> for InnerBevyError where E: ErrorSeverity and set the severity field to default to the severity as specified by the trait

[NthTensor]

We should not design anything that assumes specialization, it may never be implemented. I'll try to review the this discussion in more detail this week.

[syszery]

Hi @NthTensor, just checking in on this PR. Do you have any feedback or a decision on the changes?

[Person-93]

Should it implement Ord?

[AlephCubed]

I agree with the previous comments on implementing Ord and a Debug case for Severity. But other then that I think it looks good.