dynamoid icon indicating copy to clipboard operation
dynamoid copied to clipboard

Errors when Dynamoid.logger is nil

Open tobypinder opened this issue 5 years ago • 2 comments

It seems there is a potential bug when the logger is not configured. The context for how I inadvertently had no logger configured is trying to use Dynamoid alongside Ruby on Jets, however I do not know how important this is. I had a configuration block

Dynamoid.configure do |config|
  config.namespace     = Jets.application.config.dynamoid.namespace
  config.endpoint      = Jets.application.config.dynamoid.url
  config.region        = ENV['REGION']
  config.access_key    = ENV['ACCESS_KEY']
  config.secret_key    = ENV['SECRET_KEY']

  config.capacity_mode = :on_demand
end

Which lacks a config.logger. This was resulting in errors:

  • Running rake dynamodb:ping
Connection to DynamoDB FAILED at local endpoint 'http://localhost:8000', reason being 'undefined method `debug' for nil:NilClass'
  • Running rake dynamodb:create_tables
NoMethodError: undefined method `debug' for nil:NilClass
/home/toby/.rvm/gems/ruby-2.5.1@jets/gems/dynamoid-3.4.1/lib/dynamoid/adapter.rb:56:in `benchmark'
/home/toby/.rvm/gems/ruby-2.5.1@jets/gems/dynamoid-3.4.1/lib/dynamoid/adapter.rb:150:in `block (2 levels) in <class:Adapter>'
/home/toby/.rvm/gems/ruby-2.5.1@jets/gems/dynamoid-3.4.1/lib/dynamoid/tasks/database.rb:14:in `block in create_tables'
/home/toby/.rvm/gems/ruby-2.5.1@jets/gems/dynamoid-3.4.1/lib/dynamoid/tasks/database.rb:13:in `each'
/home/toby/.rvm/gems/ruby-2.5.1@jets/gems/dynamoid-3.4.1/lib/dynamoid/tasks/database.rb:13:in `create_tables'
/home/toby/.rvm/gems/ruby-2.5.1@jets/gems/dynamoid-3.4.1/lib/dynamoid/tasks/database.rake:12:in `block (2 levels) in <top (required)>'
/home/toby/.rvm/gems/ruby-2.5.1@jets/gems/rake-13.0.1/exe/rake:27:in `<top (required)>'
/home/toby/.rvm/gems/ruby-2.5.1@jets/bin/ruby_executable_hooks:24:in `eval'
/home/toby/.rvm/gems/ruby-2.5.1@jets/bin/ruby_executable_hooks:24:in `<main>'
Tasks: TOP => dynamoid:create_tables
(See full trace by running task with --trace)

Upon investigation these reference Dynamoid.logger, which can be nil.

Would it be possible to either:

  • Revise usage of Dynamoid.logger to Dynamoid&.logger&.debug(...) (Ruby 2.3+), Dynamoid.try(:logger).try(:debug, ...) (ActiveSupport) or with a presence check (if Dynamoid.logger ...)
  • Issue a warning to STDERR if Logger is not set or otherwise detect this (perhaps this should be a "fail to boot" hard error if it's going to break everything later?)

In addition, it might be nicer to output the stacktrace as part of the ping command - this lead to some additional confusion before I tried create_tables.

tobypinder avatar Dec 27 '19 22:12 tobypinder

Thank you for the report. Actually I didn't manage to reproduce the issue with new Rails application. Could you provide steps to reproduce the issue? (Note I copied your Dynamoid configuration to an initializer).

https://github.com/andrykonchin/dynamoid-reproduce-logger-issue

The issue is very interesting because there is a default value - Rails.logger if it's a Rails application or Logger.new($stdout) otherwise. When logging is disabled (with Dynamoid::Config.logger = nil) the NullLogger is used instead

andrykonchin avatar Dec 28 '19 14:12 andrykonchin

@andrykonchin I'll try to reproduce this when I get a moment. Perhaps this is an idiosyncrasy with Ruby on Jets (while similar to rails and using most rails modules, it's possible it behaves differently in this regard). It's very possible Jets lacks the same NullLogger fallback behaviour, and that this would be more appropriate to be addressed there instead of forcing Jets specific Dynamoid changes here. I'll try and come back with some clarity but it's possible this might just end up being an upstream Jets change and/or some documentation change about requiring a Logger in exotic environments/use-cases.

tobypinder avatar Jan 06 '20 11:01 tobypinder