Panic when log filters are incorrectly configured #13850
Labels
A-Diagnostics
Logging, crash handling, error reporting and performance analysis
C-Bug
An unexpected or incorrect behavior
C-Usability
A simple quality-of-life change that makes Bevy easier to use
D-Trivial
Nice and easy! A great choice to get started with Bevy
S-Ready-For-Implementation
This issue is ready for an implementation PR. Go for it!
Comments
rparrett
added
C-Bug
alice-i-cecile
added
D-Trivial
github-merge-queue bot
pushed a commit
that referenced
this issue
Jun 19, 2024
# Objective This PR aims to improve error handling for log filters. Closes #13850 ## Solution I changed the parsing of LogPlugin its filter to lossy, so that it prints the directives with an error but does not skip them. I decided on letting it gracefully handle the error instead of panicking to be consistent with the parsing from an environment variable that it tries to do before parsing it from the LogPlugin filter. If the user decides to specify the filter by an environment variable, it would silently fail and default to the LogPlugin filter value. It now prints an error before defaulting to the LogPlugin filter value. Unfortunately, I could not try and loosely set the filter from the environment variable because the `tracing-subscriber` module does not expose the function it uses to get the environment variable, and I would rather not copy its code. We may want to check if the maintainers are open to exposing the method. ## Testing Consider the following bevy app, where the second of the 3 filters is invalid: ``` use bevy::{log::LogPlugin, prelude::*}; fn main() { App::new().add_plugins(DefaultPlugins .set(LogPlugin { filter: "wgpu=error,my_package=invalid_log_level,naga=warn".into(), ..default() }) ).run(); } ``` In the previous situation, it would panic with a non-descriptive error: "called `Result::unwrap()` on an `Err` value: ParseError { kind: Other(None) }", while only 1 of the 3 filters is invalid. When running `cargo run`, it will now use the two valid filters and print an error on the invalid filter. > ignoring `my_package=invalid_log_level`: invalid filter directive This error comes from `tracing-subscriber` and cannot be altered as far as I can see. To test setting the log filter through an environment variable, you can use `RUST_LOG="wgpu=error,my_package=invalid_log_level,naga=warn" cargo run` to run your app. In the previous situation it would silently fail and use the LogPlugin filter. It will now print an error before using the LogPlugin filter. > LogPlugin failed to parse filter from env: invalid filter directive ## Changelog - Added warning when using invalid filter in the RUST_LOG environment variable - Prevent the app from panicking when setting an invalid LogPlugin filter --------- Co-authored-by: Luc Drenth <luc.drenth@ing.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bevy version
0.14.0-rc.2, but this seems like a very old thingWhat you did
Made a typo while configuring LogPlugin:
What went wrong
Encountered a panic on an unwrap in Bevy here: https://github.com/bevyengine/bevy/blob/main/crates/bevy_log/src/lib.rs#L170
Additional information
This is an error on the user's part and I'm not sure that we need to handle the error gracefully, but the error message presented to the user could be much more helpful.
The text was updated successfully, but these errors were encountered: