Skip to content

Improve BevyError ergonomics#24528

Open
cookie1170 wants to merge 9 commits into
bevyengine:mainfrom
cookie1170:bevy-error-ergonomics
Open

Improve BevyError ergonomics#24528
cookie1170 wants to merge 9 commits into
bevyengine:mainfrom
cookie1170:bevy-error-ergonomics

Conversation

@cookie1170
Copy link
Copy Markdown
Contributor

@cookie1170 cookie1170 commented Jun 4, 2026

Objective

Improve ergonomics when using BevyError

Solution

  • Add .info(), .warn(), etc. shorthand methods for .with_severity to ResultSeverityExt
  • Add a .context() extension method similar to the one from anyhow, which adds a "context" message to go along with the error. Also works on Option<T>!

Fixes #19714.

Testing

Added the context unit test to test messages produced by ContextError and a context_downcasting unit test to test that downcasting still works when using .context

Add a `context` method to `Result` and `Option` and some extra utilities to `ResultSeverityExt`
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events M-Release-Note Work that should be called out in the blog due to impact D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 4, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in ECS Jun 4, 2026
@alice-i-cecile alice-i-cecile added this to the 0.20 milestone Jun 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note.

Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes.

@alice-i-cecile alice-i-cecile self-requested a review June 4, 2026 05:11
@Shatur
Copy link
Copy Markdown
Contributor

Shatur commented Jun 4, 2026

Thank you for working on this! This should close #19714.

Comment thread crates/bevy_ecs/src/error/bevy_error.rs
Copy link
Copy Markdown
Contributor

@SpecificProtagonist SpecificProtagonist left a comment

Choose a reason for hiding this comment

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

Generally in favor of both the severity shorthands and the error context, though they could be done in separate PRs because they're independent. The fix to backtrace filtering also shouldn't be part of this PR.

}
}

impl<T> ContextExt<T> for Option<T> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems odd to me and might be a bit confusing to use. Attaching a context to an option turns the context into an error? And options work with .with_context but not with .with_severity (as map_severity can't work with options)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

having an Option that's None usually means something failed, i.e. getting an element from an array, so you can attach a context message to it to turn it into an error that can be displayed. anyhow does the same thing so i just copied it from there really :p
having with_severity on an Option means that it'll need to turn into an error with some message, but if you then use context on it now you have 2 messages (i.e. some context message: option is none), which is different from if you first used context and then with_severity (you'll just get some context message)

Comment thread crates/bevy_ecs/src/error/bevy_error.rs Outdated
Comment thread crates/bevy_ecs/src/error/bevy_error.rs Outdated
@alice-i-cecile
Copy link
Copy Markdown
Member

Generally in favor of both the severity shorthands and the error context, though they could be done in separate PRs because they're independent. The fix to backtrace filtering also shouldn't be part of this PR.

Agreed, I would prefer this split into three :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes M-Release-Note Work that should be called out in the blog due to impact S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

Add the ability to attach context to BevyError

5 participants