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

Latest rack (>= 2.0.8) upgrade when used alongside latest devise + mongoid + mongo_session_store + sidekiq UI throws error #1510

Closed
streetlogics opened this issue Jan 22, 2020 · 6 comments

Comments

@streetlogics
Copy link

I originally filed this bug with the mongoid team (https://jira.mongodb.org/browse/RUBY-2095), but after doing some digging and back and forth with them, we're realizing that the issue may be coming from rack. Here's a quick snippet from the ticket:

[44, 53] in /home/w/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/mongo_session_store-3.2.0/lib/mongo_session_store/mongo_store_base.rb

44:         # However, ActionPack seems to want a session id instead.
45:         record.save ? id : false
46:       end
47:
48:       def find_or_initialize_session(id)
=> 49:         existing_session = (id && session_class.where(:_id => id).first)
50:         session = existing_session if existing_session
51:         session \|\|= session_class.new(:_id => generate_sid)
52:         [session._id, session]
53:       end
(byebug) id
"545f8225efa4bbe723a0d5b909ced038f292b1ac20e27a29484004396c7abe7e"
(byebug) id.class
Rack::Session::SessionId

id there is a Rack::Session::SessionId which derives from Object. There are no docstrings on this method or surrounding (in the call stack) methods that state what type the id is expected to have. In rack 2.0.7 the id is a String.
Therefore if the way mongo_session_store uses rack is part of rack's public API, rack appears to have made an incompatible change between 2.0.7 and 2.0.8, and this bug should be filed against rack (with a potential fix to make Rack::Session::SessionId derive from String rather than Object). If mongo_session_store used a non-public rack API then bringing this issue to their attention is potentially the way to go, with the fix being to call #to_s on the session id.

@streetlogics streetlogics changed the title Latest rack (>= 2.0.8) upgrade when used alongside latest devise + mongoid + mongo_session_store Latest rack (>= 2.0.8) upgrade when used alongside latest devise + mongoid + mongo_session_store throws error Jan 22, 2020
@streetlogics
Copy link
Author

mongoid/mongo_session_store#43 - this is the PR in the mongo_session_store repo for reference

@streetlogics streetlogics changed the title Latest rack (>= 2.0.8) upgrade when used alongside latest devise + mongoid + mongo_session_store throws error Latest rack (>= 2.0.8) upgrade when used alongside latest devise + mongoid + mongo_session_store + sidekiq UI throws error Jan 22, 2020
@jeremyevans
Copy link
Contributor

Does mongo_session_store work with Rack 2-0-stable branch? There was a fix to this issue that has already been backported to 2-0-stable. If so we may want to consider a 2.0.9 release.

@streetlogics
Copy link
Author

I just tried updating the example project (https://jira.mongodb.org/secure/attachment/243898/243898_RUBY-2095-sidekiq-issue-monkey-patched.zip) to use:

gem 'rack', github: 'rack/rack', branch: "2-0-stable"

And without the monkey patch (aka: the to_s on session id), the error undefined method `bson_type' for #<Rack::Session::SessionId:0x00007fe412b85a08> still gets thrown

@jeremyevans
Copy link
Contributor

The session id is now a Rack::Session::SessionId object instead of a string (by design), so if you are defining String#bson_type and expecting that to affect Rack::Session::SessionId, you'll need to adapt to the changes and define Rack::Session::SessionId#bson_type to call public_id.bson_type if Rack::Session::SessionId is defined. Can you try that and see if it fixes things with 2-0-stable (it should work with 2.0.8 as well)?

@streetlogics
Copy link
Author

I put this in an initializer in my local repo and it worked:

module Rack
  module Session
    class SessionId
      def bson_type
        public_id.bson_type
      end

      def to_bson(arg1, arg2)
        public_id.to_bson(arg1, arg2)
      end
    end
  end
end

@jeremyevans
Copy link
Contributor

OK. I think that means this can be closed as it isn't a bug in Rack.

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

No branches or pull requests

2 participants