logger
logger copied to clipboard
Closed LoggerSet behaves confusingly
Of course, LoggerSet
isn't supposed to be usable after it was closed (with rmLoggerSet
), however it should fail in a way that doesn't cause confusion.
I use this utility to test the current behaviour:
withClosedLogger f = do
logger <- newStdoutLoggerSet defaultBufSize
rmLoggerSet logger
f logger
Trying to flush/close an already closed logger:
> withClosedLogger flushLoggerSet
*** Exception: thread blocked indefinitely in an MVar operation
> withClosedLogger rmLoggerSet
*** Exception: thread blocked indefinitely in an MVar operation
That is if you are lucky. Actually it usually takes it some time blocking after which it throws an exception (or never). It took me some time to figure out that our test suite silently hangs because it tries to close the logger twice.
Trying to write to an already closed logger:
> withClosedLogger (`pushLogStrLn` toLogStr "nope")
>
That is, nothing happens.
Better behavior
To avoid confusion all operations on a closed LoggerSet
should be either a no-op (do nothing just return) or an instant, deterministic failure (throwIO LoggerSetIsClosed
). I think that:
- Flushing a closed logger should be a no-op. Conceptually we are asked to flush buffered messages and there's nothing to flush, so we should just return.
- Closing a closed logger should be a no-op or a failure. Making it an explicit failure would bring a potential lifecycle issue to user's attention. On the other hand it would hardly hurt if the disposable resources are disposed twice. In many cases one could want to do that from several places to "really make sure" that their resources are released. I'd prefer if repeated
rmLoggerSet
would just silently return - Writing to a closed logger should be a failure. If there's a lifecycle problem and something is logged after the logger has been closed it shouldn't silently succeed loosing the important messages. It should throw, so that this problem is discovered and fixed before those messages are actually needed.
Uh, just got bitten by this. First time ever hoping for broader linear type adoption.