Fixed error trying to render 404 for missing asset

refs #8868

- Loading the admin prior to a build results in: Failed to lookup view "error-404" in views directory
- This fixes that error, by splitting the HTMLErrorRenderer and the ThemeErrorRenderer into two separate things

core/server/views/error.hbs

@@ -5,7 +5,7 @@
     <meta http-equiv="Content-Type" content="text/html" charset="UTF-8" />
     <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" />
 
-    <title>{{code}} — {{message}}</title>
+    <title>{{statusCode}} — {{message}}</title>
 
     <meta name="HandheldFriendly" content="True">
     <meta name="MobileOptimized" content="320">
@@ -30,7 +30,7 @@
                     src="{{asset "public/404-ghost@2x.png"}}"
                     srcset="{{asset "public/404-ghost.png"}} 1x, {{asset "public/404-ghost@2x.png"}} 2x"/>
                     <section class="error-message">
-                      <h1 class="error-code">{{code}}</h1>
+                      <h1 class="error-code">{{statusCode}}</h1>
                       <h2 class="error-description">{{message}}</h2>
                       <a class="error-link" href="{{@blog.url}}">Go to the front page →</a>
                     </section>

core/server/web/middleware/error-handler.js

@@ -70,11 +70,18 @@ _private.JSONErrorRenderer = function JSONErrorRenderer(err, req, res, next) { /
     });
 };
 
-// @TODO: differentiate properly between rendering errors for theme templates, and other situations
-_private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, next) {
+_private.ErrorFallbackMessage = function ErrorFallbackMessage(err) {
+    return '<h1>' + common.common.common.i18n.t('errors.errors.oopsErrorTemplateHasError') + '</h1>' +
+    '<p>' + common.common.i18n.t('errors.errors.encounteredError') + '</p>' +
+    '<pre>' + escapeExpression(err.message || err) + '</pre>' +
+    '<br ><p>' + common.i18n.t('errors.errors.whilstTryingToRender') + '</p>' +
+    err.statusCode + ' ' + '<pre>' + escapeExpression(err.message || err) + '</pre>';
+};
+
+_private.ThemeErrorRenderer = function ThemeErrorRenderer(err, req, res, next) {
     // If the error code is explicitly set to STATIC_FILE_NOT_FOUND,
     // Skip trying to render an HTML error, and move on to the basic error renderer
-    // I looked at doing this with accepts headers, but the internet is a crazy place...
+    // We do this because customised 404 templates could reference the image that's missing
     // A better long term solution might be to do this based on extension
     if (err.code === 'STATIC_FILE_NOT_FOUND') {
         return next(err);
@@ -90,9 +97,6 @@ _private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, next) {
         errorDetails: err.errorDetails || []
     };
 
-    // Context
-    // We don't do context for errors?!
-
     // Template
     templates.setTemplate(req, res);
 
@@ -118,13 +122,28 @@ _private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, next) {
 
         // And then try to explain things to the user...
         // Cheat and output the error using handlebars escapeExpression
-        return res.status(500).send(
-            '<h1>' + common.common.common.i18n.t('errors.errors.oopsErrorTemplateHasError') + '</h1>' +
-            '<p>' + common.common.i18n.t('errors.errors.encounteredError') + '</p>' +
-            '<pre>' + escapeExpression(err.message || err) + '</pre>' +
-            '<br ><p>' + common.i18n.t('errors.errors.whilstTryingToRender') + '</p>' +
-            err.statusCode + ' ' + '<pre>' + escapeExpression(err.message || err) + '</pre>'
-        );
+        return res.status(500).send(_private.ErrorFallbackMessage(err));
+    });
+};
+
+_private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, next) {  // eslint-disable-line no-unused-vars
+    var data = {
+        message: err.message,
+        statusCode: err.statusCode,
+        errorDetails: err.errorDetails || []
+    };
+
+    res.render('error', data, function renderResponse(err, html) {
+        if (!err) {
+            return res.send(html);
+        }
+
+        // re-attach new error e.g. error template has syntax error or misusage
+        req.err = err;
+
+        // And then try to explain things to the user...
+        // Cheat and output the error using handlebars escapeExpression
+        return res.status(500).send(_private.ErrorFallbackMessage(err));
     });
 };
 
@@ -158,4 +177,13 @@ errorHandler.handleHTMLResponse = [
     _private.BasicErorRenderer
 ];
 
+errorHandler.handleThemeResponse = [
+    // Make sure the error can be served
+    _private.prepareError,
+    // Render the error using theme template
+    _private.ThemeErrorRenderer,
+    // Fall back to basic if HTML is not explicitly accepted
+    _private.BasicErorRenderer
+];
+
 module.exports = errorHandler;

core/server/web/site/app.js

@@ -127,7 +127,7 @@ module.exports = function setupSiteApp() {
 
     // ### Error handlers
     siteApp.use(errorHandler.pageNotFound);
-    siteApp.use(errorHandler.handleHTMLResponse);
+    siteApp.use(errorHandler.handleThemeResponse);
 
     debug('Site setup end');