From c86933f44f232dbb705e30b2ae50a7f153504453 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 26 May 2020 19:10:29 +0100 Subject: [PATCH] Remove common errors (#11848) * refactored core/frontend/services/proxy to import common dependency like a normal person * removed all imports of `common/errors` * :fire: removed common/errors module Co-authored-by: Vikas Potluri --- core/frontend/services/proxy.js | 8 +- .../services/themes/handlebars/register.js | 3 +- core/frontend/services/themes/i18n.js | 3 +- core/server/lib/common/errors.js | 95 ------------------- core/server/lib/common/index.js | 4 - core/server/models/user.js | 4 +- test/unit/api/canary/session_spec.js | 2 +- test/unit/api/v2/session_spec.js | 2 +- test/unit/api/v3/session_spec.js | 2 +- test/unit/models/user_spec.js | 3 +- test/unit/services/auth/members/index_spec.js | 2 +- 11 files changed, 16 insertions(+), 112 deletions(-) delete mode 100644 core/server/lib/common/errors.js diff --git a/core/frontend/services/proxy.js b/core/frontend/services/proxy.js index 014d15fdb8..5460a52f5b 100644 --- a/core/frontend/services/proxy.js +++ b/core/frontend/services/proxy.js @@ -2,7 +2,9 @@ // With the exception of modules like lodash, Bluebird // We can later refactor to enforce this something like we did in apps const hbs = require('./themes/engine'); +const errors = require('@tryghost/errors'); +const {i18n, logging} = require('../../server/lib/common'); const settingsCache = require('../../server/services/settings/cache'); const config = require('../../server/config'); @@ -25,9 +27,9 @@ module.exports = { settingsCache: settingsCache, // These 3 are kind of core and required all the time - errors: require('../../server/lib/common/errors'), - i18n: require('../../server/lib/common/i18n'), - logging: require('../../server/lib/common/logging'), + errors, + i18n, + logging, // Theme i18n is separate to common i18n themeI18n: require('./themes/i18n'), diff --git a/core/frontend/services/themes/handlebars/register.js b/core/frontend/services/themes/handlebars/register.js index 2d5fab0ebf..4e1c4d9efc 100644 --- a/core/frontend/services/themes/handlebars/register.js +++ b/core/frontend/services/themes/handlebars/register.js @@ -1,7 +1,8 @@ const Promise = require('bluebird'); +const errors = require('@tryghost/errors'); const hbs = require('../engine'); const config = require('../../../../server/config'); -const {errors, logging} = require('../../../../server/lib/common'); +const {logging} = require('../../../../server/lib/common'); // Register an async handlebars helper for a given handlebars instance function asyncHelperWrapper(hbs, name, fn) { diff --git a/core/frontend/services/themes/i18n.js b/core/frontend/services/themes/i18n.js index f0dfd67eea..eb5b105ea6 100644 --- a/core/frontend/services/themes/i18n.js +++ b/core/frontend/services/themes/i18n.js @@ -1,4 +1,5 @@ -const {i18n, events, logging, errors} = require('../../../server/lib/common'); +const errors = require('@tryghost/errors'); +const {i18n, events, logging} = require('../../../server/lib/common'); const settingsCache = require('../../../server/services/settings/cache'); const config = require('../../../server/config'); diff --git a/core/server/lib/common/errors.js b/core/server/lib/common/errors.js deleted file mode 100644 index 2c92531f1a..0000000000 --- a/core/server/lib/common/errors.js +++ /dev/null @@ -1,95 +0,0 @@ -const merge = require('lodash/merge'); -const each = require('lodash/each'); -const util = require('util'); -const errors = require('ghost-ignition').errors; - -function GhostError(options) { - options = options || {}; - this.value = options.value; - - errors.IgnitionError.call(this, options); -} - -const ghostErrors = { - DataExportError: function DataExportError(options) { - GhostError.call(this, merge({ - statusCode: 500, - errorType: 'DataExportError' - }, options)); - }, - DataImportError: function DataImportError(options) { - GhostError.call(this, merge({ - statusCode: 500, - errorType: 'DataImportError' - }, options)); - }, - DatabaseVersionError: function DatabaseVersionError(options) { - GhostError.call(this, merge({ - hideStack: true, - statusCode: 500, - errorType: 'DatabaseVersionError' - }, options)); - }, - EmailError: function EmailError(options) { - GhostError.call(this, merge({ - statusCode: 500, - errorType: 'EmailError' - }, options)); - }, - ThemeValidationError: function ThemeValidationError(options) { - GhostError.call(this, merge({ - statusCode: 422, - errorType: 'ThemeValidationError', - errorDetails: {} - }, options)); - }, - DisabledFeatureError: function DisabledFeatureError(options) { - GhostError.call(this, merge({ - statusCode: 409, - errorType: 'DisabledFeatureError' - }, options)); - }, - UpdateCollisionError: function UpdateCollisionError(options) { - GhostError.call(this, merge({ - statusCode: 409, - errorType: 'UpdateCollisionError' - }, options)); - }, - HostLimitError: function HostLimitError(options) { - GhostError.call(this, merge({ - errorType: 'HostLimitError', - hideStack: true, - statusCode: 403 - }, options)); - }, - HelperWarning: function HelperWarning(options) { - GhostError.call(this, merge({ - errorType: 'HelperWarning', - hideStack: true - }, options)); - }, - PasswordResetRequiredError: function PasswordResetRequiredError(options) { - GhostError.call(this, merge({ - errorType: 'PasswordResetRequiredError', - statusCode: 401, - message: 'For security, you need to create a new password. An email has been sent to you with instructions!' - }, options)); - } -}; - -util.inherits(GhostError, errors.IgnitionError); -each(ghostErrors, function (error) { - util.inherits(error, GhostError); -}); - -// we need to inherit all general errors from GhostError, otherwise we have to check instanceof IgnitionError -each(errors, function (error) { - if (error.name === 'IgnitionError' || typeof error === 'object') { - return; - } - - util.inherits(error, GhostError); -}); - -module.exports = merge(ghostErrors, errors); -module.exports.GhostError = GhostError; diff --git a/core/server/lib/common/index.js b/core/server/lib/common/index.js index 6a9013ba32..9bc77f0c34 100644 --- a/core/server/lib/common/index.js +++ b/core/server/lib/common/index.js @@ -7,10 +7,6 @@ module.exports = { return require('./events'); }, - get errors() { - return require('./errors'); - }, - get logging() { return require('./logging'); } diff --git a/core/server/models/user.js b/core/server/models/user.js index d465b91d34..23f45c783f 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -4,7 +4,7 @@ const validator = require('validator'); const ObjectId = require('bson-objectid'); const ghostBookshelf = require('./base'); const baseUtils = require('./base/utils'); -const {i18n, errors: {PasswordResetRequiredError}} = require('../lib/common'); +const {i18n} = require('../lib/common'); const errors = require('@tryghost/errors'); const security = require('../lib/security'); const imageLib = require('../lib/image'); @@ -806,7 +806,7 @@ User = ghostBookshelf.Model.extend({ } if (user.isLocked()) { - throw new PasswordResetRequiredError(); + throw new errors.PasswordResetRequiredError(); } if (user.isInactive()) { diff --git a/test/unit/api/canary/session_spec.js b/test/unit/api/canary/session_spec.js index ae37dd67d0..ca48e7b971 100644 --- a/test/unit/api/canary/session_spec.js +++ b/test/unit/api/canary/session_spec.js @@ -1,8 +1,8 @@ const should = require('should'); const sinon = require('sinon'); +const {UnauthorizedError} = require('@tryghost/errors'); const models = require('../../../../core/server/models'); -const {UnauthorizedError} = require('../../../../core/server/lib/common/errors'); const sessionController = require('../../../../core/server/api/canary/session'); const sessionServiceMiddleware = require('../../../../core/server/services/auth/session'); diff --git a/test/unit/api/v2/session_spec.js b/test/unit/api/v2/session_spec.js index 0dff56ede1..18d0b06717 100644 --- a/test/unit/api/v2/session_spec.js +++ b/test/unit/api/v2/session_spec.js @@ -1,8 +1,8 @@ const should = require('should'); const sinon = require('sinon'); +const {UnauthorizedError} = require('@tryghost/errors'); const models = require('../../../../core/server/models'); -const {UnauthorizedError} = require('../../../../core/server/lib/common/errors'); const sessionController = require('../../../../core/server/api/v2/session'); const sessionServiceMiddleware = require('../../../../core/server/services/auth/session'); diff --git a/test/unit/api/v3/session_spec.js b/test/unit/api/v3/session_spec.js index 9978159378..30117fbebf 100644 --- a/test/unit/api/v3/session_spec.js +++ b/test/unit/api/v3/session_spec.js @@ -1,8 +1,8 @@ const should = require('should'); const sinon = require('sinon'); +const {UnauthorizedError} = require('@tryghost/errors'); const models = require('../../../../core/server/models'); -const {UnauthorizedError} = require('../../../../core/server/lib/common/errors'); const sessionController = require('../../../../core/server/api/canary/session'); const sessionServiceMiddleware = require('../../../../core/server/services/auth/session'); diff --git a/test/unit/models/user_spec.js b/test/unit/models/user_spec.js index f70b3101c9..a2a83c6e34 100644 --- a/test/unit/models/user_spec.js +++ b/test/unit/models/user_spec.js @@ -5,7 +5,6 @@ const errors = require('@tryghost/errors'); const models = require('../../../core/server/models'); const permissions = require('../../../core/server/services/permissions'); const validation = require('../../../core/server/data/validation'); -const {errors: commonErrors} = require('../../../core/server/lib/common'); const security = require('../../../core/server/lib/security'); const testUtils = require('../../utils'); @@ -144,7 +143,7 @@ describe('Unit: models/user', function () { return models.User.check({email: user.get('email'), password: 'test'}) .catch(function (err) { - (err instanceof commonErrors.PasswordResetRequiredError).should.eql(true); + (err instanceof errors.PasswordResetRequiredError).should.eql(true); }); }); }); diff --git a/test/unit/services/auth/members/index_spec.js b/test/unit/services/auth/members/index_spec.js index 661c116bfa..f42e4bedab 100644 --- a/test/unit/services/auth/members/index_spec.js +++ b/test/unit/services/auth/members/index_spec.js @@ -1,6 +1,6 @@ const jwt = require('jsonwebtoken'); const should = require('should'); -const {UnauthorizedError} = require('../../../../../core/server/lib/common/errors'); +const {UnauthorizedError} = require('@tryghost/errors'); const members = require('../../../../../core/server/services/auth/members'); describe.skip('Auth Service - Members', function () {