404 error handling

[jonleighton]

When PushType can't find a node, it raises ActiveRecord::RecordNotFound. I realised that I can add a rescue_from handler to my ApplicationController in order to handle this gracefully, but this isn't documented.

It would be helpful to add some doc for this, but I actually think it would be better to ensure that the *permalink route doesn't match unless the permalink exists in the database. Then, users can put other routes after the mount_push_type call and define their own handling for 404s (or just let Rails do its default thing, rendering public/404.html).

[aaronrussell]

I'm currently travelling so excuse the brevity. I may have missed the point here but does Rails not render a 404 by default with a ActiveRecord::RecordNotFound? Why would a rescue_from be needed?

Currently mount_push_type must be at the end of your routes file. I don't disagree - it would be better to only match the route if the permalink exists - but I'm unsure how best to do this. I would assume we'd create a middleware that queries the database, but I'd want to avoid the controller making the same query again.

[jonleighton]

I may have missed the point here but does Rails not render a 404 by default with a ActiveRecord::RecordNotFound?

Yes, it looks like it does. The reason I started looking into this, though, is because we started to get these exceptions reported to our exception notification system. It looks like Rails renders the 404 page but the exception still bubbles up (I see FATAL entries in the log file too). So I guess what I mean by "gracefully" is "without bubbling up".

I'm unsure how best to do this. I would assume we'd create a middleware that queries the database, but I'd want to avoid the controller making the same query again.

It could be done with a routing constraint. If the AR query cache is active at this point, then a double database query could be avoided. I think it would be active, but I haven't actually checked.

[aaronrussell]

I'll do a few experiments once I'm back. If the query gets cached in a routing constrain then no reason not to do this :)