What should we do about `catch_errors` for sinks?

[oxinabox]

SimpleLogger (and thus FileLogger and DateRotatingLogger) set it to false. ConsoleLogger sets it to true FormatLogger sets it to true

Originally posted by @fredrikekre in https://github.com/oxinabox/LoggingExtras.jl/pull/35#discussion_r558437994

@fredrikekre

Did you have any thoughts on this? Probably don't matter much, only captures message construction errors and those should not happen often.

@oxinabox:

Yeah I am not sure. It seemed fine to match ConsoleLogger. On the onehand it is hard to debug exceptions that are not thrown On the other hand your logger failing should not take down your process.

Maybe we should solve it compositionally. Make all sinks set it to false, then add a LoggingExceptionCatchingLogger that wrapps a logger and does catch_exceptions.

@fredrikekre

Yea that could be useful for also catching errors in handle_message (xref https://julialang.zulipchat.com/#narrow/stream/236331-logging/topic/Logger.20errors/near/222839637)

[fredrikekre]

The current behaviour/meaning of catch_exceptions is just about whether to pass a log message or throw if the message itself can not be constructed. It feels like loggers where catch_exceptions(logger) == true should also swallow exceptions that comes from actually delivering the message (handle_message). But until that changes in Base there could be a SafetyLogger or FailSafeLogger that just puts try around everything. Quick draft:

struct SafetyLogger{L <: AbstractLogger} <: AbstractLogger                                   
    logger::L                                                                                
end                                                                                          
function handle_message(logger::SafetyLogger, args...; kwargs...)                            
    try                                                                                      
        handle_message(logger.logger, args...; kwargs...)                                    
    catch err                                                                                
        try                                                                                  
            println(stderr, "failed so print err and backtrace to stderr")                                                        
        catch err                                                                            
        end                                                                                  
    end                                                                                      
end                                                                                          
shouldlog(logger::SafetyLogger, args...) = shouldlog(logger.logger, args...)                 
min_enabled_level(logger::SafetyLogger) = min_enabled_level(logger.logger)                   
catch_exceptions(logger::SafetyLogger) = true

[oxinabox]

Leaning towards saying it should generally be set to true to match ConsoleLogger that's what people are most familar with.