Rabl not running for DELETE

[guycall]

I am in the process of upgrading an old Rails app. Just tried increasing Grape from version 0.17.0 to 0.19.2. I hit on issue with the rabl no longer being processed for DELETE.

My API looks like:

desc 'Disable to send push notification'
delete '/push_notifiable', rabl: 'rooms/push_notifiable' do
  @room = Room.find(params[:room_id])
  @push_notifiable = false
end

And the test that now fails looks a bit like:

describe 'DELETE /rooms/:room_id/push_notifiable' do
  subject { delete url, params: params }
  it 'returns 200 status, and correct json' do
    subject
    expect(response.status).to eq 200
    expect(json['room']['room_type']).to eq 'XXXX'
  end
end

The status test passes, but the response body is no longer the JSON generated by my rabl, just simply the string false.

[dblock]

I think you're seeing the response from @push_notifiable = false, so whatever is bringing rabl: ... is not kicking in. Are you using grape-rabl? I would start there with integration tests against a newer version of Grape.

[guycall]

Yes, we are using grape-rabl. I tried adding DELETE methods into the grape-rabl test suite and had no issues.

I just tried with grape 1.0.3 and stepped through the code. This looks like a potential cause:

[20, 29] in /home/vagrant/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/grape-1.0.3/lib/grape/middleware/formatter.rb
   20: 
   21:       def after
   22:         return unless @app_response
   23:         status, headers, bodies = *@app_response
   24: 
=> 25:         if Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.include?(status)
   26:           @app_response
   27:         else
   28:           build_formatted_response(status, headers, bodies)
   29:         end
(byebug) status
204
(byebug) build_formatted_response(status, headers, bodies)
#<Rack::Response:0x00005562266bf7a0 @status=204, @header={"Content-Type"=>"application/json", "Content-Length"=>"89"}, @writer=#<Proc:0x00005562266bf638@/home/vagrant/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/rack-2.0.7/lib/rack/response.rb:32 (lambda)>, @block=nil, @length=89, @body=["{\"chat_room\":{\"chat_room_id\":1,\"chat_room_type\":\"GroupChatRoom\",\"push_notifiable\":false}}"]>
(byebug) Rack::Utils::STATUS_WITH_NO_ENTITY_BODY
#<Set: {100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 204, 304}>

The build_formatted_response call renders the rabl file correctly, but because status is already set to 204 it never gets called.

[guycall]

I'll test with newer versions of grape and see if issue still occurs.

[dblock]

Interesting. This probably means we're inserting the middleware in the wrong place.

[dm1try]

I think the problem is that "false" in the return means "no body" https://github.com/ruby-grape/grape/pull/1550/files#diff-e9e8424a5238d48301e313d8fe285697R133