Defining a catch-all after multiple versions of an endpoint hide endpoints after the first definition

[zherr]

Given the following test:

shared_examples_for 'versioning' do
  context 'with different versions for the same endpoint' do
    context 'catch-all' do
      before do
        subject.version 'v1', macro_options
        subject.get 'version' do
          'v1'
        end

        subject.format :txt
        subject.version 'v2', macro_options
        subject.get 'version' do
          'v2'
        end

        subject.route :any, '*path' do
          error!('Not Found', 404)
        end
      end

      it 'finds v1 of the endpoint' do
        versioned_get '/version', 'v1', macro_options
        expect(last_response.status).to eq(200)
        expect(last_response.body).to eq('v1')
      end

      it 'finds v2 of the endpoint' do 
        versioned_get '/version', 'v2', macro_options
        expect(last_response.status).to eq(200) # >>>>> This fails! Results in 404 <<<<<
        expect(last_response.body).to eq('v2')
      end
    end
  end
end

For even more context, see: master...zherr:multiple-endpoint-catchall

Before digging deeper into a fix I would like to gather feedback from contributors on whether this appears to be a bug, or if this behavior intentional.

Happy holidays!

[dblock]

It certainly smells like doing version v1 then version v2 to the same api class overrides the previous definition, so split them up and mount them and that should work better.

This might be helpful.

[zherr]

Our current implementation splits up the versions by module like so, and still experiences the same issue:

module MyApp
  class Base < Grape::API
    mount V1::Base
    mount V2::Base
    route :any, '*path' do
      error!('Not Found', 404)
    end
  end
end

Given this implementation, when a request specifies Accept-Version=v2 in the header, for example, the V2 endpoint is not found. When the version is left unspecified, the V1 endpoint is found just fine.

Unfortunately, if we switch the mount order here as the blog post suggests, we still run into the same issue if the catch-all is defined; then V1 endpoint is not found.

Removing the catch-all solves part of the issue, and allow requests to find the V1 endpoint when not specifying the version, and allows requests that do specify 'v2' to find the new version. But this leaves us unable to capture traffic that would typically 404 and let us render a reasonable message.

It still seems that the presence of the catch-all should not affect if an endpoint is available, mount order or otherwise. What do you think?

[dblock]

I can confirm that this is a bug. I PRed the spec with mount in #1722 (but it should work without mount too). I wasn't able to fix it after digging a bit, maybe someone will have better luck.