Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jesseyoungmann opened this issue Jul 17, 2024 · 1 comment · Fixed by #290
Closed

Comments

@jesseyoungmann
Copy link

jesseyoungmann commented Jul 17, 2024

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:
f7d54bf#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
Copy link

vovimayhem commented Aug 20, 2024

@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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants