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.

Sign up for GitHub

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

Jump to bottom

Panic in case of log initializiation error #4937

Closed
wants to merge 2 commits into from
Closed

Panic in case of log initializiation error #4937

wants to merge 2 commits into from

Conversation

allevo
Copy link

@allevo allevo commented Jun 5, 2022

Objective

Fix #4934

Solution

Add force parameter in order to handle better the expected behavior on error case.

@mockersf
Copy link
Member

mockersf commented Jun 5, 2022

as mentioned in the issue, it would be best to not add the LogPlugin several times instead of exposing an obscure configuration

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Diagnostics Logging, crash handling, error reporting and performance analysis labels Jun 5, 2022
@LoipesMas
Copy link
Contributor

LoipesMas commented Jun 8, 2022 •
edited
Loading

You could also test for panic when force=true with #[should_panic] attribute.
(But I agree that this PR isn't the best solution to the problem)

@alice-i-cecile
Copy link
Member

as mentioned in the issue, it would be best to not add the LogPlugin several times instead of exposing an obscure configuration

I agree with this. Closing this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Bug An unexpected or incorrect behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Avoid unwrap log initialization
4 participants
@allevo @mockersf @LoipesMas @alice-i-cecile