We have moved to GitHub Issues
Created by Meixger 18 May 2015, 09:37:48 Updated by Stephan 15 Jun 2015, 15:37:15
When the associated template is not found, a 404/503 should be thrown.
Actually an empty ActionResult will be returned:
Also the http status code is 200
Pretty sure this has been discussed a few times previously... maybe here? https://groups.google.com/forum/?fromgroups#!searchin/umbraco-dev/template$20404/umbraco-dev/AfTxEQGwWH0/QtymyqIWBukJ
Pretty sure @zpqrtbnk will have some opinions on when/where this discussion ended up
The default 404 behavior looks pretty fine to me. Still, an empty ContentResult doesn't never make any sense to me.
In this specific case (missing template file) i would expect an error or 404 page.
Have assigned to stephen to get his feedback, if we make this change we'll have to indicate that it is a breaking change.
So... this is when we're rendering a content and we have been able to identify a template to use (from the content & template in database) yet at the time of rendering, the actual template file is nowhere to be found, correct?
I guess at the moment we return an empty content "by default", there is no strong reason to do so. What else could we do?
And then... maybe that's a bit extreme. If no template is found by PublishedContentRequestEngine, the request is passed on to MVC and RenderRouteHandler.GetHandlerOnMissingTemplate transfers to PublishedContentNotFoundHandler - so we display a warning message but not a 500 error.
So in my mind we need to pick one of:
Let me know what you'd like.
Upon reflection i see this specific case really as an error 503.
4xx code: I agree, if the editor's intention was to create a working page and the template exists in the DB but not on disk, then a 4xx would be inappropriate. Even with a 'The template is missing' message.
5xx code: If the template exists in the DB but not on disk [anymore], some admin/developer should step in an fix it. But without an exception the error would simple be missed.
Regarding the error code, 503 is really a temporary error. You tell search engine crawlers that the downtime is temporary.
I would show crawlers a 503 and catch it in my global error handler where i log it and show the user a nice [temporary] error page.
If you have a global error handler that logs errors and show the user a nice error page, then crawlers will not see the 503, right? Then we might as well throw an exception in the controller, which will be handled by the error handler.
We'd just invoke EnsurePhysicalViewExists(template) which would return void, and throw instead of logging.
Oh no, my global error handler doesn't alter the http status code! So the crawler will see a 503 and the user will see a nice error page.
Yes, throw instead of logging would be fine for me.
Changing this to a 'breaking change' because it's changed behavior
Have we reached an agreement on that one? Proposing to replace the line
throw new Exception("No physical file template file was found for template " + template);
And then the site should have a proper catch-all page for 500 errors. Thoughts?
I think that is correct, but we will mark this issue as 'breaking' since some people might be relying on that behavior and will need to change it.
(ah, it's already marked as breaking)
Backwards Compatible: False
Due in version: 7.3.0