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
This commit is contained in:
Hannah Wolfe 2022-11-21 19:10:20 +00:00
parent f6870fa846
commit 682f3a2014
2 changed files with 34 additions and 4 deletions

View File

@ -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({

View File

@ -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 () {