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

respond_with doesn't accept statuses on PUT requests #9069

Closed
Dru89 opened this issue Jan 25, 2013 · 18 comments
Closed

respond_with doesn't accept statuses on PUT requests #9069

Dru89 opened this issue Jan 25, 2013 · 18 comments

Comments

@Dru89
Copy link

Dru89 commented Jan 25, 2013

Given the following controller,

class AccountsController < ApplicationController
    respond_to :json, :xml
    def update
        respond_with error_hash, status: :not_found, root: :error, 
                    location: accounts_url
    end

    def error_hash
        { :example => "Example for this question", :parameter => 42 }
    end
end

and a routing rule of resources :accounts

I would expect to see a status of 404 from a curl request, like the following:

curl -i --header "Accept: application/json" \
         --header "Content-type: application/json" \
         -X PUT -d '{"name": "test_name"}' \
         http://localhost:3000/accounts/3d2cc5d0653911e2aaadc82a14fffee9

However, it instead returns the following:

HTTP/1.1 204 No Content 
Location: http://localhost:3000/api/accounts
X-Ua-Compatible: IE=Edge
Cache-Control: no-cache
X-Request-Id: 7e6f61f9b6e533e8b875c0a96d8a99bd
X-Runtime: 0.116876
Server: WEBrick/1.3.1 (Ruby/1.9.3/2013-01-15)
Date: Fri, 25 Jan 2013 06:23:25 GMT
Connection: close

The following use of respond_with also does not work:

def update
    respond_with(error_hash) do |format|
        format.json { render status: :not_found }
    end
end

Using respond_to does work, though.

def update
    respond_to do |format|
        format.json { render json: error_hash, status: not_found }
    end
end

I believe that respond_with is broken in this way.

The version of rails that I'm using is 3.2.11

@pseidemann
Copy link
Contributor

Is that valid to the http protocol to respond with 404 while passing a Location header?

@Dru89
Copy link
Author

Dru89 commented Jan 26, 2013

Probably not. But if you remove the Location header, it still has the same problem.

@et
Copy link

et commented Jan 26, 2013

For what it's worth, the behavior is similar on master.

@et
Copy link

et commented Jan 26, 2013

@Dru89 - this appears to be intentional. It's expected to that PUT returns an empty response (hence 204) for successful requests.
I think the reason respond_to acts differently than respond_with is because the latter tends to follow the spec as strictly as possible. I checked the source and those settings really only apply when they're appropriate (e.g. defining a redirect location for POST).

@Dru89
Copy link
Author

Dru89 commented Jan 26, 2013

Should this be intentional, though? I'm not entirely sure. I don't think it should be intentional to return a success code when you're really wanting to show the user had an error. Especially since it's a PUT request and there's no content for the response, so you can't return a JSON or an XML error response.

respond_with(answer) seems like a great solution to use for an API, but this one gotcha seems a bit too big to work around, if it's intentional. I only started using rails again in the last week or two (after a year or so hiatus), so I might be missing something fundamental here.

Also, @et, can you link to the source where you found that? I'm digging for myself, but I can't seem to find where it's actually applying the settings. Thanks!

@steveklabnik
Copy link
Member

First of all, returning a 404 from a PUT makes no sense, because PUT creates resources if they do not exist.

That said, this does seem to be a weird edge case.

@Dru89
Copy link
Author

Dru89 commented Jan 26, 2013

@steveklabnik PUT updates a resource that already exists. I think you mean POST. That said, I think 404 could make sense for POST too, if the endpoint that you're trying to call to create a resource for a type that doesn't exist. (Though I guess that wouldn't be handled from an action....)

Also, 404 makes a lot of sense for PUT, too, for when you're trying to update a resource that does exist. For example:

def update
  account = Account.where(id: params[:id]).first
  unless account.nil?
    # update account
  else
    # respond with 404 message and custom error, because that account doesn't exist.
  end
end

You could use Account.find and that would deliver the 404, but you couldn't customize the output at all. You'd be stuck with the default HTML 404 page from rails, as far as I know. Plus, you couldn't use Account.find with some parameter that wasn't the id. For example, I have a sequential ID and a unique column for uuid. I use uuid to deliver to customers, instead of the sequential one. Account.find would not work for me even if I could customize the output.

@steveklabnik
Copy link
Member

@Dru89 no, PUT creates or updates. Rails generally treats it as an 'update,' but the HTTP spec says otherwise.

@trvrplk
Copy link

trvrplk commented Jan 27, 2013

@Dru89 @steveklabnik Look here: http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html

Directly from there:

The fundamental difference between the POST and PUT requests is reflected in the different meaning of the Request-URI. The URI in a POST request identifies the resource that will handle the enclosed entity. That resource might be a data-accepting process, a gateway to some other protocol, or a separate entity that accepts annotations. In contrast, the URI in a PUT request identifies the entity enclosed with the request -- the user agent knows what URI is intended and the server MUST NOT attempt to apply the request to some other resource. If the server desires that the request be applied to a different URI,
it MUST send a 301 (Moved Permanently) response; the user agent MAY then make its own decision regarding whether or not to redirect the request.

Also, on the topic of 404 and other status codes, you can't use 200 OK.

Anyway, getting back on the ground, why would you immediately 404?

@steveklabnik
Copy link
Member

Right. Ultimately, there's something funky with Rails, regardless of the specific status code and its meaning.

@Dru89
Copy link
Author

Dru89 commented Jan 28, 2013

As for why you would immediately 404, you wouldn't. I just omitted details for the sake of point. I do three things. For example, for update,

  1. If the account doesn't exist, then do a 404
  2. If the account does exist, but the update fails, then do a 422
  3. If the account exists, and it's successful, do a 204

@prathamesh-sonpatki
Copy link
Member

Any update on this?

@faun
Copy link

faun commented Apr 26, 2013

Is the convention here that after updating a resource, you have to fetch the resource again to see if it was successfully updated? It seems that returning a 422 in the case that the record not being updated would be the most efficient use of bandwidth and least confusing.

From the aforementioned spec, "If the resource could not be created or modified with the Request-URI, an appropriate error response SHOULD be given that reflects the nature of the problem."

Should an appropriate error response include being able to return the status code appropriate for the response message?

For example

HTTP/1.1 422 Unprocessable Entity
{"errors":{"foo_relation":["can't be blank"],"bar_relation":["can't be blank"]}}

seems like an appropriate response when updating a record without the requisite foreign keys.

@astronaute77
Copy link

Any info on this?
422 with an error message should be allowed on PUT IMHO.

@ffloyd
Copy link

ffloyd commented Nov 15, 2013

+1

I need 422 in my current projects. It's strange decision to always return 204 =/

@iamsolankiamit
Copy link

Any updates on this?

@Dru89 Dru89 added the stale label May 27, 2014
@rails-bot
Copy link

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@rails-bot
Copy link

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

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

No branches or pull requests