:any and "*path" Not Working

[bobber205]

The following isn't working when my route has a trailing parameter like this

put '/test/ilovetests/updatemylove/:id'

if I do a put to '/test/ilovetests/updatemylove/' the below code isn't hit.

module OurAPI

  class NotFound < Grape::API 

    version 'v3.0', using: :path
    format :json

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

  end
end

I've tried loading this class in the array sent to Rack::Cascade.new at both the beginning and at the end (like you should and the example in the READme insists on) and it's not hit whenever I throw a bogus route its way.

This started once we upgraded grape from 0.13 to 0.16 (latest) and is only happening on routes that have trailing parameters.

I noticed that rack-mount was dropped between 0.13 and 0.16 in favor of mustermann. Could that be the source of our issue?

[dblock]

The router was indeed replaced. Can you try to come up with a repro for this, a spec in the Grape project would be great.

[bobber205]

Sure thing. I'll whip something up today.

[namusyaka]

@bobber205 Thanks for the report. Confirmed the problem. I'm going to fix it in a few days.

[namusyaka]

Mmm, the route cannot be recognized if correct version does not exist.
Should path be '/v3.0/test/ilovetests/updatemylove/'?
@dblock Thoughts?

[dblock]

So the question is whether route :any should apply to version 3.0 only or not, right?

I suppose this will work without code changes?

module OurAPI

  class NotFound < Grape::API 

    version 'v3.0', using: :path
    format :json

    ...
  end

  class Parent < Grape::API
    mount NotFound

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

If that's the case I think this works as designed.

[connorshea]

After fixing the problem I mention in #1402 by replacing a before call with after_validation, I'm also seeing exactly this problem. Just my luck :)

Anything I can do to help this along?

[dblock]

Well I think it's by design @connorshea - route :any should apply to version 3 in the example above if declared next to it. My workaround of putting it outside of a versioned API should fix your problem, no?

I'd like to close this actually, lets see what you say :)

[connorshea]

@dblock We have the following in our lib/api/api.rb file:

module API
  class API < Grape::API
    include APIGuard
    version 'v3', using: :path

    rescue_from Gitlab::Access::AccessDeniedError do
      rack_response({ 'message' => '403 Forbidden' }.to_json, 403)
    end

    rescue_from ActiveRecord::RecordNotFound do
      rack_response({ 'message' => '404 Not found' }.to_json, 404)
    end

    # Retain 405 error rather than a 500 error for Grape 0.15.0+.
    # See: https://github.com/ruby-grape/grape/commit/252bfd27c320466ec3c0751812cf44245e97e5de
    rescue_from Grape::Exceptions::Base do |e|
      error! e.message, e.status, e.headers
    end

    format :json
    content_type :txt, "text/plain"

    mount ::API::Issues
    mount ::API::MergeRequests
    # etc.

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

I guess I'm not clear on how we would place the route :any outside of this?

[connorshea]

Re-reading the thread, the original issues describes (and the one I'm running into) the problem as being that the API has the following code in it:

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

But when you have a route that is defined as PUT '/test/ilovetests/updatemylove/:id', then have a spec where you PUT '/test/ilovetests/updatemylove/' (e.g. without an :id parameter) the spec recieves a different return value in 0.13.0 (and 0.15.0 in my experience) than in 0.16.2+.

@dblock I may be misunderstanding things myself, but I think the original issue has the issue of not including the version in their example, which confuses things. In my situation, the path would be PUT 'api/v3/test/ilovetests/updatemylove/:id'.

[dblock]

Are you saying you're not getting that 404 there? can you write a spec to demonstrate? Ideally take the whole version thing out of it.

[connorshea]

Just tested this again on GitLab and it looks like it's fixed on master, presumably by #1517?

[dblock]

That's correct @connorshea. Closing this.