Routing priority issue in Grape v2.1.0 when using with rails

[eiskrenkov]

Hello, everyone! When upgrading to grape v2.1.0 we've encountered an issue with routing. I created a sample rails new repo for demo purposes - https://github.com/eiskrenkov/grape-routing-issue-demo

Issue

I'm mounting grape API in rails routes at the top of the file

Rails.application.routes.draw do
  mount Twitter::Api => '/'

  get "up" => "rails/health#show", as: :rails_health_check
end

When i'm doing GET http://localhost:3000/up request in grape v2.0.0 it returns response correctly, but in grape v2.1.1 i'm getting 404 error. It seems like something was changed in route prioritising, if nothing matches grape's routes it doesn't continue the journey

Steps to reproduce

1. (export SHOULD_WORK=true; bundle; bundle exec rails s)

will use grape v2.0.0 and will respond to http://localhost:3000/up correctly

2. (export SHOULD_WORK=false; bundle; bundle exec rails s)

will use grape v2.1.0 and respond with 404

[dblock]

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.

[ericproulx]

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

[Michael-Tix]

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

[dblock]

Can someone bisect this to a change please?

[eiskrenkov]

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

[ericproulx]

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

[ericproulx]

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

[eiskrenkov]

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

[eiskrenkov]

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

[dblock]

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

[eiskrenkov]

@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!