search

reidmorrison / semantic_logger

Semantic Logger is a feature rich logging framework, and replacement for existing Ruby & Rails loggers.
https://logger.rocketjob.io/
Apache License 2.0
861 stars 119 forks source link

Semantic Logger 4.16.0 introduces a bug breaking SemanticLogger::Appenders#close #291

Closed jesseyoungmann closed 1 week ago

jesseyoungmann commented 1 month ago

Calling config.semantic_logger.clear_appenders! raises an error of Exception: NameError: undefined local variable or method 'appenders' after upgrading from semantic_logger 4.15.0 to 4.16.0, this appears to be caused by this commit: https://github.com/reidmorrison/semantic_logger/commit/f7d54bf21b6900f334155882ffd6ab287ffa58f5#diff-bfe54b7aa0c85191d839ffac6deaae7617c6cc1d8f241a74815aa7ad587fb537R52 Specifically, changing it from appenders << appender to closed_appenders << appender resolved the issue for me.

Environment

I'm working with ruby 3.2.2, rails 7.1.3.4, and rails_semantic_logger 4.17.0.

Expected Behavior

I'm attempting to replace the logger in config/environments/development.rb with a custom logger, using:

  config.rails_semantic_logger.format = DevelopmentLogFormatter.new
  config.semantic_logger.clear_appenders!
  config.semantic_logger.add_appender(io: $stdout, formatter: config.rails_semantic_logger.format)

This should not raise an error, should remove the existing logger, and should add in my custom logger formatter.

Actual Behavior

Instead, I receive this error:

2024-07-17 09:23:17.723309 E [46080:SemanticLogger::Appenders] SemanticLogger::Appenders -- Failed to close appender: SemanticLogger::Appender::IO -- Exception: NameError: undefined local variable or method `appenders' for [#<SemanticLogger::Appender::IO:0x000000010d1d0f20 @io=#<IO:<STDOUT>>, @formatter=#<SemanticLogger::Formatters::Json:0x000000010d4beb60 @time_key=:timestamp, @time_format=:iso_8601, @log_host=true, @log_application=true, @log_environment=true, @precision=6>, @application=nil, @environment=nil, @host=nil, @metrics=false, @filter=nil, @name="SemanticLogger::Appender::IO", @level_index=nil, @level=nil>]:SemanticLogger::Appenders
/Users/jesse/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/semantic_logger-4.16.0/lib/semantic_logger/appenders.rb:52:in `block in close'
/Users/jesse/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/semantic_logger-4.16.0/lib/semantic_logger/appenders.rb:50:in `each'
/Users/jesse/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/semantic_logger-4.16.0/lib/semantic_logger/appenders.rb:50:in `close'
/Users/jesse/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/semantic_logger-4.16.0/lib/semantic_logger/appender/async.rb:173:in `process_message'
/Users/jesse/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/semantic_logger-4.16.0/lib/semantic_logger/appender/async.rb:160:in `process_messages'
/Users/jesse/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/semantic_logger-4.16.0/lib/semantic_logger/appender/async.rb:121:in `process'
/Users/jesse/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/semantic_logger-4.16.0/lib/semantic_logger/appender/async.rb:77:in `block in thread'

It then fails to add my development replacement logger:

2024-07-17 09:23:17.723508 W [46080:4520] SemanticLogger::Appenders -- Ignoring attempt to add a second console appender: SemanticLogger::Appender::IO since it would result in duplicate console output.

Pull Request

Happy to make a pull request if the fix I mention is correct and if that's easier for you, let me know. Thanks very much for all the work on this library!

vovimayhem commented 3 weeks ago

@jesseyoungmann Based on the diff, to me it looks like an honest mistake, and closed_appenders should be the correct variable name, and renaming it the only fix... given I'm in a bit of a hurry, I'll just post a PR with your suggestion, so there's a version we can refer to on our Gemfile in our apps :)