diff --git a/Gruntfile.js b/Gruntfile.js index 884df1456d..8d63495395 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -87,6 +87,7 @@ const configureGrunt = function (grunt) { files: [ 'core/ghost-server.js', 'core/server/**/*.js', + 'core/shared/**/*.js', 'core/frontend/**/*.js', 'config.*.json', '!config.testing.json' diff --git a/core/server/services/bulk-email/index.js b/core/server/services/bulk-email/index.js index 251f8ceea1..d22dcc2d6b 100644 --- a/core/server/services/bulk-email/index.js +++ b/core/server/services/bulk-email/index.js @@ -3,7 +3,7 @@ const common = require('../../lib/common'); const mailgunProvider = require('./mailgun'); const configService = require('../../config'); const settingsCache = require('../settings/cache'); -const sentry = require('../../sentry'); +const sentry = require('../../../shared/sentry'); /** * An object representing batch request result diff --git a/core/server/web/admin/app.js b/core/server/web/admin/app.js index a69724aca0..61eb879793 100644 --- a/core/server/web/admin/app.js +++ b/core/server/web/admin/app.js @@ -1,21 +1,15 @@ const debug = require('ghost-ignition').debug('web:admin:app'); -const express = require('express'); +const express = require('../../../shared/express'); const serveStatic = require('express').static; const config = require('../../config'); const constants = require('../../lib/constants'); const urlUtils = require('../../lib/url-utils'); const shared = require('../shared'); const adminMiddleware = require('./middleware'); -const sentry = require('../../sentry'); module.exports = function setupAdminApp() { debug('Admin setup start'); const adminApp = express(); - adminApp.use(sentry.requestHandler); - - // Make sure 'req.secure' and `req.hostname` is valid for proxied requests - // (X-Forwarded-Proto header will be checked, if present) - adminApp.enable('trust proxy'); // Admin assets // @TODO ensure this gets a local 404 error handler @@ -52,7 +46,6 @@ module.exports = function setupAdminApp() { // Finally, routing adminApp.get('*', require('./controller')); - adminApp.use(sentry.errorHandler); adminApp.use(shared.middlewares.errorHandler.pageNotFound); adminApp.use(shared.middlewares.errorHandler.handleHTMLResponse); diff --git a/core/server/web/api/app.js b/core/server/web/api/app.js index 4e3c1506e2..a2b70a87a1 100644 --- a/core/server/web/api/app.js +++ b/core/server/web/api/app.js @@ -1,13 +1,11 @@ const debug = require('ghost-ignition').debug('web:api:default:app'); -const express = require('express'); +const express = require('../../../shared/express'); const urlUtils = require('../../lib/url-utils'); const errorHandler = require('../shared/middlewares/error-handler'); -const sentry = require('../../sentry'); module.exports = function setupApiApp() { debug('Parent API setup start'); const apiApp = express(); - apiApp.use(sentry.requestHandler); // Mount different API versions apiApp.use(urlUtils.getVersionPath({version: 'v2', type: 'content'}), require('./v2/content/app')()); @@ -22,7 +20,6 @@ module.exports = function setupApiApp() { apiApp.use(urlUtils.getVersionPath({version: 'canary', type: 'members'}), require('./canary/members/app')()); // Error handling for requests to non-existent API versions - apiApp.use(sentry.errorHandler); apiApp.use(errorHandler.resourceNotFound); apiApp.use(errorHandler.handleJSONResponse); diff --git a/core/server/web/api/canary/admin/app.js b/core/server/web/api/canary/admin/app.js index 589aac99d2..74339f72aa 100644 --- a/core/server/web/api/canary/admin/app.js +++ b/core/server/web/api/canary/admin/app.js @@ -1,16 +1,14 @@ const debug = require('ghost-ignition').debug('web:canary:admin:app'); const boolParser = require('express-query-boolean'); -const express = require('express'); +const express = require('../../../../../shared/express'); const bodyParser = require('body-parser'); const shared = require('../../../shared'); const apiMw = require('../../middleware'); const routes = require('./routes'); -const sentry = require('../../../../sentry'); module.exports = function setupApiApp() { debug('Admin API canary setup start'); const apiApp = express(); - apiApp.use(sentry.requestHandler); // API middleware @@ -35,7 +33,6 @@ module.exports = function setupApiApp() { apiApp.use(routes()); // API error handling - apiApp.use(sentry.errorHandler); apiApp.use(shared.middlewares.errorHandler.resourceNotFound); apiApp.use(shared.middlewares.errorHandler.handleJSONResponseV2); diff --git a/core/server/web/api/canary/content/app.js b/core/server/web/api/canary/content/app.js index d89bca694a..85a679180c 100644 --- a/core/server/web/api/canary/content/app.js +++ b/core/server/web/api/canary/content/app.js @@ -1,15 +1,13 @@ const debug = require('ghost-ignition').debug('web:api:canary:content:app'); const boolParser = require('express-query-boolean'); const bodyParser = require('body-parser'); -const express = require('express'); +const express = require('../../../../../shared/express'); const shared = require('../../../shared'); const routes = require('./routes'); -const sentry = require('../../../../sentry'); module.exports = function setupApiApp() { debug('Content API canary setup start'); const apiApp = express(); - apiApp.use(sentry.requestHandler); // API middleware @@ -29,7 +27,6 @@ module.exports = function setupApiApp() { apiApp.use(routes()); // API error handling - apiApp.use(sentry.errorHandler); apiApp.use(shared.middlewares.errorHandler.resourceNotFound); apiApp.use(shared.middlewares.errorHandler.handleJSONResponse); diff --git a/core/server/web/api/canary/members/app.js b/core/server/web/api/canary/members/app.js index 2d180f2321..6263b0ab04 100644 --- a/core/server/web/api/canary/members/app.js +++ b/core/server/web/api/canary/members/app.js @@ -1,21 +1,15 @@ const {URL} = require('url'); const debug = require('ghost-ignition').debug('web:canary:members:app'); -const express = require('express'); +const express = require('../../../../../shared/express'); const cors = require('cors'); const membersService = require('../../../../services/members'); const urlUtils = require('../../../../lib/url-utils'); const labs = require('../../../shared/middlewares/labs'); const shared = require('../../../shared'); -const sentry = require('../../../../sentry'); module.exports = function setupMembersApiApp() { debug('Members API canary setup start'); const apiApp = express(); - apiApp.use(sentry.requestHandler); - - // Make sure `req.ip` is correct for proxied requests - // (X-Forwarded-Proto header will be checked, if present) - apiApp.enable('trust proxy'); // Entire app is behind labs flag apiApp.use(labs.members); @@ -31,7 +25,6 @@ module.exports = function setupMembersApiApp() { apiApp.put('/subscriptions/:id', (req, res, next) => membersService.api.middleware.updateSubscription(req, res, next)); // API error handling - apiApp.use(sentry.errorHandler); apiApp.use(shared.middlewares.errorHandler.resourceNotFound); apiApp.use(shared.middlewares.errorHandler.handleJSONResponseV2); diff --git a/core/server/web/api/v2/admin/app.js b/core/server/web/api/v2/admin/app.js index d4b611e02a..8e332d6b88 100644 --- a/core/server/web/api/v2/admin/app.js +++ b/core/server/web/api/v2/admin/app.js @@ -1,16 +1,14 @@ const debug = require('ghost-ignition').debug('web:v2:admin:app'); const boolParser = require('express-query-boolean'); -const express = require('express'); +const express = require('../../../../../shared/express'); const bodyParser = require('body-parser'); const shared = require('../../../shared'); const apiMw = require('../../middleware'); const routes = require('./routes'); -const sentry = require('../../../../sentry'); module.exports = function setupApiApp() { debug('Admin API v2 setup start'); const apiApp = express(); - apiApp.use(sentry.requestHandler); // API middleware @@ -35,7 +33,6 @@ module.exports = function setupApiApp() { apiApp.use(routes()); // API error handling - apiApp.use(sentry.errorHandler); apiApp.use(shared.middlewares.errorHandler.resourceNotFound); apiApp.use(shared.middlewares.errorHandler.handleJSONResponseV2); diff --git a/core/server/web/api/v2/content/app.js b/core/server/web/api/v2/content/app.js index b2eb568294..8674bbaab9 100644 --- a/core/server/web/api/v2/content/app.js +++ b/core/server/web/api/v2/content/app.js @@ -1,15 +1,13 @@ const debug = require('ghost-ignition').debug('web:api:v2:content:app'); const boolParser = require('express-query-boolean'); const bodyParser = require('body-parser'); -const express = require('express'); +const express = require('../../../../../shared/express'); const shared = require('../../../shared'); const routes = require('./routes'); -const sentry = require('../../../../sentry'); module.exports = function setupApiApp() { debug('Content API v2 setup start'); const apiApp = express(); - apiApp.use(sentry.requestHandler); // API middleware @@ -29,7 +27,6 @@ module.exports = function setupApiApp() { apiApp.use(routes()); // API error handling - apiApp.use(sentry.errorHandler); apiApp.use(shared.middlewares.errorHandler.resourceNotFound); apiApp.use(shared.middlewares.errorHandler.handleJSONResponse); diff --git a/core/server/web/parent/app.js b/core/server/web/parent/app.js index a32b182fe5..c881a1068d 100644 --- a/core/server/web/parent/app.js +++ b/core/server/web/parent/app.js @@ -1,5 +1,5 @@ const debug = require('ghost-ignition').debug('web:parent'); -const express = require('express'); +const express = require('../../../shared/express'); const vhost = require('@tryghost/vhost-middleware'); const config = require('../../config'); const compress = require('compression'); @@ -7,18 +7,10 @@ const netjet = require('netjet'); const mw = require('./middleware'); const escapeRegExp = require('lodash.escaperegexp'); const {URL} = require('url'); -const sentry = require('../../sentry'); module.exports = function setupParentApp(options = {}) { debug('ParentApp setup start'); const parentApp = express(); - parentApp.use(sentry.requestHandler); - - // ## Global settings - - // Make sure 'req.secure' is valid for proxied requests - // (X-Forwarded-Proto header will be checked, if present) - parentApp.enable('trust proxy'); parentApp.use(mw.requestId); parentApp.use(mw.logRequest); @@ -52,8 +44,6 @@ module.exports = function setupParentApp(options = {}) { // Wrap the admin and API apps into a single express app for use with vhost const backendApp = express(); - backendApp.use(sentry.requestHandler); - backendApp.enable('trust proxy'); // required to respect x-forwarded-proto in admin requests backendApp.use('/ghost/api', require('../api')()); backendApp.use('/ghost/.well-known', require('../well-known')()); backendApp.use('/ghost', require('../../services/auth/session').createSessionFromToken, require('../admin')()); diff --git a/core/server/web/shared/middlewares/error-handler.js b/core/server/web/shared/middlewares/error-handler.js index 68f73c726b..86c3335786 100644 --- a/core/server/web/shared/middlewares/error-handler.js +++ b/core/server/web/shared/middlewares/error-handler.js @@ -5,7 +5,7 @@ const errors = require('@tryghost/errors'); const config = require('../../../config'); const {i18n} = require('../../../lib/common'); const helpers = require('../../../../frontend/services/routing/helpers'); -const sentry = require('../../../sentry'); +const sentry = require('../../../../shared/sentry'); const escapeExpression = hbs.Utils.escapeExpression; const _private = {}; diff --git a/core/server/web/site/app.js b/core/server/web/site/app.js index c5123ec7b2..69a87df5ab 100644 --- a/core/server/web/site/app.js +++ b/core/server/web/site/app.js @@ -1,6 +1,6 @@ const debug = require('ghost-ignition').debug('web:site:app'); const path = require('path'); -const express = require('express'); +const express = require('../../../shared/express'); const cors = require('cors'); const {URL} = require('url'); const errors = require('@tryghost/errors'); @@ -20,7 +20,6 @@ const membersMiddleware = membersService.middleware; const siteRoutes = require('./routes'); const shared = require('../shared'); const mw = require('./middleware'); -const sentry = require('../../sentry'); const STATIC_IMAGE_URL_PREFIX = `/${urlUtils.STATIC_IMAGE_URL_PREFIX}`; @@ -78,12 +77,6 @@ module.exports = function setupSiteApp(options = {}) { debug('Site setup start'); const siteApp = express(); - siteApp.use(sentry.requestHandler); - - // Make sure 'req.secure' is valid for proxied requests - // (X-Forwarded-Proto header will be checked, if present) - // NB: required here because it's not passed down via vhost - siteApp.enable('trust proxy'); // ## App - specific code // set the view engine @@ -201,7 +194,6 @@ module.exports = function setupSiteApp(options = {}) { siteApp.use(SiteRouter); // ### Error handlers - siteApp.use(sentry.errorHandler); siteApp.use(shared.middlewares.errorHandler.pageNotFound); siteApp.use(shared.middlewares.errorHandler.handleThemeResponse); diff --git a/core/shared/express.js b/core/shared/express.js new file mode 100644 index 0000000000..08a1b70536 --- /dev/null +++ b/core/shared/express.js @@ -0,0 +1,16 @@ +const express = require('express'); +const sentry = require('./sentry'); + +module.exports = () => { + const app = express(); + + // Make sure 'req.secure' is valid for proxied requests + // (X-Forwarded-Proto header will be checked, if present) + // NB: required here because it's not passed down via vhost + app.enable('trust proxy'); + + // Sentry must be our first error handler. Mounting it here means all custom error handlers will come after + app.use(sentry.errorHandler); + + return app; +}; diff --git a/core/server/sentry.js b/core/shared/sentry.js similarity index 77% rename from core/server/sentry.js rename to core/shared/sentry.js index 7772c17441..0aa54839c0 100644 --- a/core/server/sentry.js +++ b/core/shared/sentry.js @@ -1,4 +1,4 @@ -const config = require('./config'); +const config = require('../server/config'); const sentryConfig = config.get('sentry'); const expressNoop = function (req, res, next) { @@ -7,7 +7,7 @@ const expressNoop = function (req, res, next) { if (sentryConfig && !sentryConfig.disabled) { const Sentry = require('@sentry/node'); - const version = require('./lib/ghost-version').full; + const version = require('../server/lib/ghost-version').full; Sentry.init({ dsn: sentryConfig.dsn, release: 'ghost@' + version @@ -18,6 +18,7 @@ if (sentryConfig && !sentryConfig.disabled) { errorHandler: Sentry.Handlers.errorHandler({ shouldHandleError(error) { // Only handle 500 errors for now + // This is because the only other 5XX error should be 503, which are deliberate maintenance/boot errors return (error.statusCode === 500); } }), diff --git a/index.js b/index.js index d050ba032e..bf9ce507a3 100644 --- a/index.js +++ b/index.js @@ -3,7 +3,8 @@ const startTime = Date.now(); const debug = require('ghost-ignition').debug('boot:index'); -const sentry = require('./core/server/sentry'); +// Sentry must be initialised early on +const sentry = require('./core/shared/sentry'); debug('First requires...'); @@ -11,11 +12,13 @@ const ghost = require('./core'); debug('Required ghost'); -const express = require('express'); +const express = require('./core/shared/express'); const common = require('./core/server/lib/common'); const urlService = require('./core/frontend/services/url'); const ghostApp = express(); +// Use the request handler at the top level +// @TODO: decide if this should be here or in parent App - should it come after request id mw? ghostApp.use(sentry.requestHandler); debug('Initialising Ghost'); diff --git a/test/unit/web/parent/app_spec.js b/test/unit/web/parent/app_spec.js index bac076aa21..fc863a10ae 100644 --- a/test/unit/web/parent/app_spec.js +++ b/test/unit/web/parent/app_spec.js @@ -31,8 +31,8 @@ describe('parent app', function () { authPagesSpy = sinon.spy(); parentApp = proxyquire('../../../../core/server/web/parent/app', { - express: expressStub, '@tryghost/vhost-middleware': vhostSpy, + '../../../shared/express': expressStub, '../api': apiSpy, '../admin': adminSpy, '../well-known': wellKnownSpy,