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

Unexpected error while processing request: undefined method `downcase' for 2:Fixnum #1059

Open
patfrat opened this issue Jul 8, 2015 · 18 comments
Labels

Comments

@patfrat
Copy link

patfrat commented Jul 8, 2015

My Grape API is not working with grape v0.11.0 nor grape v0.12.0
Working with grape v0.10.0

I am using Rack Cascasde to load my API, ApiDoc with grape_swagger and my statics files
Using grape-activerecord v0.0.8, grape-entity 0.4.5 and grape-swagger 0.10.1

My database connection is loaded before invoking the API

It's only on the first GET that it fails
And it's difficult to see why and where in my project with this kind of trace !

Unexpected error while processing request: undefined method `downcase' for 2:Fixnum
    /var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/utils.rb:501:in `[]'
    /var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/response.rb:53:in `[]'
    /var/lib/gems/2.1.0/gems/activerecord-4.2.1/lib/active_record/connection_adapters/abstract/connection_pool.rb:650:in `call'
    /var/lib/gems/2.1.0/gems/grape-0.12.0/lib/grape/middleware/error.rb:27:in `block in call!'
    /var/lib/gems/2.1.0/gems/grape-0.12.0/lib/grape/middleware/error.rb:26:in `catch'
    /var/lib/gems/2.1.0/gems/grape-0.12.0/lib/grape/middleware/error.rb:26:in `call!'
    /var/lib/gems/2.1.0/gems/grape-0.12.0/lib/grape/middleware/base.rb:18:in `call'
    /var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/head.rb:13:in `call'
    /var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/builder.rb:153:in `call'
    /var/lib/gems/2.1.0/gems/grape-0.12.0/lib/grape/endpoint.rb:196:in `call!'
    /var/lib/gems/2.1.0/gems/grape-0.12.0/lib/grape/endpoint.rb:184:in `call'
    /var/lib/gems/2.1.0/gems/rack-mount-0.8.3/lib/rack/mount/route_set.rb:152:in `block in call'
    /var/lib/gems/2.1.0/gems/rack-mount-0.8.3/lib/rack/mount/code_generation.rb:96:in `block in recognize'
    /var/lib/gems/2.1.0/gems/rack-mount-0.8.3/lib/rack/mount/code_generation.rb:68:in `optimized_each'
    /var/lib/gems/2.1.0/gems/rack-mount-0.8.3/lib/rack/mount/code_generation.rb:95:in `recognize'
    /var/lib/gems/2.1.0/gems/rack-mount-0.8.3/lib/rack/mount/route_set.rb:141:in `call'
    /var/lib/gems/2.1.0/gems/grape-0.12.0/lib/grape/api.rb:98:in `call'
    /var/lib/gems/2.1.0/gems/grape-0.12.0/lib/grape/api.rb:33:in `call!'
    /var/lib/gems/2.1.0/gems/grape-0.12.0/lib/grape/api.rb:29:in `call'
    /var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/cascade.rb:33:in `block in call'
    /var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/cascade.rb:24:in `each'
    /var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/cascade.rb:24:in `call'
    /var/lib/gems/2.1.0/gems/rack-contrib-1.3.0/lib/rack/contrib/post_body_content_type_parser.rb:36:in `call'
    /var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/commonlogger.rb:33:in `call'
    /var/lib/gems/2.1.0/gems/sinatra-1.4.6/lib/sinatra/base.rb:218:in `call'
    /var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/deflater.rb:35:in `call'
    /var/lib/gems/2.1.0/gems/thin-1.6.3/lib/thin/connection.rb:86:in `block in pre_process'
    /var/lib/gems/2.1.0/gems/thin-1.6.3/lib/thin/connection.rb:84:in `catch'
    /var/lib/gems/2.1.0/gems/thin-1.6.3/lib/thin/connection.rb:84:in `pre_process'
    /var/lib/gems/2.1.0/gems/eventmachine-1.0.7/lib/eventmachine.rb:1054:in `call'
    /var/lib/gems/2.1.0/gems/eventmachine-1.0.7/lib/eventmachine.rb:1054:in `block in spawn_threadpool'
@dblock
Copy link
Member

dblock commented Jul 8, 2015

Do you think you could put a small project up that reproduces this?

@dblock dblock added the bug? label Jul 8, 2015
@patfrat
Copy link
Author

patfrat commented Jul 8, 2015

I will try it tomorrow

@patfrat
Copy link
Author

patfrat commented Jul 9, 2015

Using pry to debug ...

grape 0.10.0 send an Array as response [200, {header}, {body}]
Activerecord can use reponse[2] as body

grape 0.10.0 reading swagger_doc

[200, {"Access-Control-Allow-Origin"=>"*", "Access-Control-Request-Method"=>"*", "Content-Type"=>"application/json", "Content-Length"=>"1193"}, #<Rack::BodyProxy:0x00000005b79120 @body=#<Rack::Response:0x00000005b79648 @status=200, @header={"Access-Control-Allow-Origin"=>"*", "Access-Control-Request-Method"=>"*", "Content-Type"=>"application/json", "Content-Length"=>"1193"}, @chunked=false, @writer=#<Proc:0x00000005b792b0@/var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/response.rb:30 (lambda)>, @block=nil, @length=1193, @body=[...] ...

grape 0.12.0 send a Rack::Response and activerecord failed with rack on undefined method `downcase' for 2:Fixnum

grape 0.12.0 reading swagger_doc

#<Rack::Response:0x0000000481d9c8 @status=200, @header={"Access-Control-Allow-Origin"=>"*", "Access-Control-Request-Method"=>"*", "Content-Type"=>"application/json", "Content-Length"=>"1193"}, @chunked=false, @writer=#<Proc:0x0000000481d748@/var/lib/gems/2.1.0/gems/rack-1.6.4/lib/rack/response.rb:30 (lambda)>, @block=nil, @length=1193, @body=[...] ...

If i transform the Rack::Response to an array, it could work !
So, is it a grape bug? or not ?

@patfrat
Copy link
Author

patfrat commented Jul 9, 2015

If i set this is connection_pool.rb in activerecord line 649

response = response.to_a if !response.is_a?(Array)

It works ! but this is not a solution to "monkey patch" activerecord !

@dblock
Copy link
Member

dblock commented Jul 9, 2015

A grape middleware could do the to_a, but something tells me this is just a consequence of something else. Put up a project with a repro please and I'll help debug.

@patfrat
Copy link
Author

patfrat commented Jul 9, 2015

It will be difficult for me to put up a project with all dependencies in a short time.
I will try to debug with pry and pry-byebug to search where the problem could be localized

@dblock
Copy link
Member

dblock commented Jul 9, 2015

Do you use some connection pooling middleware anywhere explicitly via a use?

FWIW https://github.com/dblock/grape-on-rails worked without changes with Grape 0.12.0.

The clean way of working around this is to insert a middleware ahead of the ActiveRecord one that calls to_a on the Rack::Response.

class RackResponseMiddleware
  def initialize(app)
    @app = app       
  end                

  def self.call(env)      
    response = @app.call(env)   
    response = response.to_a if response.is_a?(Rack::Response)
    response
  end                
end                  
# config.ru
use RackResponseMiddleware

@patfrat
Copy link
Author

patfrat commented Jul 9, 2015

No, i don't use a connection pooling middleware via a use

I establish a connection to the database in an initializer via ActiveRecord::Base.establish_connection ...
I have tried your solution but it does not work.
I have had to correct def self.call(env) to def.call(env) to have RackResponseMiddleware working but the bug continue to appear

If i create a monkey patch for ActiveRecord, grape 0.12.0 works

module ActiveRecord
  module ConnectionAdapters
    class ConnectionManagement
      def initialize(app)
        @app = app
      end

      def call(env)
        testing = env['rack.test']
        response = @app.call(env)
        response = response.to_a if response.is_a?(Rack::Response)
        response[2] = ::Rack::BodyProxy.new(response[2]) do
          ActiveRecord::Base.clear_active_connections! unless testing
        end

        response
      rescue Exception
        ActiveRecord::Base.clear_active_connections! unless testing
        raise
      end
    end
  end
end

@patfrat
Copy link
Author

patfrat commented Jul 9, 2015

Working with your solution with grape 0.12.0 without monkey patching ActiveRecord

I have set this in an initializer/90_rackresponse_middleware.rb file loaded after establishing connection but before mounting my API

class RackResponseMiddleware
  def initialize(app)
    @app = app       
  end                

  def call(env)      
    response = @app.call(env)   
    response = response.to_a if response.is_a?(Rack::Response)
    response
  end                
end     

And in my API class

class Api < Grape::API
  include Grape::ActiveRecord::Extension
  use RackResponseMiddleware
  ...

Thank you !

@dblock
Copy link
Member

dblock commented Jul 9, 2015

I think we really need to understand whether this is the right solution and document it. But not without a setup that reproduces the problem in which we understand what's going on. Note that the above patch prevents streaming, and while it's maybe a 1% scenario it still means someone is doing something wrong somewhere ;)

Leaving this open.

@patfrat
Copy link
Author

patfrat commented Jul 10, 2015

I have reproduced the bug with a short code that you could retrieve here
https://gist.github.com/patfrat/d4463a85d327300a54d8

It seems to become from Grape::ActiveRecord extension

@patfrat
Copy link
Author

patfrat commented Jul 10, 2015

For informations, I use include Grape::ActiveRecord::Extension to have rake tasks associated to my project

@patfrat
Copy link
Author

patfrat commented Jul 23, 2015

I got finally another issue with database connection pools.
This middleware fix it and fix also the 2:Fixnum error when using grape with activerecord without rails

# Middleware to clear sleeping connections with ActiveRecord/Grape
# @see https://github.com/intridea/grape/issues/517
class GrapeActiveRecordMiddleware
  def initialize(app)
    @app = app
  end

  def call(env)
    ActiveRecord::Base.connection_pool.connections.map(&:verify!)
    status, headers, bodies = catch(:error) do
      @app.call(env)
    end

    ActiveRecord::Base.clear_active_connections!
    if status.is_a?(Hash)
      throw :error, status
    else
      [status, headers, bodies]
    end
  end
end

class API < Grape::API
include Grape::ActiveRecord::Extension
  use GrapeActiveRecordMiddleware
  ...

@dblock
Copy link
Member

dblock commented Jul 23, 2015

@patfrat I encourage you to write some tests, release this as a gem and we should link it from Grape README.

@patfrat
Copy link
Author

patfrat commented Jul 23, 2015

@dblock when I have time, I will do that !

terceiro added a commit to terceiro/grape_logging that referenced this issue Jul 23, 2015
Sometimes `response` is not what you think it is. `@app_response` is
always a proper Rack::Response object so it's safe to get its status.

See ruby-grape/grape#1059
@stevendaniels
Copy link

Recently encountered a similar issue. Is this issue related to the fact that Grape::Middleware::Base#call returns an instance of Rack::Response instead of an array?

In ActiveRecord 4.2.x's Rack middleware, it does this:

def call(env)
  ...
  response[2] = ::Rack::BodyProxy.new(response[2]) do
    ActiveRecord::Base.clear_active_connections! unless testing
  end
  ...
end

Specifically, response[2] fails because Rack::Response#[] is an alias for Rack::Response.get_header and 2 does not have a #downcase method!

@dblock
Copy link
Member

dblock commented Nov 18, 2016

Yes @stevendaniels. We went back and forth on this and somewhere decided that we should continue returning a Rack::Response. Maybe Grape's should implement []? I am not sure what the right thing to do is.

@qd3v
Copy link

qd3v commented Jan 6, 2017

In case you're using otr-activerecord (formerly known as grape-activerecord) and don't have time to find a workaround, just make sure you call

use OTR::ActiveRecord::ConnectionManagement

inside config.ru not in Grape::API subclass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants