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

Don't output the whole Rails::Railtie object when a NoMethodError is raised #44495

Merged
merged 1 commit into from
Mar 2, 2022

Conversation

Edouard-chin
Copy link
Member

@Edouard-chin Edouard-chin commented Feb 21, 2022

Don't output the whole Rails::Railtie object when a NoMethodError is raised

  • Summary

    The terminal output on a Rails application running ruby 3 will be
    cluttered with thousands of lines if one inadventarly call a
    unexisting method.
    This happen in various places (IntegrationTest, when running a db
    migration ...).
    This is related to a change in ruby 3 when a NoMethodError is
    raised.

    Simple reproduction

    class A
      def initialize(session)
        @A = session
      end
    end
    
    test = A.new("*" * 36)
    test.dsad # undefined method `dsad' for #<A:0x00007f847d8494b0 @A="************************************"> (NoMethodError)
    # Note that the "#<A:0x00007f847d8494b0 @A="************************************">" part is 65 chars long.
    
    test = test = A.new("*" * 37)
    test.dsad # undefined method `dsad' for #<A:0x00007fa8c38299c0> (NoMethodError)

    On Ruby < 3, the NoMethodError message (everything starting from the
    "#" char) could only be 65 characters long. If it was above that
    ruby would only output the name of the class and its address.

    On Ruby >= 3, that limitation has been removed and the message can
    be any length long.

    Change upstream was here ruby/ruby@1258a0f

    On Rails

    Anytime a method is called on a object that holds the entire
    Rails::Application, the terminal would output the entire application
    which is annoying and be can be dangerous because it will leak
    everything containing the credentials (stored inside the Application
    object).

@rails-bot rails-bot bot added the actionpack label Feb 21, 2022
@jonathanhefner
Copy link
Member

I noticed this too when typing e.g. Rails.application.config.does_not_exist in the console. What do you think about reducing the output of Application#inspect instead?

@Edouard-chin
Copy link
Member Author

Make sense 👍 , just noticed this behaviour in many more places . Will change

@Edouard-chin
Copy link
Member Author

Done, decided to override the inspect method on the Railtie object because any engine would have this problem as well. Also updated my explanation on why this is happening because it wasn't very accurate.

@Edouard-chin Edouard-chin changed the title Make test output readable again when running integration tests: Don't output the whole Rails::Railtie object when a NoMethodError is raised Feb 22, 2022
@@ -24,4 +24,14 @@ def initialize
end
assert_equal "world", klass.instance.hello
end

test "application object isn't output when a NoMethorError is raised" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we be testing this at the railtie level?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is checking with an application though, I think we need to either use an engine or a railtie and move this file in this case.

@Edouard-chin
Copy link
Member Author

Thanks for the reviews, I tested on a Railtie instead and moved the test in a more appropriate test file

- ### Summary

  The terminal output on a Rails application running ruby 3 will be
  cluttered with thousands of lines if one inadventarly call a
  unexisting method.
  This happen in various places (IntegrationTest, when running a db
  migration ...).
  This is related to a change in ruby 3 when a NoMethodError is
  raised.

  ### Simple reproduction

  ```
  class A
    def initialize(session)
      @A = session
    end
  end

  test = A.new("*" * 36)
  test.dsad # undefined method `dsad' for #<A:0x00007f847d8494b0 @A="************************************"> (NoMethodError)
  # Note that the "#<A:0x00007f847d8494b0 @A="************************************">" part
  # is 65 chars long.

  test = test = A.new("*" * 37)
  test.dsad # undefined method `dsad' for #<A:0x00007fa8c38299c0> (NoMethodError)
  ```

  On Ruby < 3, the NoMethodError message (everything starting from the
  "#" char) could only be 65 characters long. If it was above that
  ruby would only output the name of the class and its address.

  On Ruby >= 3, that limitation has been removed and the message can
  be any length long.

  ### On Rails

  Anytime a method is called on a object that holds the entire
  Rails::Application, the terminal would output the entire application
  which is annoying be can be dangerous because it will leak
  everything containing the credentials (stored inside the Application
  object).
@rafaelfranca rafaelfranca merged commit e3353e5 into rails:main Mar 2, 2022
rafaelfranca added a commit that referenced this pull request Jul 6, 2022
Don't output the whole Rails::Railtie object when a NoMethodError is raised
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants