ruby-grape / grape

An opinionated framework for creating REST-like APIs in Ruby.
http://www.ruby-grape.org
MIT License
9.9k stars 1.22k forks source link

format :json is not scoped to one API, it affects other APIs #1534

Open hantuzun opened 7 years ago

hantuzun commented 7 years ago

We have two endpoints, one accepts application/json content-type, the other accepts text/plain. They live together. However, when format :json is included in one of our API's the other one is effected as well.

Consider the example below:

class API < Grape::API
  format :json
  get '/v1/users' do
    {'api': 'api'}
  end
end

class BAPI < Grape::API
  post '/v1/users' do
    {'bapi': 'bapi'}
  end
end

application = Rack::Builder.new do
  map '/' do
    run Rack::Cascade.new [API, BAPI]
  end
end

In this case, POST /v1/users returns {"error":"The requested content-type 'text/plain' is not supported."}. When format :json is removed, it returns {"bapi": "bapi"} as expected.

dblock commented 7 years ago

Something is fishy. Try to build a spec in Grape for this?

dm1try commented 7 years ago

@dblock the behavior above is expected because Rack::Cascadecatches only the responses with 404/405 status code but the grape middleware formatter returns the response with 406 status code(when explicit json format is provided but the request has a different type). so Rack::Cascade.new([API, BAPI], 404..406) might help.

But I have a different problem here :tired_face:

When format :json is removed, it returns {"bapi": "bapi"} as expected.

I cannot repeat this on master branch.

@hantuzun which grape version is used?

hantuzun commented 7 years ago

Thanks guys!

@dmitry; I experienced that with grape 0.10.1.

I achieved what I wanted by swapping the endpoints in cascade constructor array like: run Rack::Cascade.new [BAPI, API]. It worked "as I expected" in that case – serving two methods with different content types.

What's the best practice for serving two sets of methods that accepts different content-types?

dblock commented 7 years ago

0.10.0 is really old, we're on 0.18.0, just want to make sure we're not fixing something that's fixed long time ago

hantuzun commented 7 years ago

Oh, I don't know why bundler installed that. Thanks for the notice!

I'll try to reproduce the error on the latest release.