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

Invalid rack response. #1572

Closed
tonobo opened this issue Feb 9, 2017 · 20 comments
Closed

Invalid rack response. #1572

tonobo opened this issue Feb 9, 2017 · 20 comments
Labels

Comments

@tonobo
Copy link

tonobo commented Feb 9, 2017

Please see here puma/puma#1208

@namusyaka
Copy link
Contributor

namusyaka commented Feb 9, 2017

@dopykuh Could you provide minimal failing project as github repository?

@tonobo
Copy link
Author

tonobo commented Feb 9, 2017

These 2 lines will produce my exception dab3c09#diff-e9e8424a5238d48301e313d8fe285697

@dblock dblock added the bug? label Feb 9, 2017
@dblock
Copy link
Member

dblock commented Feb 9, 2017

I wouldn't be so sure that this is a grape bug. It looks like the response is a hash that is not being formatted into a string. Which content type is here? What is the response from the DELETE (implemented by the user)?

@tonobo
Copy link
Author

tonobo commented Feb 9, 2017

Request on v.18.0

curl localhost:9000/api/v1/ldap?mac=00%3A50%3A56%3A00%3A00%3A00 -X DELETE -i; echo
HTTP/1.1 201 Created
Content-Type: application/json
Content-Length: 96

{"ip":"10.10.10.1","mac":"00:50:56:00:00:00","gateway":"10.10.10.10","netmask":"255.255.255.224"}

Request on v.19.0

curl localhost:9000/api/v1/ldap?mac=00%3A50%3A56%3A00%3A00%3A00 -X DELETE -i; echo
HTTP/1.1 500 Internal Server Error


@tonobo
Copy link
Author

tonobo commented Feb 9, 2017

The issue could also been appeared earlier, but it looks like an grape bug. You only setup a default right now, but response_body shouldn't be served as hash or not?

@dblock
Copy link
Member

dblock commented Feb 9, 2017

This is a little more complicated. In 0.19.0 we made the server return 204 on a DELETE, then in 0.19.1 we fixed #1550 to return 200 when there's no content. That's ultimately what's causing this, but it's not necessarily a bug.

So what you need to find out is why isn't your JSON formatter invoked on a DELETE. Wrong content type? Is JSON not the default formatter? I would definitely try explicitly 0.19.1, then HEAD, then I would dig through the code - maybe you have a custom formatter or something else in the way?

As a workaround you can probably add in status 200 in your API response and the problem will go away.

@tonobo
Copy link
Author

tonobo commented Feb 9, 2017

Ok, i'll check this tomorrow. I also tested head and 019.1, but how is it possible that the formatters are completly skipped, it would be nice if a fallback will raise or log a warning or sth. else. I'll try to find out where it comes from, i am able to reproduce them within my environment. ;)

@dblock
Copy link
Member

dblock commented Feb 9, 2017

I am just speculating from the top of my head. Build a simple repro, maybe even a spec.

@tonobo
Copy link
Author

tonobo commented Feb 9, 2017

I am unable to reproduce it out of my app environments. Currently i don't know why...
Alternativly i switch my job to gardener. ;)

@dblock
Copy link
Member

dblock commented Feb 9, 2017

@LeFnord
Copy link
Member

LeFnord commented Feb 10, 2017

Hi @dopykuh … how did you achieve this behavior?

curl localhost:9000/api/v1/ldap?mac=00%3A50%3A56%3A00%3A00%3A00 -X DELETE -i; echo
HTTP/1.1 201 Created

you make a DELETE request, but your response assumes, you made a POST request

for further investigating, I suggest to pry into a rescue from all block →

rescue_from :all do |error|
  require 'pry'; binding.pry
end

to catch the errors and see what is really happened

@dblock one minor correction, in 0.19.0 status 204 was introduced as default for delete requests with an empty body, in 0.19.1, it was completed, to return 200 if the response did have a body 😉

@tonobo
Copy link
Author

tonobo commented Feb 10, 2017

@LeFnord sorry, wrong output posted.

HTTP/1.1 200 OK
Content-Type: application/json
Content-Length: 163

json body...

@tonobo
Copy link
Author

tonobo commented Feb 10, 2017

@LeFnord its also not possible to rescue there because the exception will be raised within puma response handler due to unformatted response. Grape possibly skips the formatters so the impact is only visible later.

@tonobo
Copy link
Author

tonobo commented Feb 10, 2017

The exception doesn't occur by manually setting status 200

@tonobo
Copy link
Author

tonobo commented Feb 10, 2017

v0.19.0...v0.19.1#diff-e9e8424a5238d48301e313d8fe285697R133

It looks like the body isn't already detected here, but will be served later. So it returned a 204 with content, which will trigger the issue?

@dblock
Copy link
Member

dblock commented Feb 10, 2017

You narrowed down the problem, 204 means no content, so you should not have content with a 204. Now find how this happens.

@tonobo
Copy link
Author

tonobo commented Feb 10, 2017

@dblock I'll take a look at monday. Could you give some tips, or do you have any ideas where the issue possibly come from? I don't know anything about your framework structure and debugging large dsl's are not as easy. :D

@dblock
Copy link
Member

dblock commented Feb 10, 2017

I don't think I have much more useful info to add ;( Let us know what you find! If you put a repro up obviously it would be helpful.

@tonobo
Copy link
Author

tonobo commented Feb 10, 2017

I tried, but i am unable to reproduce them. Don't know why ...

dblock pushed a commit that referenced this issue Feb 19, 2017
@dblock
Copy link
Member

dblock commented Feb 19, 2017

Fixed in #1579.

@dblock dblock closed this as completed Feb 19, 2017
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