Skip to content
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 by Bors] - improve error messages for render graph runner #3930

Conversation

dataphract
Copy link
Member

Objective

Currently, errors in the render graph runner are exposed via a Result::unwrap() panic message, which dumps the debug representation of the error.

Solution

This PR updates render_system to log the chain of errors, followed by an explicit panic:

ERROR bevy_render::renderer: Error running render graph:
ERROR bevy_render::renderer: > encountered an error when running a sub-graph
ERROR bevy_render::renderer: > tried to pass inputs to sub-graph "outline_graph", which has no input slots
thread 'main' panicked at 'Error running render graph: encountered an error when running a sub-graph', /[redacted]/bevy/crates/bevy_render/src/renderer/mod.rs:44:9

Some errors' Display impls (via thiserror) have also been updated to provide more detail about the cause of the error.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Feb 13, 2022
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Feb 13, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

This is a good change. Just a consistency nit to clean up.

MissingSubGraph(Cow<'static, str>),
#[error("passed in inputs, but this sub-graph doesn't have any")]
#[error("attempted to pass inputs to sub-graph \"{0}\", which has no input slots")]
Copy link
Member

Choose a reason for hiding this comment

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

These are sometimes `, sometimes " and sometimes nothing at all. I would prefer ` everywhere, but would like consistency regardless.

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 14, 2022
@mockersf
Copy link
Member

mockersf commented Mar 5, 2022

@dataphract could you update this on current main?

@dataphract dataphract force-pushed the fix/missing-subgraph-input-diagnostic branch from bb40f71 to a661a5a Compare March 7, 2022 02:33
@mockersf
Copy link
Member

mockersf commented Mar 7, 2022

bors r+

bors bot pushed a commit that referenced this pull request Mar 7, 2022
# Objective

Currently, errors in the render graph runner are exposed via a `Result::unwrap()` panic message, which dumps the debug representation of the error.

## Solution

This PR updates `render_system` to log the chain of errors, followed by an explicit panic:

```
ERROR bevy_render::renderer: Error running render graph:
ERROR bevy_render::renderer: > encountered an error when running a sub-graph
ERROR bevy_render::renderer: > tried to pass inputs to sub-graph "outline_graph", which has no input slots
thread 'main' panicked at 'Error running render graph: encountered an error when running a sub-graph', /[redacted]/bevy/crates/bevy_render/src/renderer/mod.rs:44:9
```

Some errors' `Display` impls (via `thiserror`) have also been updated to provide more detail about the cause of the error.
@bors bors bot changed the title improve error messages for render graph runner [Merged by Bors] - improve error messages for render graph runner Mar 7, 2022
@bors bors bot closed this Mar 7, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
# Objective

Currently, errors in the render graph runner are exposed via a `Result::unwrap()` panic message, which dumps the debug representation of the error.

## Solution

This PR updates `render_system` to log the chain of errors, followed by an explicit panic:

```
ERROR bevy_render::renderer: Error running render graph:
ERROR bevy_render::renderer: > encountered an error when running a sub-graph
ERROR bevy_render::renderer: > tried to pass inputs to sub-graph "outline_graph", which has no input slots
thread 'main' panicked at 'Error running render graph: encountered an error when running a sub-graph', /[redacted]/bevy/crates/bevy_render/src/renderer/mod.rs:44:9
```

Some errors' `Display` impls (via `thiserror`) have also been updated to provide more detail about the cause of the error.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

Currently, errors in the render graph runner are exposed via a `Result::unwrap()` panic message, which dumps the debug representation of the error.

## Solution

This PR updates `render_system` to log the chain of errors, followed by an explicit panic:

```
ERROR bevy_render::renderer: Error running render graph:
ERROR bevy_render::renderer: > encountered an error when running a sub-graph
ERROR bevy_render::renderer: > tried to pass inputs to sub-graph "outline_graph", which has no input slots
thread 'main' panicked at 'Error running render graph: encountered an error when running a sub-graph', /[redacted]/bevy/crates/bevy_render/src/renderer/mod.rs:44:9
```

Some errors' `Display` impls (via `thiserror`) have also been updated to provide more detail about the cause of the error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants