Merge pull request #1815 from ErisDS/more-tests

Error handling simplification & test improvements
TryGhost · Jan 3, 2014 · a50f5e1 · a50f5e1
2 parents 029a01f + 495b7b7
commit a50f5e1
Show file tree

Hide file tree

Showing 4 changed files with 244 additions and 86 deletions.
diff --git a/core/server/errorHandling.js b/core/server/errorHandling.js
@@ -9,17 +9,17 @@ var _           = require('underscore'),
     // Paths for views
     defaultErrorTemplatePath = path.resolve(configPaths().adminViews, 'user-error.hbs'),
     userErrorTemplatePath    = path.resolve(configPaths().themePath, 'error.hbs'),
-    userErrorTemplateExists,
+    userErrorTemplateExists   = false,
 
     ONE_HOUR_S  = 60 * 60;
 
 /**
  * Basic error handling helpers
  */
 errors = {
-    updateActiveTheme: function (activeTheme) {
+    updateActiveTheme: function (activeTheme, hasErrorTemplate) {
         userErrorTemplatePath = path.resolve(configPaths().themePath, activeTheme, 'error.hbs');
-        userErrorTemplateExists = undefined;
+        userErrorTemplateExists = hasErrorTemplate;
     },
 
     throwError: function (err) {
@@ -165,20 +165,7 @@ errors = {
         }
 
         // We're not admin and the template doesn't exist. Render the default.
-        if (userErrorTemplateExists === false) {
-            return renderErrorInt(defaultErrorTemplatePath);
-        }
-
-        // userErrorTemplateExists is undefined, which means we
-        // haven't yet checked for it. Do so now!
-        fs.stat(userErrorTemplatePath, function (err, stat) {
-            userErrorTemplateExists = !err;
-            if (userErrorTemplateExists) {
-                return renderErrorInt();
-            }
-
-            renderErrorInt(defaultErrorTemplatePath);
-        });
+        return renderErrorInt(defaultErrorTemplatePath);
     },
 
     error404: function (req, res, next) {

diff --git a/core/server/middleware/index.js b/core/server/middleware/index.js
@@ -114,7 +114,7 @@ function activateTheme(activeTheme) {
     expressServer.set('theme view engine', hbs.express3(hbsOptions));
 
     // Update user error template
-    errors.updateActiveTheme(activeTheme);
+    errors.updateActiveTheme(activeTheme, config.paths().availableThemes[activeTheme].hasOwnProperty('error'));
 }
 
  // ### ManageAdminAndTheme Middleware

diff --git a/core/test/functional/routes/admin_test.js b/core/test/functional/routes/admin_test.js
@@ -39,7 +39,7 @@ describe('Admin Routing', function () {
 
     before(function (done) {
         testUtils.clearData().then(function () {
-            // we initialise data, but not a user. No user should be required for navigating the frontend
+            // we initialise data, but not a user.
             return testUtils.initData();
         }).then(function () {
             done();