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

Call exit/panic in Fatal/Panic log events even if the logging is disabled for this event #393

Merged
merged 1 commit into from Jan 4, 2022

Conversation

a-canya
Copy link
Contributor

@a-canya a-canya commented Jan 4, 2022

Issue #392

@rs
Copy link
Owner

rs commented Jan 4, 2022

Looks good. Keep in mind the panic won't have the message either.

@rs
Copy link
Owner

rs commented Jan 4, 2022

One way to workaround the issue of panic with no message would be to change the Event's msg method to always call done even if the event is nil. The nil check would have to be moved from Msg/Msgf to msg.

@rs
Copy link
Owner

rs commented Jan 4, 2022

Ignore my previous message, you can't have a nil event and e.done set…

@rs rs merged commit 3efdd82 into rs:master Jan 4, 2022
@a-canya
Copy link
Contributor Author

a-canya commented Jan 4, 2022

That's true... The only workaround I can think of is to make the event be non-nil when it is disabled, and instead have an extra bool field in the struct to indicate that the event is disabled. Then apart of nil checks we would also need to check that the event is not disabled. This means more changes and I'm not sure what other implicactions it might have.

@rs
Copy link
Owner

rs commented Jan 4, 2022

True, I'm not a big fan of such large change. Let's see if this trips more people as it's a bit of an edge case.

pablitoc pushed a commit to nxcr-org/zerolog that referenced this pull request Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants