Moved express init + sentry to a shared util

- added core/shared to watched folders in grunt
- moved sentry to shared
- moved express initialisation to a shared file
- always set trust proxy + sentry error handler
- use this new express init everywhere, and remove duplicate trust proxy and sentry error handler code

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'

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

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);
 

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);
 

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);
 

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);
 

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);
 

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);
 

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);
 

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')());

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 = {};

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);
 

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;
+};

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);
             }
         }),

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');

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,