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

undefined method `logger' for Rails:Module #180

Merged
merged 3 commits into from Jun 8, 2022

Conversation

TSMMark
Copy link
Contributor

@TSMMark TSMMark commented Apr 20, 2022

In some codebases Rails might be defined but not have a logger method defined. This should avoid the issue.

in some codebases Rails might be defined but not have a logger. This should avoid the issue.
@schneems
Copy link
Member

schneems commented Jun 6, 2022

Do you have an example of this happening? I would think not having Rails.logger defined would be a bug in an app. Maybe you could show me an https://codetriage.com/example_app showing the problem so I can better understand the fix?

@TSMMark
Copy link
Contributor Author

TSMMark commented Jun 6, 2022

Some gems define Rails as a module, even when you do not have any Rails app in your codebase (Roda, Sinatra, vanilla Ruby, etc).

I've already patched this in other gems, e.g. resque/resque#1799

rails/web-console gem is the specific gem I experienced this with. Here's an example from my dev console:

development [1] Vydia(main)> Rails
NameError: uninitialized constant Rails
from (pry):1:in `__pry__'
development [2] Vydia(main)> require "web-console"
=> true
development [3] Vydia(main)> Rails
=> Rails
development [4] Vydia(main)> Rails.logger
NoMethodError: undefined method `logger' for Rails:Module
from (pry):4:in `__pry__'
development [5] Vydia(main)> Rails.class
=> Module
development [6] Vydia(main)> show-source Rails

From: /bundle/ruby/3.0.0/gems/railties-6.1.4/lib/rails/paths.rb:5
Module name: Rails
Number of monkeypatches: 4. Use the `-a` option to display all available monkeypatches
Number of lines: 239

module Rails
  module Paths
    # This object is an extended hash that behaves as root of the <tt>Rails::Paths</tt> system.
    # It allows you to collect information about how you want to structure your application
    # paths through a Hash-like API. It requires you to give a physical path on initialization.
    #
    #   root = Root.new "/rails"
    #   root.add "app/controllers", eager_load: true
    #
    # The above command creates a new root object and adds "app/controllers" as a path.
    # This means we can get a <tt>Rails::Paths::Path</tt> object back like below:
    #
    #   path = root["app/controllers"]
    #   path.eager_load?               # => true
    #   path.is_a?(Rails::Paths::Path) # => true
    #
    # The +Path+ object is simply an enumerable and allows you to easily add extra paths:
    #
    #   path.is_a?(Enumerable) # => true
    #   path.to_ary.inspect    # => ["app/controllers"]
    #
    #   path << "lib/controllers"
    #   path.to_ary.inspect    # => ["app/controllers", "lib/controllers"]
    #
    # Notice that when you add a path using +add+, the path object created already
    # contains the path with the same path value given to +add+. In some situations,
    # you may not want this behavior, so you can give <tt>:with</tt> as option.
    #
    #   root.add "config/routes", with: "config/routes.rb"
    #   root["config/routes"].inspect # => ["config/routes.rb"]
    #
    # The +add+ method accepts the following options as arguments:
    # eager_load, autoload, autoload_once, and glob.
    #
    # Finally, the +Path+ object also provides a few helpers:
    #
    #   root = Root.new "/rails"
    #   root.add "app/controllers"
    #
    #   root["app/controllers"].expanded # => ["/rails/app/controllers"]
    #   root["app/controllers"].existent # => ["/rails/app/controllers"]
    #
    # Check the <tt>Rails::Paths::Path</tt> documentation for more information.
    class Root
      attr_accessor :path

      def initialize(path)
        @path = path
        @root = {}
      end

      def []=(path, value)
        glob = self[path] ? self[path].glob : nil
        add(path, with: value, glob: glob)
      end
development [7] Vydia(main)> 

As a workaround I have this, but I think it would be best if gems did not assume just because defined?(Rails) it means that there is any Rails app.

Rails.define_singleton_method(:logger) { Logger.new($stdout) }

@schneems
Copy link
Member

schneems commented Jun 7, 2022

Than's for the explanation, this seems good. Can you add a changelog entry for this as it's a change in behavior?

I think that will also re-trigger CI to run.

@TSMMark
Copy link
Contributor Author

TSMMark commented Jun 7, 2022

@schneems done

@schneems
Copy link
Member

schneems commented Jun 8, 2022

Thanks!

@schneems schneems merged commit 4bc879b into zombocom:master Jun 8, 2022
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 this pull request may close these issues.

None yet

2 participants