search

ruby-grape / grape

An opinionated framework for creating REST-like APIs in Ruby.
http://www.ruby-grape.org
MIT License
9.89k stars 1.22k forks source link

Returning an empty hash in a HTTP delete results in an exception #1768

Open basjanssen opened 6 years ago

basjanssen commented 6 years ago

After upgrading to Grape >= 0.19.1, the default status code for DELETE is now 204 instead of 200. When returning a response which is not empty (for example {'key'=>'value'}, a 200 is returned with the correct response body.

When a non empty hash is returned on webrick (default development rails server) this goes well: the hash is returned. However, if an empty hash is provided {}, it results in an exception: 

[2018-07-10 15:38:03] ERROR TypeError: no implicit conversion of Hash into String
    /Users/bas.janssen/.rvm/gems/ruby-2.4.4@rails4/gems/rack-1.6.10/lib/rack/handler/webrick.rb:113:in `block in service'
    /Users/bas.janssen/.rvm/gems/ruby-2.4.4@rails4/gems/rack-1.6.10/lib/rack/body_proxy.rb:31:in `each'
    /Users/bas.janssen/.rvm/gems/ruby-2.4.4@rails4/gems/rack-1.6.10/lib/rack/body_proxy.rb:31:in `each'
    /Users/bas.janssen/.rvm/gems/ruby-2.4.4@rails4/gems/rack-1.6.10/lib/rack/body_proxy.rb:31:in `each'
    /Users/bas.janssen/.rvm/gems/ruby-2.4.4@rails4/gems/rack-1.6.10/lib/rack/body_proxy.rb:31:in `each'
    /Users/bas.janssen/.rvm/gems/ruby-2.4.4@rails4/gems/rack-1.6.10/lib/rack/body_proxy.rb:31:in `each'
    /Users/bas.janssen/.rvm/gems/ruby-2.4.4@rails4/gems/rack-1.6.10/lib/rack/body_proxy.rb:31:in `each'
    /Users/bas.janssen/.rvm/gems/ruby-2.4.4@rails4/gems/rack-1.6.10/lib/rack/body_proxy.rb:31:in `each'
    /Users/bas.janssen/.rvm/gems/ruby-2.4.4@rails4/gems/rack-1.6.10/lib/rack/handler/webrick.rb:112:in `service'
    /Users/bas.janssen/.rvm/rubies/ruby-2.4.4/lib/ruby/2.4.0/webrick/httpserver.rb:140:in `service'
    /Users/bas.janssen/.rvm/rubies/ruby-2.4.4/lib/ruby/2.4.0/webrick/httpserver.rb:96:in `run'
    /Users/bas.janssen/.rvm/rubies/ruby-2.4.4/lib/ruby/2.4.0/webrick/server.rb:308:in `block in start_thread'

with the following output (http 500): 

<HTML>
    <HEAD>
        <TITLE>Internal Server Error</TITLE>
    </HEAD>
    <BODY>
        <H1>Internal Server Error</H1>
    no implicit conversion of Hash into String
        <HR>
        <ADDRESS>
     WEBrick/1.3.1 (Ruby/2.4.4/2018-03-28) at
     localhost:3005
    </ADDRESS>
    </BODY>
</HTML>

Adding an explicit status of 200 solves this:

delete do
  status 200
  {}
end

Is this issue known, can someone confirm this and can it be solved in some way?

basjanssen commented 6 years ago

When investigating in the grape code, I see that inside_route wil check for a value in the body using .present? This will return false if an empty hash is used. I'm not sure if this should be the case. Returning an empty json object is done returning {}. The body is clearly not empty: it returns a valid json object. However: the grape implementation would regard this as no content -> 204.

dblock commented 6 years ago

Interesting, I think this is a bug. Try writing a test for it, maybe a fix?

basjanssen commented 6 years ago

I love to get some feedback. As a grape rookie, I hope I'm on the right track.