✨ Implement custom errors 2.0 (#8148)

closes #8079

- add a new view type of defaultViews, as this is NOTHING to do with the admin!
- rename user-error.hbs to error.hbs, because this can be for any sort of error
- reimplement custom errors, but with a stack like channels & single templates
- change ghost_head to only not output on 500+ server errors, rather than 400+ user errors
- add coverage for the new template functions

core/server/config/overrides.json

@@ -5,6 +5,7 @@
         "clientAssets": "core/built/assets",
         "helperTemplates": "core/server/helpers/tpl/",
         "adminViews": "core/server/views/",
+        "defaultViews": "core/server/views/",
         "internalAppPath": "core/server/apps/",
         "internalStoragePath": "core/server/storage/",
         "internalSchedulingPath": "core/server/scheduling/",

core/server/controllers/frontend/templates.js

@@ -2,9 +2,35 @@
 //
 // Figure out which template should be used to render a request
 // based on the templates which are allowed, and what is available in the theme
+// TODO: consider where this should live as it deals with channels, singles, and errors
 var _ = require('lodash'),
+    path = require('path'),
+    config = require('../../config'),
     themes = require('../../themes');
 
+/**
+ * ## Get Error Template Hierarchy
+ *
+ * Fetch the ordered list of templates that can be used to render this error statusCode.
+ *
+ * The default is the
+ *
+ * @param {integer} statusCode
+ * @returns {String[]}
+ */
+function getErrorTemplateHierarchy(statusCode) {
+    var errorCode = _.toString(statusCode),
+        templateList = ['error'];
+
+    // Add error class template: E.g. error-4xx.hbs or error-5xx.hbs
+    templateList.unshift('error-' + errorCode[0] + 'xx');
+
+    // Add statusCode specific template: E.g. error-404.hbs
+    templateList.unshift('error-' + errorCode);
+
+    return templateList;
+}
+
 /**
  * ## Get Channel Template Hierarchy
  *
@@ -103,8 +129,15 @@ function getTemplateForChannel(channelOpts) {
     return pickTemplate(templateList, fallback);
 }
 
+function getTemplateForError(statusCode) {
+    var templateList = getErrorTemplateHierarchy(statusCode),
+        fallback = path.resolve(config.get('paths').defaultViews, 'error.hbs');
+    return pickTemplate(templateList, fallback);
+}
+
 module.exports = {
     channel: getTemplateForChannel,
     single: getTemplateForSingle,
+    error: getTemplateForError,
     pickTemplate: pickTemplate
 };

core/server/helpers/ghost_head.js

@@ -77,8 +77,8 @@ function getAjaxHelper(clientId, clientSecret) {
 }
 
 function ghost_head(options) {
-    // if error page do nothing
-    if (this.statusCode >= 400) {
+    // if server error page do nothing
+    if (this.statusCode >= 500) {
         return;
     }
 

core/server/middleware/error-handler.js

@@ -1,9 +1,9 @@
 var _ = require('lodash'),
-    path = require('path'),
     hbs = require('express-hbs'),
     config = require('../config'),
     errors = require('../errors'),
     i18n = require('../i18n'),
+    templates = require('../controllers/frontend/templates'),
     _private = {},
     errorHandler = {};
 
@@ -99,9 +99,7 @@ _private.JSONErrorRenderer = function JSONErrorRenderer(err, req, res, /*jshint
 };
 
 _private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, /*jshint unused:false */ next) {
-    // @TODO re-implement custom error templates see #8079
-    var defaultTemplate = path.resolve(config.get('paths').adminViews, 'user-error.hbs'),
-        templateData = {
+    var templateData = {
             message: err.message,
             code: err.statusCode
         };
@@ -116,7 +114,7 @@ _private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, /*jshint un
         req.app.engine('hbs', hbs.express3());
     }
 
-    res.render(defaultTemplate, templateData, function renderResponse(err, html) {
+    res.render(templates.error(err.statusCode), templateData, function renderResponse(err, html) {
         if (!err) {
             return res.send(html);
         }

core/test/unit/controllers/frontend/templates_spec.js

@@ -210,4 +210,83 @@ describe('templates', function () {
             view.should.eql('index');
         });
     });
+
+    describe('error', function () {
+        beforeEach(function () {
+            hasTemplateStub = sandbox.stub().returns(false);
+
+            getActiveThemeStub = sandbox.stub(themes, 'getActive').returns({
+                hasTemplate: hasTemplateStub
+            });
+        });
+
+        it('will fall back to default if there is no activeTheme', function () {
+            getActiveThemeStub.returns(undefined);
+
+            templates.error(500).should.match(/core\/server\/views\/error.hbs$/);
+        });
+
+        it('will fall back to default for all statusCodes with no custom error templates', function () {
+            templates.error(500).should.match(/core\/server\/views\/error.hbs$/);
+            templates.error(503).should.match(/core\/server\/views\/error.hbs$/);
+            templates.error(422).should.match(/core\/server\/views\/error.hbs$/);
+            templates.error(404).should.match(/core\/server\/views\/error.hbs$/);
+        });
+
+        it('will use custom error.hbs for all statusCodes if there are no other templates', function () {
+            hasTemplateStub.withArgs('error').returns(true);
+
+            templates.error(500).should.eql('error');
+            templates.error(503).should.eql('error');
+            templates.error(422).should.eql('error');
+            templates.error(404).should.eql('error');
+        });
+
+        it('will use more specific error-4xx.hbs for all 4xx statusCodes if available', function () {
+            hasTemplateStub.withArgs('error').returns(true);
+            hasTemplateStub.withArgs('error-4xx').returns(true);
+
+            templates.error(500).should.eql('error');
+            templates.error(503).should.eql('error');
+            templates.error(422).should.eql('error-4xx');
+            templates.error(404).should.eql('error-4xx');
+        });
+
+        it('will use explicit error-404.hbs for 404 statusCode if available', function () {
+            hasTemplateStub.withArgs('error').returns(true);
+            hasTemplateStub.withArgs('error-4xx').returns(true);
+            hasTemplateStub.withArgs('error-404').returns(true);
+
+            templates.error(500).should.eql('error');
+            templates.error(503).should.eql('error');
+            templates.error(422).should.eql('error-4xx');
+            templates.error(404).should.eql('error-404');
+        });
+
+        it('cascade works the same for 500 errors', function () {
+            hasTemplateStub.withArgs('error').returns(true);
+            hasTemplateStub.withArgs('error-5xx').returns(true);
+            hasTemplateStub.withArgs('error-503').returns(true);
+
+            templates.error(500).should.eql('error-5xx');
+            templates.error(503).should.eql('error-503');
+            templates.error(422).should.eql('error');
+            templates.error(404).should.eql('error');
+        });
+
+        it('cascade works with many specific templates', function () {
+            hasTemplateStub.withArgs('error').returns(true);
+            hasTemplateStub.withArgs('error-5xx').returns(true);
+            hasTemplateStub.withArgs('error-503').returns(true);
+            hasTemplateStub.withArgs('error-4xx').returns(true);
+            hasTemplateStub.withArgs('error-404').returns(true);
+
+            templates.error(500).should.eql('error-5xx');
+            templates.error(503).should.eql('error-503');
+            templates.error(422).should.eql('error-4xx');
+            templates.error(404).should.eql('error-404');
+            templates.error(401).should.eql('error-4xx');
+            templates.error(501).should.eql('error-5xx');
+        });
+    });
 });