From c0f32b774672e4bb43e16ac9383e4d97bf0ed6b9 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Mon, 19 Jul 2021 11:46:38 +0100 Subject: [PATCH] Replaced usage of Error with @tryghost/errors (#13161) refs https://github.com/TryGhost/Ghost/commit/2f1123d6ca7894fc21499739f46f0ae5c9eda73a Usage of the raw Error class has been deprecated in favour of our own errors, which are more descriptive and have built in HTTP status codes. This also updates the same errors to use @tryghost/tpl for the error messages, which is the new pattern we are following in order for us to deprecate the i18n module. --- core/server/services/members/config.js | 8 +++++++- core/server/services/members/importer/importer.js | 15 +++++++++++---- core/server/services/members/service.js | 14 +++++++++++--- core/server/services/members/stripe-connect.js | 10 ++++++++-- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/core/server/services/members/config.js b/core/server/services/members/config.js index 017af9bd01..23b5cf0e8a 100644 --- a/core/server/services/members/config.js +++ b/core/server/services/members/config.js @@ -1,8 +1,14 @@ +const errors = require('@tryghost/errors'); +const tpl = require('@tryghost/tpl'); const {URL} = require('url'); const crypto = require('crypto'); const createKeypair = require('keypair'); const path = require('path'); +const messages = { + incorrectKeyType: 'type must be one of "direct" or "connect".' +}; + class MembersConfigProvider { /** * @param {object} options @@ -92,7 +98,7 @@ class MembersConfigProvider { */ getStripeKeys(type) { if (type !== 'direct' && type !== 'connect') { - throw new Error(); + throw new errors.IncorrectUsageError(tpl(messages.incorrectKeyType)); } const secretKey = this._settingsCache.get(`stripe_${type === 'connect' ? 'connect_' : ''}secret_key`); diff --git a/core/server/services/members/importer/importer.js b/core/server/services/members/importer/importer.js index d460f95ed9..83180ad963 100644 --- a/core/server/services/members/importer/importer.js +++ b/core/server/services/members/importer/importer.js @@ -2,6 +2,8 @@ const moment = require('moment-timezone'); const path = require('path'); const fs = require('fs-extra'); const membersCSV = require('@tryghost/members-csv'); +const errors = require('@tryghost/errors'); +const tpl = require('@tryghost/tpl'); const GhostMailer = require('../../mail').GhostMailer; const urlUtils = require('../../../../shared/url-utils'); const db = require('../../../data/db'); @@ -9,6 +11,11 @@ const emailTemplate = require('./email-template'); const jobsService = require('../../jobs'); const labsService = require('../../../../shared/labs'); +const messages = { + filenameCollision: 'Filename already exists, please try again.', + jobAlreadyComplete: 'Job is already complete.' +}; + const ghostMailer = new GhostMailer(); module.exports = class MembersCSVImporter { /** @@ -71,7 +78,7 @@ module.exports = class MembersCSVImporter { const pathExists = await fs.pathExists(outputFilePath); if (pathExists) { - throw new Error('Maybe we need better name generation'); + throw new errors.DataImportError(tpl(messages.filenameCollision)); } const rows = await membersCSV.parse(inputFilePath, headerMapping, defaultLabels); @@ -102,7 +109,7 @@ module.exports = class MembersCSVImporter { const job = await this.getJob(id); if (job.status === 'complete') { - throw new Error('Job is already complete'); + throw new errors.BadRequestError(tpl(messages.jobAlreadyComplete)); } const rows = membersCSV.parse(job.filename); @@ -178,8 +185,8 @@ module.exports = class MembersCSVImporter { }; } catch (error) { // The model layer can sometimes throw arrays of errors - const errors = [].concat(error); - const errorMessage = errors.map(({message}) => message).join(', '); + const errorList = [].concat(error); + const errorMessage = errorList.map(({message}) => message).join(', '); await trx.rollback(); return { ...resultAccumulator, diff --git a/core/server/services/members/service.js b/core/server/services/members/service.js index 33fa24339a..bda20885b6 100644 --- a/core/server/services/members/service.js +++ b/core/server/services/members/service.js @@ -1,3 +1,5 @@ +const errors = require('@tryghost/errors'); +const tpl = require('@tryghost/tpl'); const MembersSSR = require('@tryghost/members-ssr'); const db = require('../../data/db'); const MembersConfigProvider = require('./config'); @@ -12,6 +14,12 @@ const config = require('../../../shared/config'); const ghostVersion = require('@tryghost/version'); const _ = require('lodash'); +const messages = { + noLiveKeysInDevelopment: 'Cannot use live stripe keys in development. Please restart in production mode.', + sslRequiredForStripe: 'Cannot run Ghost without SSL when Stripe is connected. Please update your url config to use "https://".', + remoteWebhooksInDevelopment: 'Cannot use remote webhooks in development. See https://ghost.org/docs/webhooks/#stripe-webhooks for developing with Stripe.' +}; + // Bind to settings.edited to update systems based on settings changes, similar to the bridge and models/base/listeners const events = require('../../lib/common/events'); @@ -69,16 +77,16 @@ const membersService = { if (env !== 'production') { if (!process.env.WEBHOOK_SECRET && membersConfig.isStripeConnected()) { process.env.WEBHOOK_SECRET = 'DEFAULT_WEBHOOK_SECRET'; - logging.warn('Cannot use remote webhooks in development. See https://ghost.org/docs/webhooks/#stripe-webhooks for developing with Stripe.'); + logging.warn(tpl(messages.remoteWebhooksInDevelopment)); } if (paymentConfig && paymentConfig.secretKey.startsWith('sk_live')) { - throw new Error('Cannot use live stripe keys in development. Please restart in production mode.'); + throw new errors.IncorrectUsageError(tpl(messages.noLiveKeysInDevelopment)); } } else { const siteUrl = urlUtils.getSiteUrl(); if (!/^https/.test(siteUrl) && membersConfig.isStripeConnected()) { - throw new Error('Cannot run Ghost without SSL when Stripe is connected. Please update your url config to use "https://"'); + throw new errors.IncorrectUsageError(tpl(messages.sslRequiredForStripe)); } } }, diff --git a/core/server/services/members/stripe-connect.js b/core/server/services/members/stripe-connect.js index ab9545a9a7..4dd27f6cc1 100644 --- a/core/server/services/members/stripe-connect.js +++ b/core/server/services/members/stripe-connect.js @@ -1,7 +1,13 @@ +const errors = require('@tryghost/errors'); +const tpl = require('@tryghost/tpl'); const {Buffer} = require('buffer'); const {randomBytes} = require('crypto'); const {URL} = require('url'); +const messages = { + incorrectState: 'State did not match.' +}; + const STATE_PROP = 'stripe-connect-state'; const liveClientID = 'ca_8LBuZWhYshxF0A55KgCXu8PRTquCKC5x'; @@ -45,7 +51,7 @@ async function getStripeConnectOAuthUrl(setSessionProp, mode = 'live') { * @param {string} encodedData - A string encoding the response from Stripe Connect * @param {(prop: string) => Promise} getSessionProp - A function to retrieve data from the current session * - * @returns {Promise<{secret_key: string, public_key: string, livemode: boolean}>} + * @returns {Promise<{secret_key: string, public_key: string, livemode: boolean, display_name: string, account_id: string}>} */ async function getStripeConnectTokenData(encodedData, getSessionProp) { const data = JSON.parse(Buffer.from(encodedData, 'base64').toString()); @@ -53,7 +59,7 @@ async function getStripeConnectTokenData(encodedData, getSessionProp) { const state = await getSessionProp(STATE_PROP); if (state !== data.s) { - throw new Error('State did not match'); + throw new errors.NoPermissionError(tpl(messages.incorrectState)); } return {