Ruby on Rails Monday, April 9, 2018

Hi all! Recently I've noticed an issues with autoloading of a specific classes in my app. I've created a small project which reproduces the issue: https://github.com/knovoselic/autoload_repro/

All changes required for this repro are part of this commit: https://github.com/knovoselic/autoload_repro/commit/a8cecda742d6d41876abd94f6e0aef4c9758c588

You can just pull the code and run it. The following URL: http://localhost:3000/api/graph/results/crash should crash with uninitialized constant Api::Graph::ResultsController::Graph error. 

Calling http://localhost:3000/api/graph/results/success will work fine, and any calls to crash URL after calling success will work fine, until results_controller.rb is modified. Then again crash URL fails.

I've been looking into this and I understand that Rails doesn't know nesting at the point where unknown constant is referenced. This is because Ruby doesn't provide that info in const_missing method. I've been looking at Rails code (specifically lib/active_support/dependencies.rb#load_missing_constant):
    def load_missing_constant(from_mod, const_name)
      unless qualified_const_defined?(from_mod.name) && Inflector.constantize(from_mod.name).equal?(from_mod)
        raise ArgumentError, "A copy of #{from_mod} has been removed from the module tree but is still active!"
      end

      qualified_name = qualified_name_for from_mod, const_name
      path_suffix = qualified_name.underscore

      file_path = search_for_file(path_suffix)

      if file_path
        expanded = File.expand_path(file_path)
        expanded.sub!(/\.rb\z/, "".freeze)

        if loading.include?(expanded)
          raise "Circular dependency detected while autoloading constant #{qualified_name}"
        else
          require_or_load(expanded, qualified_name)
          raise LoadError, "Unable to autoload constant #{qualified_name}, expected #{file_path} to define it" unless from_mod.const_defined?(const_name, false)
          return from_mod.const_get(const_name)
        end
      elsif mod = autoload_module!(from_mod, const_name, qualified_name, path_suffix)
        return mod
      elsif (parent = from_mod.parent) && parent != from_mod &&
            ! from_mod.parents.any? { |p| p.const_defined?(const_name, false) }
        # If our parents do not have a constant named +const_name+ then we are free
        # to attempt to load upwards. If they do have such a constant, then this
        # const_missing must be due to from_mod::const_name, which should not
        # return constants from from_mod's parents.
        begin
          # Since Ruby does not pass the nesting at the point the unknown
          # constant triggered the callback we cannot fully emulate constant
          # name lookup and need to make a trade-off: we are going to assume
          # that the nesting in the body of Foo::Bar is [Foo::Bar, Foo] even
          # though it might not be. Counterexamples are
          #
          #   class Foo::Bar
          #     Module.nesting # => [Foo::Bar]
          #   end
          #
          # or
          #
          #   module M::N
          #     module S::T
          #       Module.nesting # => [S::T, M::N]
          #     end
          #   end
          #
          # for example.
          return parent.const_missing(const_name)
        rescue NameError => e
          raise unless e.missing_name? qualified_name_for(parent, const_name)
        end
      end

      name_error = NameError.new("uninitialized constant #{qualified_name}", const_name)
      name_error.set_backtrace(caller.reject { |l| l.starts_with? __FILE__ })
      raise name_error
    end

I'd like to understand more about the bolded part of the code:
! from_mod.parents.any? { |p| p.const_defined?(const_name, false) }

What's the purpose of this check? Why do we stop looking for the constant upwards if it exists in one of these places? If we have the following code:
class Api::Graph::ResultsController < ApplicationController
  def crash
    render json: Graph::ResultsFetcher.new.result
  end

  def success
    render json: ::Graph::ResultsFetcher.new.result
  end
end


and we're trying to load Graph, the code will never try to load ::Graph as it will see that Api namespace has Graph defined. But in this example it's pretty clear that Graph doesn't reference Api::Graph. 
Why don't we just try to load ::Graph and see if it's there? Can you show me some examples where this would be unwanted behavior?

Just for testing I've modified lib/active_support/dependencies.rb#load_missing_constant like so:
    def load_missing_constant(from_mod, const_name)
      unless qualified_const_defined?(from_mod.name) && Inflector.constantize(from_mod.name).equal?(from_mod)
        raise ArgumentError, "A copy of #{from_mod} has been removed from the module tree but is still active!"
      end

      qualified_name = qualified_name_for from_mod, const_name
      path_suffix = qualified_name.underscore

      file_path = search_for_file(path_suffix)

      if file_path
        expanded = File.expand_path(file_path)
        expanded.sub!(/\.rb\z/, "".freeze)

        if loading.include?(expanded)
          raise "Circular dependency detected while autoloading constant #{qualified_name}"
        else
          require_or_load(expanded, qualified_name)
          raise LoadError, "Unable to autoload constant #{qualified_name}, expected #{file_path} to define it" unless from_mod.const_defined?(const_name, false)
          return from_mod.const_get(const_name)
        end
      elsif mod = autoload_module!(from_mod, const_name, qualified_name, path_suffix)
        return mod
      elsif (parent = from_mod.parent) && parent != from_mod
        if from_mod.parents.any? { |p| p.const_defined?(const_name, false) }
          return Object.const_missing(const_name)
        else
          # If our parents do not have a constant named +const_name+ then we are free
          # to attempt to load upwards. If they do have such a constant, then this
          # const_missing must be due to from_mod::const_name, which should not
          # return constants from from_mod's parents.
          begin
            # Since Ruby does not pass the nesting at the point the unknown
            # constant triggered the callback we cannot fully emulate constant
            # name lookup and need to make a trade-off: we are going to assume
            # that the nesting in the body of Foo::Bar is [Foo::Bar, Foo] even
            # though it might not be. Counterexamples are
            #
            #   class Foo::Bar
            #     Module.nesting # => [Foo::Bar]
            #   end
            #
            # or
            #
            #   module M::N
            #     module S::T
            #       Module.nesting # => [S::T, M::N]
            #     end
            #   end
            #
            # for example.
            return parent.const_missing(const_name)
          rescue NameError => e
            raise unless e.missing_name? qualified_name_for(parent, const_name)
          end
        end
      end

      name_error = NameError.new("uninitialized constant #{qualified_name}", const_name)
      name_error.set_backtrace(caller.reject { |l| l.starts_with? __FILE__ })
      raise name_error
    end

This works fine with the repro project (it doesn't crash anymore) and also passes full rails spec suite (tested on v5.1.6). Any reasons not to do it (or something similar)?

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe@googlegroups.com.
To post to this group, send email to rubyonrails-talk@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/2527e1ab-e6ec-4464-8b08-4c48431fd293%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

No comments:

Post a Comment