zerolog icon indicating copy to clipboard operation
zerolog copied to clipboard

Fatal() does os.Exit(1) when Nop() log is disabled

Open lbergesio opened this issue 2 years ago • 5 comments

I think this is an error, It was discovered after upgrading from 1.26.1 to 1.28.0:

Fatal() here https://github.com/rs/zerolog/blob/d894f123bc5c2a887c95e90218b9410563141d67/log.go#L367

calls newEvent https://github.com/rs/zerolog/blob/d894f123bc5c2a887c95e90218b9410563141d67/log.go#L444

with the done paremeter being os.Exit(1) so here even the log is disabled, done is executed which was not like this before:

https://github.com/rs/zerolog/blob/d894f123bc5c2a887c95e90218b9410563141d67/log.go#L448

lbergesio avatar Sep 08 '22 10:09 lbergesio

This has been introduced in 1.27.0, is this the new expected behaviour?

lbergesio avatar Sep 08 '22 10:09 lbergesio

This is the expected behavior though. Disabling logging should only disable the logging and not change the flow of you code.

rs avatar Sep 10 '22 13:09 rs

The problem to me is that the logger I am using is Nop() and it reads: Nop returns a disabled logger for which all operation are no-op. So I would expect that even Fatal() is not op for it, meaning not exiting my program. Does it make sense?

lbergesio avatar Sep 10 '22 14:09 lbergesio

If you call Fatal on a logger in your code, you are not supposed to know the logger is disabled, and the code logic is not supposed to pass the Fatal statement.

rs avatar Sep 10 '22 18:09 rs

Agree. My point is more in the semantics of "no op" not if in the log is disabled or not. But if according to you a nop logger should be exiting ok, i will just modify my code that was using 1.26.1. this can be closed in such case. Thanks

lbergesio avatar Sep 10 '22 18:09 lbergesio