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

Enable returning 204 No Content #645

Closed
p-originate opened this issue May 15, 2014 · 12 comments · Fixed by #850
Closed

Enable returning 204 No Content #645

p-originate opened this issue May 15, 2014 · 12 comments · Fixed by #850

Comments

@p-originate
Copy link

Per the readme, a formatter must supply a content type. However, when sending an empty response body, no content type is wanted at all.

Related issue is #215. As far as I see even with my own formatter I cannot return empty response bodies/204 no content.

@dblock dblock changed the title Returning an empty response body is impossible Enable returning 204 No Content May 15, 2014
@dblock
Copy link
Member

dblock commented May 15, 2014

I've renamed this to "Enable returning 204 No Content".

See #618 for a possible solution, still looking forward to a PR.

@p-originate
Copy link
Author

I do not want to serve a text/plain response to a request that expects a json response (http://.../foo.json).

An empty string, not being valid json, also won't work.

@p-originate
Copy link
Author

I don't know if I will be the person sending requested PR, but, do you care what the api for this functionality looks like?

@dblock
Copy link
Member

dblock commented May 16, 2014

So, if you want to return valid JSON, you should return {}, but a 204 No Content (or similar) is definitely a valid response too.

I really don't have a strong opinion of what the API looks like, but I can comment when I see a suggestion :)

@dblock
Copy link
Member

dblock commented Dec 16, 2014

I chose body false to return a 204 No Content, which is just an alias for:

@body = nil
status 204

#850

@stephancom
Copy link

stephancom commented Mar 27, 2018

I realize this is closed, but I wanted to share an issue I just ran into the hard way.
Apparently, @body = nil will cause errors if you're using Rack::Lint.

ERROR -- : app error: Body yielded non-string value 204 (Rack::Lint::LintError)

Reason being that, it would seem, the Rack spec does not precisely follow HTTP spec. There is some dissent on the matter, as seen in a related issue (no content-type for empty body) here:
rack/rack#472

But the spec states:

The Body must respond to each and must only yield String values. The Body itself should not be an instance of String, as this will break in Ruby 1.9.

https://github.com/rack/rack/blob/master/SPEC#L244-L248

This just came up, I'll be solving it tomorrow, I suspect @body = [] or @body = {} should fix it, as appropriate.

I hope this helps someone else.

edit: here's a related StackOverflow discussion:
https://stackoverflow.com/questions/7701113/rack-error-racklintlinterror-response-body-must-respond-to-each

@dblock
Copy link
Member

dblock commented Mar 27, 2018

My question in rack#472 remained unanswered btw.

@stephancom
Copy link

@dblock oh, haha, I hadn't even noticed that was you. Now I'm not sure if I'm right about this, turns out I had a bigger issue going on - has this been your experience, do I need to return [] or is nil actually OK?

@dblock
Copy link
Member

dblock commented Mar 27, 2018

I guess the answer is "I am not sure"...

@dm1try
Copy link
Member

dm1try commented Mar 27, 2018

The issues mentioned above looks unrelated

first,

@body = nil
do I need to return [] or is nil

From the perspective of Grape user, you should use '' because Grape wraps body value in an array that is needed to Rack. Moreover, from the perspective of Rack user, nil value should be converted to '' implicitly by Grape as Rack has strict requirements mentioned above.

@dm1try
Copy link
Member

dm1try commented Mar 27, 2018

as for 405 Not Allowed

@dblock says Rack::Lint is not doing the right thing raising an error when there's no Content-Type with a 405

I agree, the spec says

The "Content-Type" header field indicates the media type of the
associated representation: either the representation enclosed in the
message payload or the selected representation, as determined by the
message semantics.

and 405 Not Allowed

The 405 (Method Not Allowed) status code indicates that the method
received in the request-line is known by the origin server but not
supported by the target resource. The origin server MUST generate an
Allow header field in a 405 response containing a list of the target
resource's currently supported methods.

so it looks like the semantics of 405 response does not assume any resource representation

@stephancom
Copy link

stephancom commented Mar 27, 2018

In (some of) my case(s), returning an empty array or hash is the more appropriate than a string - it's a case where I was returning a nil body and a 204 where I might otherwise be returning a collection. An empty collection would make sense there.

Other cases, yeah, I guess an empty string is an ok substitute for nil. It still bothers me a little, I don't like that Rack insists on this, it's weird.

As for grape converting a nil to '', are there cases where it's not feeding to rack, to something else instead? If so, that should at least be a configurable option, because nil seems semantically more appropriate.

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

Successfully merging a pull request may close this issue.

4 participants