Incompatibility between Rack::Session and ActionDispatch::Request::Session #15843

Closed
pboling opened this Issue Jun 21, 2014 · 17 comments

Projects

None yet
@pboling
Contributor
pboling commented Jun 21, 2014

It is preventing integration testing in test-unit of my Grape app mounted in my Rails 4.1.1 routes.rb file.

Here's a simplified example: https://github.com/pboling/x-cascade_header_rails/tree/e34a94ba76f9350aed13486f282e99a98e2d9821

(Never mind the name of the repo... I was trying to understand the X-Cascade Header.)

Repro after checking out the repo, bundle install, rake db:migrate:

bin/rake test
...
  1) Failure:
XCascadeGrapeTest#test_/api/auth/login_invalid_credentials_is_401 [/Users/pboling/Documents/src/my/x-cascade_header_rails/test/integration/x_cascade_grape_test.rb:10]:
{"error":"undefined method `each' for #\u003cActionDispatch::Request::Session:0x103aebb00 not yet loaded\u003e"}.
Expected: 401
  Actual: 500
  1. Here's another of the exact same error from Sinatra mounted in rails:
    https://gist.github.com/toolmantim/9597022
  2. Here is another report of the same error from a Grape app mounted in Rails 4.0.2:
    http://stackoverflow.com/questions/22439361/rspec-testing-api-with-rack-protection
@mwindholtz

+1, Using Sinatra sessions with Rails 4.1.5 raise: ActionDispatch::Request::Session does not implement #each.
Only workaround is to disable sessions in Sinatra if you don't need them.
Like this: ejschmitt/delayed_job_web@eac65cf

If you do need Sinatra sessions then you're out of luck.

@jtoy
jtoy commented Sep 13, 2014

I have seen this also, anyway to get around this?

@jgandt
jgandt commented Sep 15, 2014

+1 on this. I will work on putting together an additional bug report.

Short version: My grape (0.9.0) is mounted on actionpack (4.1.5) and rack (1.5.2) and is called by rspec (3.1) and capybara-webkit (1.3.0)

@gxbe
gxbe commented Dec 21, 2014

+1 seeing this also..

Rails 4.0.3

@joost
joost commented Jan 14, 2015

Trying to mount a Rack app in Rails I got the following Rack::Lint::LintError
(Lint assertion):

session #<ActionDispatch::Request::Session:0x7f8466bfa3c8 not yet loaded> must respond to store and []=

This is because Rack::Lint expects session to respond to store. You could try monkey patching with something like:

class ActionDispatch::Request::Session
  def store(name, value)
    self[name] = value
  end
end
@pboling
Contributor
pboling commented Mar 18, 2015

Any chance this could be addressed by the Rails 5 Gods? I don't even know who to tag.

It seems like the ideal scenario would be for the ActionDispatch::Request::Session to be more like an enhancement of the Rack Session, similar to how HashWithIndifferentAccess tries to play nicely with Hash.

I fear that changing this will be a lot of work and involve breaking a lot of shit, but I don't see how Rails can be in a stack with middleware using a Rack Session without some sidelong glances.

:bump:

@kitop
Contributor
kitop commented Apr 7, 2015

Just updated a Rails 3.2 app to Rails 4 and hit this same exact issue.

Is there any other solution besides monkey-patching?

+1 to @pboling suggestion of making it play nice with Rack::Session ala HashWithIndifferentAccess and Hash

@pboling
Contributor
pboling commented Apr 7, 2015

@kitop I asked rails core about this issue on twitter and they responded. It seems that there is interest in addressing this in the upcoming Rails 5.

@abhinandankothari

Just Created new app with Rails 4.2 and got this error

@mperham
Contributor
mperham commented Jul 28, 2015

Warning: use at your own risk. Monkeypatching is never a great idea, especially a private method. Put this in config/initializers/session_store.rb:

# Monkeypatch necessary due to https://github.com/rails/rails/issues/15843
require 'rack/session/abstract/id'
class Rack::Session::Abstract::SessionHash
  private
  def stringify_keys(other)
    hash = {}
    other = other.to_hash unless other.is_a?(Hash) # hack hack hack
    other.each do |key, value|
      hash[key.to_s] = value
    end
    hash
  end
end
@LouisSayers

Another temporary solution that you might find helpful:

# Gemfile 
gem 'mocha'

# some_test.rb
def add_session_value(session_id, session_value)
  ActionDispatch::Request::Session.
    any_instance.stubs(:[]).with(session_id).
    returns(session_value)
end
@maclover7
Member

@tenderlove might have some ideas about this...

@mperham
Contributor
mperham commented Dec 19, 2015

Here's the monkeypatch that Sidekiq uses to work with Rails today:

https://github.com/mperham/sidekiq/blob/master/lib/sidekiq/web.rb#L266

@maclover7
Member

@mperham Would both the each and stringify_keys methods need to be added? Also, how would we go about testing this?

@mperham
Contributor
mperham commented Dec 19, 2015

I don't know.

On Dec 19, 2015, at 06:06, Jon Moss notifications@github.com wrote:

@mperham Would both the each and stringify_keys methods need to be added? Also, how would we go about testing this?


Reply to this email directly or view it on GitHub.

@maclover7 maclover7 added a commit to maclover7/rails that referenced this issue May 2, 2016
@maclover7 maclover7 Ensure compatibility between ActionDispatch::Request::Session and Rack
Adding the `each` method is required for ensuring compability between
Rails, and other Rack frameworks (like Sinatra, etc.), that are mounted
within Rails, and wish to use its session tooling. Prior to this, there
was an inconsistency between ActionDispatch::Request::Session and
Rack::Session::Cookie, due to the absence of the `each` method. This
should hopefully fix that error. :)

For a full integration test with Sinatra and a standalone Rack
application, you can check out the gist for that here: https://gist.github.com/maclover7/08cd95b0bfe259465314311941326470.

Solves #15843.
d04f2a4
@maclover7 maclover7 added a commit to maclover7/rails that referenced this issue May 2, 2016
@maclover7 maclover7 Ensure compatibility between ActionDispatch::Request::Session and Rack
Adding the `each` method is required for ensuring compability between
Rails, and other Rack frameworks (like Sinatra, etc.), that are mounted
within Rails, and wish to use its session tooling. Prior to this, there
was an inconsistency between ActionDispatch::Request::Session and
Rack::Session::Cookie, due to the absence of the `each` method. This
should hopefully fix that error. :)

For a full integration test with Sinatra and a standalone Rack
application, you can check out the gist for that here: https://gist.github.com/maclover7/08cd95b0bfe259465314311941326470.

Solves #15843.
184ccbc
@maclover7 maclover7 added a commit to maclover7/rails that referenced this issue May 2, 2016
@maclover7 maclover7 Ensure compatibility between ActionDispatch::Request::Session and Rack
Adding the `each` method is required for ensuring compability between
Rails, and other Rack frameworks (like Sinatra, etc.), that are mounted
within Rails, and wish to use its session tooling. Prior to this, there
was an inconsistency between ActionDispatch::Request::Session and
Rack::Session::Cookie, due to the absence of the `each` method. This
should hopefully fix that error. :)

For a full integration test with Sinatra and a standalone Rack
application, you can check out the gist for that here: https://gist.github.com/maclover7/08cd95b0bfe259465314311941326470.

Solves #15843.
c794a2a
@maclover7 maclover7 added a commit to maclover7/rails that referenced this issue May 2, 2016
@maclover7 maclover7 Ensure compatibility between ActionDispatch::Request::Session and Rack
Adding the `each` method is required for ensuring compatibility between
Rails, and other Rack frameworks (like Sinatra, etc.), that are mounted
within Rails, and wish to use its session tooling. Prior to this, there
was an inconsistency between ActionDispatch::Request::Session and
Rack::Session::Cookie, due to the absence of the `each` method. This
should hopefully fix that error. :)

For a full integration test with Sinatra and a standalone Rack
application, you can check out the gist for that here: https://gist.github.com/maclover7/08cd95b0bfe259465314311941326470.

Solves #15843.
46cb49a
@maclover7
Member

Opened PR #24820 to try and solve this :)

@maclover7 maclover7 added a commit to maclover7/rails that referenced this issue May 3, 2016
@maclover7 maclover7 Ensure compatibility between ActionDispatch::Request::Session and Rack
Adding the `each` method is required for ensuring compatibility between
Rails, and other Rack frameworks (like Sinatra, etc.), that are mounted
within Rails, and wish to use its session tooling. Prior to this, there
was an inconsistency between ActionDispatch::Request::Session and
Rack::Session::Cookie, due to the absence of the `each` method. This
should hopefully fix that error. :)

For a full integration test with Sinatra and a standalone Rack
application, you can check out the gist for that here: https://gist.github.com/maclover7/08cd95b0bfe259465314311941326470.

Solves #15843.
4c57b2e
@maclover7 maclover7 added a commit to maclover7/rails that referenced this issue May 3, 2016
@maclover7 maclover7 Ensure compatibility between ActionDispatch::Request::Session and Rack
Adding the `each` method is required for ensuring compatibility between
Rails, and other Rack frameworks (like Sinatra, etc.), that are mounted
within Rails, and wish to use its session tooling. Prior to this, there
was an inconsistency between ActionDispatch::Request::Session and
Rack::Session::Cookie, due to the absence of the `each` method. This
should hopefully fix that error. :)

For a full integration test with Sinatra and a standalone Rack
application, you can check out the gist for that here: https://gist.github.com/maclover7/08cd95b0bfe259465314311941326470.

Solves #15843.
93af003
@maclover7 maclover7 added a commit to maclover7/rails that referenced this issue May 4, 2016
@maclover7 maclover7 Ensure compatibility between ActionDispatch::Request::Session and Rack
Adding the `each` method is required for ensuring compatibility between
Rails, and other Rack frameworks (like Sinatra, etc.), that are mounted
within Rails, and wish to use its session tooling. Prior to this, there
was an inconsistency between ActionDispatch::Request::Session and
Rack::Session::Cookie, due to the absence of the `each` method. This
should hopefully fix that error. :)

For a full integration test with Sinatra and a standalone Rack
application, you can check out the gist for that here: https://gist.github.com/maclover7/08cd95b0bfe259465314311941326470.

Solves #15843.
09159d8
@maclover7
Member

Fixed via #24820.

@maclover7 maclover7 closed this May 4, 2016
@Neodelf Neodelf added a commit to Neodelf/rails that referenced this issue May 5, 2016
@maclover7 @Neodelf maclover7 + Neodelf Ensure compatibility between ActionDispatch::Request::Session and Rack
Adding the `each` method is required for ensuring compatibility between
Rails, and other Rack frameworks (like Sinatra, etc.), that are mounted
within Rails, and wish to use its session tooling. Prior to this, there
was an inconsistency between ActionDispatch::Request::Session and
Rack::Session::Cookie, due to the absence of the `each` method. This
should hopefully fix that error. :)

For a full integration test with Sinatra and a standalone Rack
application, you can check out the gist for that here: https://gist.github.com/maclover7/08cd95b0bfe259465314311941326470.

Solves #15843.
55d698b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment