Routing priority issue in Grape v2.1.0 when using with rails about grape HOT 13 CLOSED

@dblock I just did, the test provided by @ericproulx starts failing at 3ae7b7a720911a11a933b1788fc82764fbeaf52c, after this pr got merged

from grape.

A test like this one would have catch it

context 'cascade within another app' do
  subject { last_response.headers }

  let(:api) do
    Class.new(Grape::API) do
      get('/test'){ return_no_content }
    end
  end

  let(:app) do
    test_api = api
    Rack::Builder.app do
      use Rack::Lint
      run test_api.new
    end
  end

  before { get '/' }

  it { is_expected.to include('x-cascade') }
end

Rack::Lint would have raised

Rack::Lint::LintError:
  uppercase character in header name: X-Cascade

from grape.

I do confirm, this test in our stack is passing in 2.0.0 but not in 2.1.0. There's is something with the / it works when mounting to /api.

context 'rails mounted' do
  let(:app) do
    require 'rails'
    require 'action_controller/railtie'

    api = Class.new(Grape::API) do
      get('/test_grape'){ 'rails mounted' }
    end

    Class.new(Rails::Application) do
      config.eager_load = true
      config.load_defaults 7.1
      config.api_only = true
      config.consider_all_requests_local = true
      config.hosts << 'example.org'

      routes.append do
        mount api => "/"

        get 'up', to: ->(_env) do
          ['200', {}, ['hello world']]
        end

      end
    end
  end

  before { app.initialize! }

  it 'responds' do
    get '/test_grape'
    expect(last_response).to be_successful
    expect(last_response.body).to eq('rails mounted')
    get '/up'
    expect(last_response).to be_successful
    expect(last_response.body).to eq('hello world')
  end
end

from grape.

@ericproulx yes, it's 3.1.3 in both my demo repo and grape's Gemfile.lock on my machine where i did bisect

from grape.

Looks like a regression. Try porting this into a spec in this repo without rails? Then it's easy to bisect down to where it changed.

from grape.

Same problem confirmed. I'm moving the mount definition for my Grape routes to the bottom of the routes.rb file as a temporary fix

from grape.

Can someone bisect this to a change please?

from grape.

I found something related to the X-Cascade. I'm still digging

from grape.

@eiskrenkov are you using rack >= 3 ? While testing, I found that its only happening with Rack >= 3 but maybe I'm wrong

from grape.

Thank you so much for your work guys @dblock @ericproulx <3 Will try the new version once it's out

from grape.

@eiskrenkov please do try HEAD, github: 'ruby-grape/grape' in Gemfile, appreciate if you could confirm the fix works.

from grape.

@dblock sure thing! Just tried it in my project, it helped! Routes that are defined under grape mounting are reachable now. Thank you so much for such a quick fix again!

from grape.

@dblock Until now, I couldn't wrap my head around this bug because obviously we had a test for cascading. While looking at the caller when calling the default response, I figured that rack-test processes the request with a Rack::MockResponse which will lowercase the headers through Rack::Response by design. So, even though we were returning X-Cascade, the last_response.headers would have x-cascade.

Now, when running within a rails app, the default-response is managed by Rails's routing which will look for x-cascade with Rack 3 instead of X-Cascade.

from grape.