404 routing is done on a per http request not on a per hmvc request

[peter-fritchley]

It is common to use a 404 route in a module as a catch all. But once the request has been through the 404 route, subsequent hmvc requests that should also be routed through the 404 route fail.

I think that the handling of the HttpNotFoundException should be moved into the request forge or execute method to allow for this, but I'm not sure how that would work without causing an infinate loop in the event that the 404 route doesn't exist.

One suggestion was to mark the request as a 404 request and stop trying to re-route it.

[jschreuder]

No it really shouldn't. The 404 handling isn't part of the framework as you may have noticed. The 404 exception is caught in the front controller and a new request to the 404 route is created to generate the output. Handling of the 404 happens completely outside the Core and the Core has no knowledge of this whatsoever. Exceptions are used here just for the very reason to allow you to handle a 404 in any way you may want to, just put your HMVC request in a try-catch, catch the NotFound exception and handle it however you please.

The way you're proposing would actually make the framework less flexible and make it impossible to handle NotFound from an HMVC request in a specific way. Which will be much better than returning full HTML 404 output for each HMVC request that is meant to only retrieve a small portion of a page or even just raw data.

[andrewwheal]

@jschreuder so what your saying is that in every place that we do a HMVC request (which is a lot of places) we need to replicate the code that is in index.php to catch the first instance of a 404 exception and route it as required? We have some apps here that make a lot of HMVC requests which all need to go through out Admin module which uses the 404 route as a catchall.

Could you not put the 404 handling in the Request::forge() (or where ever is appropriate) and if you want to handle your HMVC 404's differently then have the 404 action detect that it was generated from a HMVC request and re-throw the exception?

Just trying to work out the best way that it can all be handled, any suggestions would be welcome.

[jschreuder]

Either do the request to a place you're sure exists or be prepared to handle the problem. Failing silently or making assumptions about how everyone wants it handled is wrong for any framework. Problems shouldn't arise and if they do they should be handled.

Things like putting the 404 handling in the forge is really a wrong place for generic usage. If that's allright for your app: extend the Request class and make it catch the exception.

[jschreuder]

Or better yet: create an HMVC request method somewhere (for example in a base controller) that does the HMVC request for you and handles the HMVC's NotFound exceptions in the way you want them handled. That is exactly why it works like this: to allow you to decide what to do with it. I assume everyone knows how to code DRY.

[andrewwheal]

I can obviously see your point of needing to keep everything generic, we are just trying to get our Admin HMVC requests to work at as low a level as possible.

We are thinking about changing our catchall route from using _404_ to use (:any) but this has the drawback that we need to explicitly specify all routes within our admin module so that it doesn't try to re-route them. It is almost as though there needs to be a _catchall_ route which gets called just before _404_, if it exists.

[jschreuder]

That last problem doesn't exist in 2.0 anymore: a route only matches when the controller it routes to exists, otherwise the route won't match.

But as I said: you can solve your current problem easily by creating your own method for HMVC requests that handles the exceptions and reroutes to the _404_ route when an exception was thrown. Or you create a _catchall_ route that gets called first and only then fallback to the _404_. It's up to you...

[andrewwheal]

Thanks for the help @jschreuder, would there be any point in putting a pull request in if we were to implement a _catchall_ route?

[jschreuder]

No, I always shot that down because we prefer to keep the number of arbitrary special routes to an absolute minimum. And I completely eliminated them in 2.0 (it has application methods for handling 404s/exceptions instead of routes, and the default route _root_ is just '/').

Ask yourself: what does _catchall_ mean that is really different from _404_? There are 2 possible differences:

• a 404 can also be thrown when the route matched but the controller or it's action didn't - while a catchall would only be used when no route matched
• a 404 might be thrown from within a controller, while the catchall could never be triggered in that way

The first one I don't really consider a difference, it's more something I don't like about the routing in 1.x (it shouldn't route to a non-existent controller). The second is a bigger difference but is easily solvable in other ways.
As I said this has been solved in 2.0 by making routes only match on existing controllers. This makes it possible to do the normal controller searching manually in 2.0 and have a catchall :any route after that. But to implement stuff like this in the 1.x branch would just be creating a mess. It wasn't meant for that and the changes would be so big that the internal structure would change which we prefer not to do anymore for 1.x releases.