rs / zerolog

Zero Allocation JSON Logger
MIT License
10.62k stars 572 forks source link

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

Closed a-canya closed 2 years ago

a-canya commented 2 years ago

Issue #392

rs commented 2 years ago

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

rs commented 2 years ago

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 commented 2 years ago

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

a-canya commented 2 years ago

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 commented 2 years ago

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.