Fix undefined method `DelegateClass' for Rack::Session::Cookie:Class

[onigra]

Summary

Rack::Session::Cookie requires require 'delegate'

Test Repository: https://github.com/onigra/rack-undefined-method-delegate-class

Environment

• rack 2.2.2
• Ruby 2.7.0

Before

# require "delegate"
require "rack/session/cookie"

run ->(env) { [200, { 'Content-Type' => 'text/plain' }, ['ok']] }

$ rackup

Traceback (most recent call last):
        20: from /Users/onigra/.asdf/installs/ruby/2.7.0/bin/rackup:23:in `<main>'
        19: from /Users/onigra/.asdf/installs/ruby/2.7.0/bin/rackup:23:in `load'
        18: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/bin/rackup:5:in `<top (required)>'
        17: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/server.rb:168:in `start'
        16: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/server.rb:311:in `start'
        15: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/server.rb:379:in `handle_profiling'
        14: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/server.rb:312:in `block in start'
        13: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/server.rb:422:in `wrapped_app'
        12: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/server.rb:249:in `app'
        11: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/server.rb:349:in `build_app_and_options_from_config'
        10: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/builder.rb:66:in `parse_file'
         9: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/builder.rb:105:in `load_file'
         8: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/builder.rb:116:in `new_from_string'
         7: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/builder.rb:116:in `eval'
         6: from /Users/onigra/src/github.com/onigra/rack-undefined-method-delegate-class/config.ru:2:in `block in <main>'
         5: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:72:in `require'
         4: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:72:in `require'
         3: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/session/cookie.rb:9:in `<top (required)>'
         2: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/session/cookie.rb:11:in `<module:Rack>'
         1: from /Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/session/cookie.rb:50:in `<module:Session>'
/Users/onigra/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/rack-2.2.2/lib/rack/session/cookie.rb:155:in `<class:Cookie>': undefined method `DelegateClass' for Rack::Session::Cookie:Class (NoMethodError)

After

require "delegate"
require "rack/session/cookie"

run ->(env) { [200, { 'Content-Type' => 'text/plain' }, ['ok']] }

$ rackup

[2020-02-24 14:13:29] INFO  WEBrick 1.6.0
[2020-02-24 14:13:29] INFO  ruby 2.7.0 (2019-12-25) [x86_64-darwin18]
[2020-02-24 14:13:29] INFO  WEBrick::HTTPServer#start: pid=5735 port=9292

[jeremyevans]

While adding the delegate require does fix the problem, accepting this will lead to the slippery slope of adding such internal requires for all deep requires of rack. That is something we are trying to avoid starting in Rack 2.2. As stated in the release notes for 2.2.0, under Changed:

• Rely on autoload to load constants instead of requiring internal files, make sure to require 'rack' and not just 'rack/...'.

If you change your require to be "rack" instead of "rack/session/cookie", that will solve the problem. That is the recommended way going forward. While this particular issue was not expected, it was expected there would some fallout from the change to rely more heavily on autoload.

I'm not going to close this immediately, I would like to get some feedback from other members of the core team first.

[ioquatix]

I think individual files should pull in relevant dependencies, and as such I would accept a PR to fix this. Personally, I think autoload is tangential to this issue, as that is about loading files relative to rack/lib, not stdlib or other gems.

[jeremyevans]

@ioquatix Just to confirm, you are OK with this PR because the require is for a stdlib library, but you wouldn't be OK with adding a require for rack/*, even if you got the same result (direct require of rack/session/cookie not working)? Note that I'm not opposed to that approach, I'm just trying to understand the reasoning.

[tenderlove]

If you change your require to be "rack" instead of "rack/session/cookie", that will solve the problem.

How would this fix it? delegate needs to be required at some point. I don't see it required at all:

[aaron@tc-lan-adapter ~/g/rack (master)]$ git grep delegate | grep require
[aaron@tc-lan-adapter ~/g/rack (master)]$

Since rack/session/cookie is the one using DelegateClass, it seems to be the right place to put the require.

[tenderlove]

[aaron@tc-lan-adapter ~/g/rack (master)]$ ruby --disable-gems -I lib -r rack -e'p Rack::Session::Cookie'
Traceback (most recent call last):
	5: from -e:1:in `<main>'
	4: from -e:1:in `require'
	3: from /Users/aaron/git/rack/lib/rack/session/cookie.rb:9:in `<top (required)>'
	2: from /Users/aaron/git/rack/lib/rack/session/cookie.rb:11:in `<module:Rack>'
	1: from /Users/aaron/git/rack/lib/rack/session/cookie.rb:50:in `<module:Session>'
/Users/aaron/git/rack/lib/rack/session/cookie.rb:155:in `<class:Cookie>': undefined method `DelegateClass' for Rack::Session::Cookie:Class (NoMethodError)

[jeremyevans]

Yep, @tenderlove is correct. When I originally tested this, it appeared to work with just requiring rack, but my testing was broken. I'll merge this now.

[onigra]

Thank you!! 🙌

[ioquatix]

Do we want to back port this or does it not matter?

[jeremyevans]

I think it is a candidate for 2.2.3. However, the workaround is fairly simple, so I don't feel strongly about it.