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

Fix: Add `to_hash` to wrap Hash and Session classes on `stringify_keys` #1428

Merged
merged 1 commit into from Jan 11, 2020

Conversation

@oleh-demyanyuk
Copy link
Contributor

oleh-demyanyuk commented Dec 16, 2019

When using Rails application and go to Sidekiq Web management page I've got the following error:

undefined method 'transform_keys' for #<ActionDispatch::Request::Session:0x00007f8fa6d3bcc8>

With code :

        def stringify_keys(other)
          other.transform_keys(&:to_s) # <-- Error happened here
        end
      end

Application Trace:

/Users/olehd/.rvm/gems/ruby-2.3.7/bundler/gems/rack-e82f06b354fa/lib/rack/session/abstract/id.rb:185:in `stringify_keys'
/Users/olehd/.rvm/gems/ruby-2.3.7/bundler/gems/rack-e82f06b354fa/lib/rack/session/abstract/id.rb:121:in `update'
/Users/olehd/.rvm/gems/ruby-2.3.7/bundler/gems/rack-e82f06b354fa/lib/rack/session/abstract/id.rb:290:in `prepare_session'
/Users/olehd/.rvm/gems/ruby-2.3.7/bundler/gems/rack-e82f06b354fa/lib/rack/session/abstract/id.rb:249:in `context'
/Users/olehd/.rvm/gems/ruby-2.3.7/bundler/gems/rack-e82f06b354fa/lib/rack/session/abstract/id.rb:244:in `call'
/Users/olehd/.rvm/gems/ruby-2.3.7/bundler/gems/rack-e82f06b354fa/lib/rack/urlmap.rb:77:in `block in call'
/Users/olehd/.rvm/gems/ruby-2.3.7/bundler/gems/rack-e82f06b354fa/lib/rack/urlmap.rb:61:in `each'
/Users/olehd/.rvm/gems/ruby-2.3.7/bundler/gems/rack-e82f06b354fa/lib/rack/urlmap.rb:61:in `call'
/Users/olehd/.rvm/gems/ruby-2.3.7/bundler/gems/rack-e82f06b354fa/lib/rack/builder.rb:176:in `call'
sidekiq (5.2.7) lib/sidekiq/web.rb:103:in `call'
sidekiq (5.2.7) lib/sidekiq/web.rb:108:in `call'
actionpack (5.0.7.2) lib/action_dispatch/routing/mapper.rb:17:in `block in <class:Constraints>'
actionpack (5.0.7.2) lib/action_dispatch/routing/mapper.rb:46:in `serve'
actionpack (5.0.7.2) lib/action_dispatch/journey/router.rb:39:in `block in serve'
actionpack (5.0.7.2) lib/action_dispatch/journey/router.rb:26:in `each'
actionpack (5.0.7.2) lib/action_dispatch/journey/router.rb:26:in `serve'
actionpack (5.0.7.2) lib/action_dispatch/routing/route_set.rb:727:in `call'

Gems Versions:
ruby - 2.3.7
rails - 5.0.7.2
sidekiq - 5.2.7
rack - master branch

This PR solved issue.

@oleh-demyanyuk oleh-demyanyuk changed the title [WIP] Fix: Add `to_hash` to wrap Hash and Session classes on `stringify_keys` Fix: Add `to_hash` to wrap Hash and Session classes on `stringify_keys` Dec 16, 2019
@ahmadhasankhan

This comment has been minimized.

Copy link

ahmadhasankhan commented Jan 11, 2020

I am also facing this issue

NoMethodError (undefined method `transform_keys' for #<ActionDispatch::Request::Session:0x7fe692288f68 not yet loaded>):
  
rack (2.1.0) lib/rack/session/abstract/id.rb:212:in `stringify_keys'
rack (2.1.0) lib/rack/session/abstract/id.rb:148:in `update'
rack (2.1.0) lib/rack/session/abstract/id.rb:317:in `prepare_session'
rack (2.1.0) lib/rack/session/abstract/id.rb:276:in `context'
rack (2.1.0) lib/rack/session/abstract/id.rb:271:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack (2.1.0) lib/rack/urlmap.rb:77:in `block in call'
rack (2.1.0) lib/rack/urlmap.rb:61:in `each'
rack (2.1.0) lib/rack/urlmap.rb:61:in `call'
rack (2.1.0) lib/rack/builder.rb:176:in `call'
rollbar (2.23.1) lib/rollbar/middleware/rack/builder.rb:16:in `block in call_with_rollbar'
rollbar (2.23.1) lib/rollbar.rb:145:in `scoped'
rollbar (2.23.1) lib/rollbar/middleware/rack/builder.rb:14:in `call_with_rollbar'
sidekiq (6.0.4) lib/sidekiq/web.rb:104:in `call'
sidekiq (6.0.4) lib/sidekiq/web.rb:109:in `call'
actionpack (5.0.7.2) lib/action_dispatch/routing/mapper.rb:17:in `block in <class:Constraints>'
actionpack (5.0.7.2) lib/action_dispatch/routing/mapper.rb:46:in `serve'
actionpack (5.0.7.2) lib/action_dispatch/journey/router.rb:39:in `block in serve'
actionpack (5.0.7.2) lib/action_dispatch/journey/router.rb:26:in `each'
actionpack (5.0.7.2) lib/action_dispatch/journey/router.rb:26:in `serve'
actionpack (5.0.7.2) lib/action_dispatch/routing/route_set.rb:727:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/rack/agent_hooks.rb:30:in `traced_call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/rack/browser_monitoring.rb:32:in `traced_call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
scout_apm (2.6.6) lib/scout_apm/instant/middleware.rb:53:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
scout_apm (2.6.6) lib/scout_apm/middleware.rb:20:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
warden (1.2.8) lib/warden/manager.rb:36:in `block in call'
warden (1.2.8) lib/warden/manager.rb:34:in `catch'
warden (1.2.8) lib/warden/manager.rb:34:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack (2.1.0) lib/rack/etag.rb:27:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack (2.1.0) lib/rack/conditional_get.rb:27:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack (2.1.0) lib/rack/head.rb:14:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack (2.1.0) lib/rack/session/abstract/id.rb:277:in `context'
rack (2.1.0) lib/rack/session/abstract/id.rb:271:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
actionpack (5.0.7.2) lib/action_dispatch/middleware/cookies.rb:613:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
activerecord (5.0.7.2) lib/active_record/migration.rb:553:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
actionpack (5.0.7.2) lib/action_dispatch/middleware/callbacks.rb:38:in `block in call'
activesupport (5.0.7.2) lib/active_support/callbacks.rb:97:in `__run_callbacks__'
activesupport (5.0.7.2) lib/active_support/callbacks.rb:750:in `_run_call_callbacks'
activesupport (5.0.7.2) lib/active_support/callbacks.rb:90:in `run_callbacks'
actionpack (5.0.7.2) lib/action_dispatch/middleware/callbacks.rb:36:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
actionpack (5.0.7.2) lib/action_dispatch/middleware/executor.rb:12:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
actionpack (5.0.7.2) lib/action_dispatch/middleware/remote_ip.rb:79:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rollbar (2.23.1) lib/rollbar/middleware/rails/rollbar.rb:25:in `block in call'
rollbar (2.23.1) lib/rollbar.rb:145:in `scoped'
rollbar (2.23.1) lib/rollbar/middleware/rails/rollbar.rb:22:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
actionpack (5.0.7.2) lib/action_dispatch/middleware/debug_exceptions.rb:49:in `call'
rollbar (2.23.1) lib/rollbar/middleware/rails/show_exceptions.rb:22:in `call_with_rollbar'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
web-console (3.7.0) lib/web_console/middleware.rb:135:in `call_app'
web-console (3.7.0) lib/web_console/middleware.rb:30:in `block in call'
web-console (3.7.0) lib/web_console/middleware.rb:20:in `catch'
web-console (3.7.0) lib/web_console/middleware.rb:20:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
actionpack (5.0.7.2) lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
railties (5.0.7.2) lib/rails/rack/logger.rb:36:in `call_app'
railties (5.0.7.2) lib/rails/rack/logger.rb:24:in `block in call'
activesupport (5.0.7.2) lib/active_support/tagged_logging.rb:69:in `block in tagged'
activesupport (5.0.7.2) lib/active_support/tagged_logging.rb:26:in `tagged'
activesupport (5.0.7.2) lib/active_support/tagged_logging.rb:69:in `tagged'
railties (5.0.7.2) lib/rails/rack/logger.rb:24:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
sprockets-rails (3.2.1) lib/sprockets/rails/quiet_assets.rb:13:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack-timeout (0.6.0) lib/rack/timeout/core.rb:151:in `block in call'
rack-timeout (0.6.0) lib/rack/timeout/support/timeout.rb:19:in `timeout'
rack-timeout (0.6.0) lib/rack/timeout/core.rb:150:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
actionpack (5.0.7.2) lib/action_dispatch/middleware/request_id.rb:24:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack (2.1.0) lib/rack/method_override.rb:24:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack (2.1.0) lib/rack/runtime.rb:24:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack-timeout (0.6.0) lib/rack/timeout/core.rb:151:in `block in call'
rack-timeout (0.6.0) lib/rack/timeout/support/timeout.rb:19:in `timeout'
rack-timeout (0.6.0) lib/rack/timeout/core.rb:150:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
activesupport (5.0.7.2) lib/active_support/cache/strategy/local_cache_middleware.rb:28:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
actionpack (5.0.7.2) lib/action_dispatch/middleware/executor.rb:12:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
actionpack (5.0.7.2) lib/action_dispatch/middleware/static.rb:136:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
rack (2.1.0) lib/rack/sendfile.rb:113:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
railties (5.0.7.2) lib/rails/engine.rb:522:in `call'
newrelic_rpm (6.8.0.360) lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
puma (3.8.2) lib/puma/configuration.rb:224:in `call'
puma (3.8.2) lib/puma/server.rb:600:in `handle_request'
puma (3.8.2) lib/puma/server.rb:435:in `process_client'
puma (3.8.2) lib/puma/server.rb:299:in `block in run'
puma (3.8.2) lib/puma/thread_pool.rb:120:in `block in spawn_thread'

@ioquatix

This comment has been minimized.

Copy link
Member

ioquatix commented Jan 11, 2020

What is the value of other in the failing case?

Do we prefer to_h or to_hash?

cc @tenderlove @rafaelfranca

@ahorek

This comment has been minimized.

Copy link

ahorek commented Jan 11, 2020

in my opinion, #1401 and #1391 should be reverted

adding #to_hash will fix the issue, but the original performance improvement for hash arguments is gone
Rack::Session::Abstract::SessionHash is actually 40% slower
and ActionDispatch::Request::Session is broken

@ioquatix

This comment has been minimized.

Copy link
Member

ioquatix commented Jan 11, 2020

@ahorek I don't disagree with you.

I can merge this, and then can you make a follow up PR to improve both performance and spec coverage on the downstream requirements? We can get it out in 2.2.0

@ahorek

This comment has been minimized.

Copy link

ahorek commented Jan 11, 2020

yes, I think it's ok to merge.

after this PR, the change in #1401 should be reverted anyway, because
Rack::Session::Abstract::SessionHash does respond #to_hash and the custom #transform_keys method is no longer necessary

@ioquatix ioquatix merged commit 8659d9f into rack:master Jan 11, 2020
@ioquatix

This comment has been minimized.

Copy link
Member

ioquatix commented Jan 11, 2020

Can you make a PR to revert #1401.

@kapso

This comment has been minimized.

Copy link

kapso commented Jan 14, 2020

This still seems to be an issue for me with Rack v2.1.1

@PikachuEXE

This comment has been minimized.

Copy link

PikachuEXE commented Jan 14, 2020

@kapso The revert of #1401 is not included in 2.1.1
image

@Rim-777

This comment has been minimized.

Copy link

Rim-777 commented Jan 14, 2020

Hello, I have the same issue with Rack v2.1.1, could anyone please give me a hint what need I do to solve it?

@mati0090

This comment has been minimized.

Copy link

mati0090 commented Jan 14, 2020

In your Gemfile use following line as temporary solution:
gem 'rack', github: 'rack/rack', ref: 'f690bb71425aa31d7b9b3113829af773950d8ab5'

@garside

This comment has been minimized.

Copy link

garside commented Jan 15, 2020

If you need to deploy this to heroku, use this format in the gemfile instead or your build will fail with private key errors.

gem 'rack', github: 'rack/rack', :ref => 'f690bb71425aa31d7b9b3113829af773950d8ab5'
@Rim-777

This comment has been minimized.

Copy link

Rim-777 commented Jan 15, 2020

@mati0090 Thank you, it helped.
@garside Thank you too

@jjstafford

This comment has been minimized.

Copy link

jjstafford commented Jan 15, 2020

@garside that worked great. Hopefully this is merged soon....

@saiqulhaq

This comment has been minimized.

Copy link

saiqulhaq commented Jan 16, 2020

can we have v2.1.1.1 version to release this?

jholton added a commit to department-of-veterans-affairs/vets-api that referenced this pull request Jan 16, 2020
…ile we wait for a gem release.

rack/rack#1428
@jholton jholton mentioned this pull request Jan 16, 2020
5 of 7 tasks complete
jholton added a commit to department-of-veterans-affairs/vets-api that referenced this pull request Jan 16, 2020
* Update Rack gem to reference a specific Github commit to fix a bug while we wait for a gem release.

rack/rack#1428
@chongfai13

This comment has been minimized.

Copy link

chongfai13 commented Jan 17, 2020

In your Gemfile use following line as temporary solution:
gem 'rack', github: 'rack/rack', ref: 'f690bb71425aa31d7b9b3113829af773950d8ab5'

I encountered this error after adding

Could not find gem 'browserify-:q!ils' in any of the gem sources listed in your Gemfile or in gems cached in vendor/cache.

@saiqulhaq

This comment has been minimized.

Copy link

saiqulhaq commented Jan 17, 2020

In your Gemfile use following line as temporary solution:
gem 'rack', github: 'rack/rack', ref: 'f690bb71425aa31d7b9b3113829af773950d8ab5'

I encountered this error after adding

Could not find gem 'browserify-:q!ils' in any of the gem sources listed in your Gemfile or in gems cached in vendor/cache.

your issue is not related to rack
this is my Gemfile
image
I updated it 2 hours ago without any problem after bundle install

@chongfai13

This comment has been minimized.

Copy link

chongfai13 commented Jan 17, 2020

In your Gemfile use following line as temporary solution:
gem 'rack', github: 'rack/rack', ref: 'f690bb71425aa31d7b9b3113829af773950d8ab5'

I encountered this error after adding
Could not find gem 'browserify-:q!ils' in any of the gem sources listed in your Gemfile or in gems cached in vendor/cache.

your issue is not related to rack
this is my Gemfile
image
I updated it 2 hours ago without any problem after bundle install

It works! Terima Kasih banyak banyak

@ioquatix ioquatix modified the milestone: v2.1.2 Jan 17, 2020
@ioquatix ioquatix modified the milestones: v2.2.0, v2.1.2 Jan 17, 2020
@ioquatix

This comment has been minimized.

Copy link
Member

ioquatix commented Jan 17, 2020

@tenderlove should we backport this and cut a v2.1.2 release or wait until a v2.2.0 release?

@natematykiewicz

This comment has been minimized.

Copy link

natematykiewicz commented Jan 18, 2020

I’d love a release. My Sidekiq admin page is busted and it took my a while to figure out what the problem was.

@estebanbouza

This comment has been minimized.

Copy link

estebanbouza commented Jan 18, 2020

This approach works on development:

gem 'rack', github: 'rack/rack', ref: 'f690bb71425aa31d7b9b3113829af773950d8ab5'

But in Production, I get:

Step #1 - "docker-build": Step 10/11 : RUN cd /app && RAILS_ENV=production SECRET_KEY_BASE=xxx rails assets:precompile
Step #1 - "docker-build": ---> Running in 57e0841136
Step #1 - "docker-build": /usr/local/lib/ruby/2.6.0/rubygems/dependency.rb:311:in `to_specs': Could not find 'rack' (~> 2.0, >= 2.0.8) among 228 total gem(s) (Gem::MissingSpecError)
Step #1 - "docker-build": Checked in 'GEM_PATH=/xxx/.gem/ruby/2.6.0:/usr/local/lib/ruby/gems/2.6.0:/usr/local/bundle', execute `gem env` for more information
Step #1 - "docker-build": from /usr/local/lib/ruby/2.6.0/rubygems/specification.rb:1449:in `block in activate_dependencies'

Which seems to be the expected behavior based on bundler doc

Because RubyGems lacks the ability to handle gems from git, any gems installed from a git repository will not show up in gem list.

Any clue? A new release would be appreciated otherwise.

@Shleeble

This comment has been minimized.

Copy link

Shleeble commented Jan 20, 2020

ETA til 2.1.2?

@springerigor

This comment has been minimized.

Copy link

springerigor commented Jan 21, 2020

If you are waiting for the 2.1.2 you can temporarily monkey patch rack:

Fo Rails projects you can create e.g. config/initializers/rack_monkey_path.rb file:

# Monkey patch `stringify_keys` to make rack 2.1.1 compatible with Sidekiq UI Admin panel.
# Should be removed when 2.1.2 is released.
# https://github.com/rack/rack/pull/1428
module Rack
  module Session
    module Abstract
      class SessionHash
        private

        def stringify_keys(other)
          other.to_hash.transform_keys(&:to_s)
        end
      end
    end
  end
end
@Rim-777

This comment has been minimized.

Copy link

Rim-777 commented Jan 21, 2020

If you need to deploy this to heroku, use this format in the gemfile instead or your build will fail with private key errors.

gem 'rack', github: 'rack/rack', :ref => 'f690bb71425aa31d7b9b3113829af773950d8ab5'

@garside I got some problems with deploy on AWS. is there an appropriate ref hash for that?

@mstruve mstruve mentioned this pull request Jan 21, 2020
2 of 2 tasks complete
camallen added a commit to zooniverse/caesar that referenced this pull request Jan 22, 2020
@camallen camallen mentioned this pull request Jan 22, 2020
camallen added a commit to zooniverse/caesar that referenced this pull request Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.