rack / rack-session

MIT License
33 stars 14 forks source link

Needs rack-session.rb #6

Closed mperham closed 2 years ago

mperham commented 2 years ago

Bundler will only automatically require lib/rack-session.rb based on the gem name. I think you need to add it which requires "rack/session".

gem "sidekiq", path: ".."
gem "concurrent-ruby"

# with Rack >= 3.0
gem "rackup"
gem "rack-session"
% bundle exec rackup simple.ru
bundler: failed to load command: rackup (/Users/mperham/.gem/ruby/3.1.2/bin/rackup)
/Users/mperham/src/sidekiq/bare/simple.ru:18:in `block in <main>': uninitialized constant Rack::Session (NameError)
Did you mean?  Rack::RACK_SESSION
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:106:in `eval'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:106:in `new_from_string'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:97:in `load_file'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:67:in `parse_file'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:351:in `build_app_and_options_from_config'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:260:in `app'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:421:in `wrapped_app'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:323:in `block in start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:379:in `handle_profiling'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:322:in `start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:179:in `start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/bin/rackup:5:in `<top (required)>'
    from /Users/mperham/.gem/ruby/3.1.2/bin/rackup:25:in `load'
    from /Users/mperham/.gem/ruby/3.1.2/bin/rackup:25:in `<top (required)>'
jeremyevans commented 2 years ago

Alternatively, you can use require: 'rack/session' in the Gemfile. No objections to adding a rack-session.rb file, though.

mperham commented 2 years ago

I know that, I'm just following the readme and pointing out issues where it does not work. The README also does not mention the need to require "rack/session/cookie".

jeremyevans commented 2 years ago

I submitted a pull request to add rack-session.rb. Note that if you require 'rack/session' (or 'rack-session' with the PR), you should not need to specifically require rack/session/cookie, as Rack::Session::Cookie should be autoloaded.

mperham commented 2 years ago

Note that if you require 'rack/session' (or 'rack-session' with the PR), you should not need to specifically require rack/session/cookie, as Rack::Session::Cookie should be autoloaded.

That was not my experience, I had to explicitly require it in my .ru file. Maybe eval does not autoload constants?

jeremyevans commented 2 years ago

eval does autoload constants. You should be able to test that this works from a checkout:

$ irb -I lib -r rack/session
irb(main):001:0> Rack::Session.autoload?(:Cookie)
=> "rack/session/cookie"
irb(main):002:0> eval 'Rack::Session::Cookie'
=> Rack::Session::Cookie
mperham commented 2 years ago

It doesn't work for me.

% bundle exec rackup simple.ru
bundler: failed to load command: rackup (/Users/mperham/.gem/ruby/3.1.2/bin/rackup)
/Users/mperham/src/sidekiq/bare/simple.ru:18:in `block in <main>': uninitialized constant Rack::Session (NameError)
Did you mean?  Rack::RACK_SESSION
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:106:in `eval'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:106:in `new_from_string'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:97:in `load_file'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:67:in `parse_file'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:351:in `build_app_and_options_from_config'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:260:in `app'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:421:in `wrapped_app'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:323:in `block in start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:379:in `handle_profiling'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:322:in `start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:179:in `start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/bin/rackup:5:in `<top (required)>'
    from /Users/mperham/.gem/ruby/3.1.2/bin/rackup:25:in `load'
# Easiest way to run Sidekiq::Web.
# Run with "bundle exec rackup simple.ru"

require "sidekiq/web"

# In a multi-process deployment, all Web UI instances should share
# this secret key so they can all decode the encrypted browser cookies
# and provide a working session.
# Rails does this in /config/initializers/secret_token.rb
secret_key = SecureRandom.hex(32)
use Rack::Session::Cookie, secret: secret_key, same_site: true, max_age: 86400
run Sidekiq::Web
gem "rack-session", require: "rack/session"
jeremyevans commented 2 years ago

Can you create a self contained example that doesn't use sidekiq or bundle exec and still has this issue? As I mentioned in https://github.com/rack/rack-session/issues/7#issuecomment-1209562821, I cannot recreate this in a simple.ru that doesn't use either. That should help determine the root cause.

mperham commented 2 years ago
% bundle
Fetching gem metadata from https://rubygems.org/..
Resolving dependencies...
Using rack 3.0.0.beta1
Using rack-session 0.1.0
Using webrick 1.7.0
Using bundler 2.3.7
Using rackup 0.2.1
Bundle complete! 3 Gemfile dependencies, 5 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.
% rackup bare.ru            
/Users/mperham/src/sidekiq/bare/bare.ru:4:in `block in <main>': uninitialized constant Rack::Session (NameError)
Did you mean?  Rack::RACK_SESSION
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:106:in `eval'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:106:in `new_from_string'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:97:in `load_file'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:67:in `parse_file'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:351:in `build_app_and_options_from_config'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:260:in `app'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:421:in `wrapped_app'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:323:in `block in start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:379:in `handle_profiling'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:322:in `start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:179:in `start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/bin/rackup:5:in `<top (required)>'
    from /Users/mperham/.gem/ruby/3.1.2/bin/rackup:25:in `load'
    from /Users/mperham/.gem/ruby/3.1.2/bin/rackup:25:in `<main>'
% bundle exec rackup bare.ru
bundler: failed to load command: rackup (/Users/mperham/.gem/ruby/3.1.2/bin/rackup)
/Users/mperham/src/sidekiq/bare/bare.ru:4:in `block in <main>': uninitialized constant Rack::Session (NameError)
Did you mean?  Rack::RACK_SESSION
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:106:in `eval'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:106:in `new_from_string'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:97:in `load_file'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rack-3.0.0.beta1/lib/rack/builder.rb:67:in `parse_file'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:351:in `build_app_and_options_from_config'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:260:in `app'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:421:in `wrapped_app'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:323:in `block in start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:379:in `handle_profiling'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:322:in `start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/lib/rackup/server.rb:179:in `start'
    from /Users/mperham/.gem/ruby/3.1.2/gems/rackup-0.2.1/bin/rackup:5:in `<top (required)>'
    from /Users/mperham/.gem/ruby/3.1.2/bin/rackup:25:in `load'
    from /Users/mperham/.gem/ruby/3.1.2/bin/rackup:25:in `<top (required)>'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/cli/exec.rb:58:in `load'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/cli/exec.rb:58:in `kernel_load'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/cli/exec.rb:23:in `run'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/cli.rb:484:in `exec'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/cli.rb:31:in `dispatch'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/cli.rb:25:in `start'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.7/libexec/bundle:48:in `block in <top (required)>'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/3.1.0/bundler/friendly_errors.rb:103:in `with_friendly_errors'
    from /Users/mperham/.rubies/ruby-3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.7/libexec/bundle:36:in `<top (required)>'
    from /Users/mperham/.gem/ruby/3.1.2/bin/bundle:25:in `load'
    from /Users/mperham/.gem/ruby/3.1.2/bin/bundle:25:in `<main>'
% ruby -v
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]
% more bare.ru
require "securerandom"

secret_key = SecureRandom.hex(32)
use Rack::Session::Cookie, secret: secret_key, same_site: true, max_age: 86400
run do
  # nothing
end
jeremyevans commented 2 years ago

You need to add require 'rack/session' to bare.ru. Then it should run. rack-session is no longer part of rack, so rack itself won't autoload it.

jeremyevans commented 2 years ago

I think we should update the README to add require 'rack/session', I'll submit a PR for that.

mperham commented 2 years ago

rack/session does not define Rack::Session. I think that’s the problem.

On Tue, Aug 9, 2022 at 11:17 Jeremy Evans @.***> wrote:

I think we should update the README to add require 'rack/session', I'll submit a PR for that.

— Reply to this email directly, view it on GitHub https://github.com/rack/rack-session/issues/6#issuecomment-1209716591, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAWX6NRUJQUHEBT5FCUNTVYKOBZANCNFSM56BA2JWQ . You are receiving this because you authored the thread.Message ID: @.***>

jeremyevans commented 2 years ago

rack/session does not define Rack::Session. I think that’s the problem.

https://github.com/rack/rack-session/blob/main/lib/rack/session.rb#L2

mperham commented 2 years ago

My rack-session 0.1.0 gem has this content:

https://github.com/rack/rack-session/blob/2c4cd627ba1ff5ee2c4a97bca2102ac7ec81729d/lib/rack/session.rb

jeremyevans commented 2 years ago

You'll need rack-session 0.2.0, released last week.

mperham commented 2 years ago

It does not install with rack 3.0.0.beta1. Does this need to be "> 2" to work with a beta?

https://github.com/rack/rack-session/blob/58b582f433004677ae6221fe753abdd68576c02e/rack-session.gemspec#L21

% bundle up rack-session
Fetching gem metadata from https://rubygems.org/..
Resolving dependencies...
Using bundler 2.3.7
Using rack 3.0.0.beta1
Using rack-session 0.1.0
Using webrick 1.7.0
Using rackup 0.2.1
Bundler attempted to update rack-session but its version stayed the same
jeremyevans commented 2 years ago

It does not install with rack 3.0.0.beta1. Does this need to be "> 2" to work with a beta?

See #9

ioquatix commented 2 years ago

I reached out to the bundler team to ask them, and they said it should load rack/session.rb (as well as rack-session.rb). I confirmed that this shouldn't need any rack-session.rb to work, here is the backtrace showing bundler requiring the correct file:

 "/Users/samuel/Projects/rack/rack-session/lib/rack/session.rb:9:in `<top (required)>'",
 "/Users/samuel/.gem/ruby/3.1.2/gems/bundler-2.3.8/lib/bundler/runtime.rb:73:in `require'",
 "/Users/samuel/.gem/ruby/3.1.2/gems/bundler-2.3.8/lib/bundler/runtime.rb:73:in `rescue in block in require'",
 "/Users/samuel/.gem/ruby/3.1.2/gems/bundler-2.3.8/lib/bundler/runtime.rb:51:in `block in require'",
 "/Users/samuel/.gem/ruby/3.1.2/gems/bundler-2.3.8/lib/bundler/runtime.rb:44:in `each'",
 "/Users/samuel/.gem/ruby/3.1.2/gems/bundler-2.3.8/lib/bundler/runtime.rb:44:in `require'",
 "/Users/samuel/.gem/ruby/3.1.2/gems/bundler-2.3.8/lib/bundler.rb:176:in `require'",
 "/Users/samuel/Projects/rack/testapp/app.rb:3:in `<top (required)>'",

Bundler.require will correctly load in rack/session.rb.

Confirmed with the following app.rb:

#!/usr/bin/env ruby

Bundler.require
# gems.rb
source "https://rubygems.org"

gem "rack", path: "../rack"
gem "rack-session", path: "../rack-session"

I modified my local copy of rack-session to get a backtrace when rack/session.rb was loaded.

It looks like it's working to me?

> Bundler::VERSION
=> "2.3.8"
mperham commented 2 years ago

Ok. I guess something is wrong with my local env; it's not working for me using 3.1.2 / 2.3.20.

ioquatix commented 2 years ago

I tried this on Linux too, and it worked. Are you calling Bundler.require?

> bundle exec ./app.rb

From: /home/samuel/Projects/rack/myapp/app.rb @ line 4 :

    1: #!/usr/bin/env ruby
    2: 
    3: Bundler.require
 => 4: binding.irb

irb(main):001:0> Rack::Session::Cookie
=> Rack::Session::Cookie
irb(main):002:0> Bundler::VERSION
=> "2.3.10"
irb(main):003:0> 

It seems to load the right constant:

irb(main):008:0> Rack::Session.const_source_location(:Cookie)
=> ["/home/samuel/Projects/rack/rack-session/lib/rack/session/cookie.rb", 87]
mperham commented 2 years ago

I'm not. My understanding is that Bundler calls Bundler.require as part of bundle exec?

ioquatix commented 2 years ago

I'm not.

Are you actually calling Bundler.require or just expecting bundle exec to do that? Because I don't think that's the documented behaviour. Let me try it.

ioquatix commented 2 years ago

Is it possible you are hitting this issue? https://stackoverflow.com/questions/34870704/why-does-bundler-require-not-require-dependencies

ioquatix commented 2 years ago
#!/usr/bin/env ruby                                    

# Bundler.require                                      
binding.irb                                            

I changed it to this, and you are right, it's really weird behaviour.

ioquatix commented 2 years ago

I see, when you load the local gemspec, it loads rack/session/version.rb which defines some of the constants:

 "/home/samuel/Projects/rack/rack-session/lib/rack/session/version.rb:9:in `<top (required)>'",
 "/home/samuel/Projects/rack/rack-session/rack-session.gemspec:3:in `require_relative'",
 "/home/samuel/Projects/rack/rack-session/rack-session.gemspec:3:in `<main>'",
 "/home/samuel/.gem/ruby/3.1.2/gems/bundler-2.3.10/lib/bundler.rb:665:in `eval'",
 "/home/samuel/.gem/ruby/3.1.2/gems/bundler-2.3.10/lib/bundler.rb:665:in `eval_gemspec'",

This looks like a half defined Rack::Session but it's not actually pulling in the gem proper. Let me see if adding rack-session.rb fixes this.

ioquatix commented 2 years ago

Okay, I added lib/rack-session.rb with binding.irb, but I could not trigger it with the app.rb:

#!/usr/bin/env ruby                                    

# Bundler.require                                      
binding.irb                                            

I can only get it to trigger with Bundler.require.

Uncommenting Bundler.require will load it, or rack/session.rb.

> bundle exec ./app.rb

From: /home/samuel/Projects/rack/rack-session/lib/rack-session.rb @ line 2 :

    1: 
 => 2: binding.irb
ioquatix commented 2 years ago

I think the key point here is, bundle exec sets up the environment but won't do Bundler.require, you need to do that explicitly, and we've shown that it works with either rack-session.rb or rack/session.rb (exists already).

mperham commented 2 years ago

Should rackup run Bundler,require before eval’ing or does every .ru file need to call Bundler.require at some point?

On Thu, Aug 11, 2022 at 21:52 Samuel Williams @.***> wrote:

I think the key point here is, bundle exec sets up the environment but won't do Bundler.require, you need to do that explicitly, and we've shown that it works with either rack-session.rb or rack/session.rb (exists already).

— Reply to this email directly, view it on GitHub https://github.com/rack/rack-session/issues/6#issuecomment-1212727497, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAWX3RPSBL2KEF3LNPAEDVYXJ7XANCNFSM56BA2JWQ . You are receiving this because you modified the open/close state.Message ID: @.***>

ioquatix commented 2 years ago

I don't have a strong opinion about that. I think it's probably application framework specific. We could ask team Bundler for their advice. cc @indirect

mperham commented 2 years ago

It does seem like something that should be well defined for rackup v1.0.

On Thu, Aug 11, 2022 at 22:12 Samuel Williams @.***> wrote:

I don't have a strong opinion about that. I think it's probably application framework specific. We could ask team Bundler for their advice. cc @indirect https://github.com/indirect

— Reply to this email directly, view it on GitHub https://github.com/rack/rack-session/issues/6#issuecomment-1212735847, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAWX5PCRONMYS3LITCJ2DVYXMKBANCNFSM56BA2JWQ . You are receiving this because you modified the open/close state.Message ID: @.***>

ioquatix commented 2 years ago

This was never part of rackup before, but I agree we can discuss it and consider it as a feature.

jeremyevans commented 2 years ago

I'm strongly opposed to running Bundler.require by default. However, maybe if BUNDLE_GEMFILE is set in the environment (bundle exec sets this), then we can run it.