This project is archived and is in readonly mode.

#5199 ✓ resolved
Nikolay Belous

respond_with returns " " on PUT and DELETE verb

Reported by Nikolay Belous | July 26th, 2010 @ 04:40 PM | in 3.x

RVM + Ruby 1.9.2 + Rails 3.0.0.beta4

class UsersController < ApplicationController
  respond_to :json, :xml

  def create #POST users.json
    @user = User.first
    respond_with(@user) #200 OK, returns data
  end

  def show #GET users/1.json
    @user = User.first
    respond_with(@user) #200 OK, returns data
  end

  def update #PUT users/1.json
    @user = User.first
    respond_with(@user) #200 OK, returns " "
  end

  def destroy #DELETE users/1.json
    @user = User.first
    respond_with(@user) #200 OK, returns " " 
  end
end

I think this is very strange.
For example I want to return error description in the requested format.

Comments and changes to this ticket

  • José Valim

    José Valim July 26th, 2010 @ 04:52 PM

    • State changed from “new” to “invalid”
    • Importance changed from “” to “Low”

    Respond with does not save your resource. You need to call .save or update_attributes before calling respond_with accordingly.

  • Nikolay Belous

    Nikolay Belous July 26th, 2010 @ 05:04 PM

    This is just an example.
    Real code below

      def create
        @user = User.new(params[:user])
        if can? :create, @user
          @user.save
          respond_with(@user)
        else
          raise SWC::Exceptions.new(I18n.t('user.errors.insufficientAccountPermissions'))
        end
      end
    
      def show
        @user = User.find(params[:id])
        if can? :read, @user
          respond_with(@user)
        else
          raise SWC::Exceptions.new(I18n.t('user.errors.insufficientAccountPermissions'))
        end
      end
    
      def update
        @user = User.find(:id => params[:id])
        if can? :update, @user
          @user.update_attributes(params[:user])
          respond_with(@user)
        else
          raise SWC::Exceptions.new(I18n.t('user.errors.insufficientAccountPermissions'))
        end
      end
    
      def destroy
        @user = User.find(params[:id])
        if can? :destroy, @user
          @user.destroy
          respond_with(I18n.t('user.successful.accountDestroyed'))
        else
          raise SWC::Exceptions.new(I18n.t('user.errors.insufficientAccountPermissions'))
        end
      end
    

    If I try to remove non-existent user I get " " rather than the error in JSON format

  • matin
  • Jamie Hill

    Jamie Hill August 11th, 2010 @ 10:03 PM

    Sorry to resurrect this ticket but what is the solution here? As far as I can see both update and destroy should return something for say a json response as otherwise the client will error, not being able to pass an empty string as json.

  • Jamie Hill

    Jamie Hill August 11th, 2010 @ 10:06 PM

    Sorry, should have said that to get around this I am currently using this for the json response:

    render :text => '{}', :status => :ok
    

    This is the only way to get Rails to play nice with libraries such as http://github.com/benpickles/js-model

  • Brandon Keepers

    Brandon Keepers August 20th, 2010 @ 06:32 PM

    I ran into this issue today too. As is, respond_with is not helpful for any API that expects JSON to be returned.

    This ticket should be reopened.

  • José Valim

    José Valim August 20th, 2010 @ 06:46 PM

    • Milestone set to 3.x
    • State changed from “invalid” to “incomplete”

    I cannot see what is the issue. Can someone post a better example? Maybe a failing patch to Rails test suite?

  • Rohit Arondekar

    Rohit Arondekar August 25th, 2010 @ 03:25 AM

    Can you try writing a test for this? I think you should try writing it in actionpack/test/controller/mime_responds_test.rb

  • José Valim

    José Valim September 2nd, 2010 @ 07:46 AM

    • State changed from “incomplete” to “needs-more-info”
  • jduff

    jduff September 13th, 2010 @ 01:06 AM

    The issue with this is if you are using a strict JSON parser on the client side it will try to parse " " and fail. This really depends on the client you are using. For example if I make an ajax put request with jQuery to an action that users respond_with it will always fail because it cannot parse the returned " "

    $.ajax({
      url: 'http://localhost:3000/users/1.json',
      dataType: 'json',
      type: 'PUT',
      data: {
        user: {
          name: "something new"
        }
      },
      error: function(XMLHttpRequest, textStatus, errorThrown) {
         alert('error');
      },
      complete: function() {alert('complete');}
    });
    

    The error callback will always get thrown because it fails to parse the response of " ".

    Personally I think Rails is behaving correctly; if it is a successful update there is no reason to return the object again since you already have it, head ok should be good enough.

    The solution I have for this is on the jQuery side, I've added this line to my application.js and everything works perfectly:

    jQuery.ajaxSetup({ dataFilter: function(data, type){ return (!data || jQuery.trim(data)=="") ? "{}" : data; } });
    
  • fj

    fj September 18th, 2010 @ 05:50 PM

    Here's a minimal test case.

    ## Rails 3 doesn't seem to handle PUT methods correctly.
    
    ## routing
    
    FruitApplication::Application.routes.draw do
      resources :fruits do
        member do
          put :peel  # A HTTP PUT method that will map to fruits/:id/peel
        end
      end
    end
    
    ## controller
    
    class FruitsController < ApplicationController
      respond_to :json
      def peel
        respond_with :status => :not_found    # This appears to be ignored and 200 is returned.
      end
    end
    
    ## spec
    
    describe FruitController do
      subject { @controller }
      before(:each) { put :peel, :id => 1234 }
      it { should respond_with :not_found }
    end
    
    # Failure/Error: it { should respond_with :not_found }
    # Expected response to be a 404, but was 200
    # # ./spec/controllers/fruit_controller_spec.rb:3
    # # /home/johnf/.rvm/gems/ruby-1.8.7-p299@project-getaroom/gems/activesupport-3.0.0/lib/active_support/dependencies.rb:239:in `inject'
    
  • José Valim

    José Valim September 18th, 2010 @ 07:14 PM

    Please add a test case to Rails suite?

  • Szymon Nowak

    Szymon Nowak October 3rd, 2010 @ 06:12 PM

    I've ran into this issue recently as well.

    The problem is with the Responder#api_behavior method:

    def api_behavior(error)
      raise error unless resourceful?
    
      if get?
        display resource
      elsif has_errors?
        display resource.errors, :status => :unprocessable_entity
      elsif post?
        display resource, :status => :created, :location => api_location
      else
        head :ok
      end
    end
    

    In this case for PUT and DELETE verbs it does "head :ok" that, because of some strange error in some old version of Safari, actually returns a single space instead of empty body (http://dev.rubyonrails.org/changeset/1818).

    For PUT verb it could simply return an updated resource in specified format - just like for POST, but with different status. However, I'm not sure if it's necessary.

    For DELETE verb I guess it should return a valid, empty document - whatever this means for specified format. For JSON, "" and " " strings are not considered valid by native JSON parser in WebKit. However, "{}" is valid. Maybe we could introduce a default "empty _ #{format} _ resource" methods for JSON (not sure about XML) that would return a valid document as a string. The Responder#api_behavior method would simply check if responder responds to this method and render an empty resource using "render :text", otherwise it would fallback to "head :ok". This would allow users to add new methods to Responder class for whatever formats they use.

  • José Valim

    José Valim October 3rd, 2010 @ 06:33 PM

    • State changed from “needs-more-info” to “open”

    Can someone please check if this is still a bug in Safari? If not, removing the Safari hax is the best way to handle this issue.

  • Szymon Nowak

    Szymon Nowak October 3rd, 2010 @ 07:12 PM

    I'm not sure if this will actually solve this problem for JSON format, because empty string is still considered as invalid JSON - that's why most people use what Jamie mentioned before:

    render :text => '{}', :status => :ok
    
  • José Valim

    José Valim October 3rd, 2010 @ 10:52 PM

    Checking if the status is json and explicitly returning "{}" in such cases is ok. Patch with tests please?

  • Szymon Nowak

    Szymon Nowak October 11th, 2010 @ 06:46 PM

    Here's a patch that changes the behavior to return "{}" instead of " " for JSON requests with PUT and DELETE.

    It also allows to add new methods to return a valid resource in any format. Currently only empty_json_resource method is defined, but if for someone " " is not a valid XML, one can easily fix it by defining AC::Responder#empty_xml_resource method and return <?xml version="1.0" encoding="UTF-8" ?>* or something similar instead.

    The patch is at http://gist.github.com/620923, because I couldn't upload it here.

  • Repository

    Repository October 11th, 2010 @ 11:56 PM

    • State changed from “open” to “resolved”

    (from [0d3333257156544feba729ba28f6874d5a30d561]) Return a valid empty JSON on successful PUT and DELETE requests. [#5199 state:resolved]

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/0d3333257156544feba729ba28f687...

  • Repository

    Repository October 11th, 2010 @ 11:56 PM

    (from [1556e0874dd5c7e160a9bc5091dd60eb4bd5ec97]) Return a valid empty JSON on successful PUT and DELETE requests. [#5199 state:resolved]

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/1556e0874dd5c7e160a9bc5091dd60...

  • jim123456

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Tickets have moved to Github

The new ticket tracker is available at https://github.com/rails/rails/issues

Shared Ticket Bins

Referenced by

Pages