404 does not pass through formatter

[sinantaifour]

Since Rack::Mount::RouteSet is used for routing before any middleware is run, if a route does not exist RouteSet generates the 404, and it never passes through a formatter.

This behavior might be undesirable if the client expects all responses to follow the correct format.

As a temporary hack, I running a middleware before the subclass of Grape::API, that invokes methods from Grape::Middleware::Error if a 404 is returned.

[dblock]

I think that handling 404s from the router should be something that Grape provides. Maybe something like rescue_from :404? Please suggest.

[sinantaifour]

I would suggest instead of creating a new approach for handling 404s, to instead just raise a Grape::Exceptions::NotFound, and use the facilities already there to deal with exceptions.

Basically, I'd suggest moving Grape::Middleware::Error to stand before @route_set, and cascade @route_set with code to raise Grape::Exceptions::NotFound if the response from @route_set is a 404 and cascade? == false.

Then, the 404s could be handled with a rescue_from :all or a rescue_from Grape::Exceptions::NotFound.

[dblock]

@StaII That sounds reasonable. Try making a PR.

[sinantaifour]

I have a structural question first: currently build_middleware is the one point where all middleware is put into the Rack app. Since this happens inside the endpoint, it is not a suitable place to use the Error middleware, if the suggestion above is to be implemented.

Where do you suggest inside API should the Rack::Builder be created?

[dblock]

I really don't know the best place, but either way the stacking of the midleware in Grape is probably something that should be refactored. Maybe there's a cleaner way to organize this whether it's the midleware or the API part.

[mscrivo]

I would suggest instead of creating a new approach for handling 404s, to instead just raise a Grape::Exceptions::NotFound, and use the facilities already there to deal with exceptions.

Basically, I'd suggest moving Grape::Middleware::Error to stand before @route_set, and cascade @route_set with code to raise Grape::Exceptions::NotFound if the response from @route_set is a 404 and cascade? == false.

Then, the 404s could be handled with a rescue_from :all or a rescue_from Grape::Exceptions::NotFound.

So much this!

Running into this right now. If I add the 404 handler as specified in the README, then everything 404's, including endpoints that exist but the method is not allowed, and instead of telling the user that, it just says 404.