From 682f3a2014b34648ba68e259ca1c24d3c1c598d2 Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <github.erisds@gmail.com>
Date: Mon, 21 Nov 2022 19:10:20 +0000
Subject: [PATCH] Downgraded express-hbs errors to 400

refs: https://github.com/TryGhost/Team/issues/2289
refs: https://github.com/TryGhost/express-hbs/issues/161

- Themes that resuse layouts as templates trigger horrible errors, which are thrown as 500s
- But there's nothing the server is doing wrong, it's a theme user, so we downgrade these to 400s
- There is more to do here to improve the errors shown, but this is just a first step to ensure that theme issues don't look like server failures
---
 .../mw-error-handler/lib/mw-error-handler.js  | 11 ++++++--
 .../test/error-handler.test.js                | 27 +++++++++++++++++--
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/ghost/mw-error-handler/lib/mw-error-handler.js b/ghost/mw-error-handler/lib/mw-error-handler.js
index 1e793b53a5..c37c476df8 100644
--- a/ghost/mw-error-handler/lib/mw-error-handler.js
+++ b/ghost/mw-error-handler/lib/mw-error-handler.js
@@ -1,4 +1,5 @@
 const _ = require('lodash');
+const path = require('path');
 const semver = require('semver');
 const debug = require('@tryghost/debug')('error-handler');
 const errors = require('@tryghost/errors');
@@ -53,6 +54,12 @@ const messages = {
     UnknownError: 'Unknown error - {name}, cannot {action}.'
 };
 
+function isDependencyInStack(dependency, err) {
+    const dependencyPath = path.join('node_modules', dependency);
+
+    return err?.stack?.match(dependencyPath);
+}
+
 /**
  * Get an error ready to be shown the the user
  */
@@ -71,8 +78,8 @@ module.exports.prepareError = (err, req, res, next) => {
             err = new errors.NotFoundError({
                 err: err
             });
-        // Catch handlebars errors, and render them as 400, rather than 500 errors
-        } else if (err.stack.match(/node_modules\/handlebars\//)) {
+        // Catch handlebars / express-hbs errors, and render them as 400, rather than 500 errors as the server isn't broken
+        } else if (isDependencyInStack('handlebars', err) || isDependencyInStack('express-hbs', err)) {
             // Temporary handling of theme errors from handlebars
             // @TODO remove this when #10496 is solved properly
             err = new errors.IncorrectUsageError({
diff --git a/ghost/mw-error-handler/test/error-handler.test.js b/ghost/mw-error-handler/test/error-handler.test.js
index deb209a364..57f18cf68c 100644
--- a/ghost/mw-error-handler/test/error-handler.test.js
+++ b/ghost/mw-error-handler/test/error-handler.test.js
@@ -1,6 +1,7 @@
 // Switch these lines once there are useful utils
 // const testUtils = require('./utils');
 require('./utils');
+const path = require('path');
 const should = require('should');
 const assert = require('assert');
 const {InternalServerError, NotFoundError} = require('@tryghost/errors');
@@ -68,19 +69,41 @@ describe('Prepare Error', function () {
         });
     });
 
-    it('Correctly prepares a handlebars errpr', function (done) {
+    it('Correctly prepares a handlebars error', function (done) {
         let error = new Error('obscure handlebars message!');
-        error.stack += '\nnode_modules/handlebars/something';
+
+        error.stack += '\n';
+        error.stack += path.join('node_modules', 'handlebars', 'something');
 
         prepareError(error, {}, {
             set: () => {}
         }, (err) => {
             err.statusCode.should.eql(400);
             err.name.should.eql('IncorrectUsageError');
+            // TODO: the message shouldn't be trusted here
+            err.message.should.eql('obscure handlebars message!');
             err.stack.should.startWith('Error: obscure handlebars message!');
             done();
         });
     });
+
+    it('Correctly prepares an express-hbs error', function (done) {
+        let error = new Error('obscure express-hbs message!');
+
+        error.stack += '\n';
+        error.stack += path.join('node_modules', 'express-hbs', 'lib');
+
+        prepareError(error, {}, {
+            set: () => {}
+        }, (err) => {
+            err.statusCode.should.eql(400);
+            err.name.should.eql('IncorrectUsageError');
+            // TODO: the message shouldn't be trusted here
+            err.message.should.eql('obscure express-hbs message!');
+            err.stack.should.startWith('Error: obscure express-hbs message!');
+            done();
+        });
+    });
 });
 
 describe('Prepare Stack', function () {