Decouple sentry from analytics events handlers by expecting general exception handler

no issue

- The class should not rely on being passed a specific dependency, but rather needs to communicate with types what structure and method it needs to function correctly.
- Replaced the specific dependency to `sentry` with a generic definition of what is expected.

ghost/core/core/server/services/segment/DomainEventsAnalytics.js

@@ -9,7 +9,8 @@ const {MilestoneCreatedEvent} = require('@tryghost/milestones');
  */
 
 /**
- * @typedef {import('../../../shared/sentry')} sentry
+ * @typedef {object} IExceptionHandler
+ * @prop {(err: Error) => void} captureException
  */
 
 /**
@@ -22,7 +23,7 @@ const {MilestoneCreatedEvent} = require('@tryghost/milestones');
  * @param {logging} logging
  * @param {object} trackDefaults
  * @param {string} prefix
- * @param {sentry} sentry
+ * @param {IExceptionHandler} exceptionHandler
  * @param {DomainEvents} DomainEvents
  * @prop {} subscribeToEvents
  */
@@ -34,8 +35,8 @@ module.exports = class DomainEventsAnalytics {
     #trackDefaults;
     /** @type {string} */
     #prefix;
-    /** @type {sentry} */
-    #sentry;
+    /** @type {IExceptionHandler} */
+    #exceptionHandler;
     /** @type {logging} */
     #logging;
     /** @type {DomainEvents} */
@@ -45,7 +46,7 @@ module.exports = class DomainEventsAnalytics {
         this.#analytics = deps.analytics;
         this.#trackDefaults = deps.trackDefaults;
         this.#prefix = deps.prefix;
-        this.#sentry = deps.sentry;
+        this.#exceptionHandler = deps.exceptionHandler;
         this.#logging = deps.logging;
         this.#DomainEvents = deps.DomainEvents;
     }
@@ -63,13 +64,13 @@ module.exports = class DomainEventsAnalytics {
         if (event.data.milestone
             && event.data.milestone.value === 100
         ) {
-            const eventName = event.data.milestone.type === 'arr' ? '$100 MRR reached' : '100 members reached';
+            const eventName = event.data.milestone.type === 'arr' ? '$100 MRR reached' : '100 Members reached';
 
             try {
                 this.#analytics.track(Object.assign(this.#trackDefaults, {}, {event: this.#prefix + eventName}));
             } catch (err) {
                 this.#logging.error(err);
-                this.#sentry.captureException(err);
+                this.#exceptionHandler.captureException(err);
             }
         }
     }

ghost/core/core/server/services/segment/ModelEventsAnalytics.js

@@ -9,7 +9,8 @@ const _ = require('lodash');
  */
 
 /**
- * @typedef {import('../../../shared/sentry')} sentry
+ * @typedef {object} IExceptionHandler
+ * @prop {(err: Error) => void} captureException
  */
 
 /**
@@ -22,7 +23,7 @@ const _ = require('lodash');
  * @param {logging} logging
  * @param {object} trackDefaults
  * @param {string} prefix
- * @param {sentry} sentry
+ * @param {IExceptionHandler} exceptionHandler
  * @param {events} events
  * @prop {} subscribeToEvents
  */
@@ -38,8 +39,8 @@ module.exports = class ModelEventsAnalytics {
     #trackDefaults;
     /** @type {string} */
     #prefix;
-    /** @type {sentry} */
-    #sentry;
+    /** @type {IExceptionHandler} */
+    #exceptionHandler;
     /** @type {events} */
     #events;
     /** @type {logging} */
@@ -74,7 +75,7 @@ module.exports = class ModelEventsAnalytics {
         this.#analytics = deps.analytics;
         this.#trackDefaults = deps.trackDefaults;
         this.#prefix = deps.prefix;
-        this.#sentry = deps.sentry;
+        this.#exceptionHandler = deps.exceptionHandler;
         this.#events = deps.events;
         this.#logging = deps.logging;
     }
@@ -84,7 +85,7 @@ module.exports = class ModelEventsAnalytics {
             this.#analytics.track(event);
         } catch (err) {
             this.#logging.error(err);
-            this.#sentry.captureException(err);
+            this.#exceptionHandler.captureException(err);
         }
     }
 

ghost/core/core/server/services/segment/index.js

@@ -17,7 +17,7 @@ module.exports.init = function () {
         analytics,
         trackDefaults,
         prefix,
-        sentry,
+        exceptionHandler: sentry,
         DomainEvents,
         logging
     });
@@ -26,14 +26,14 @@ module.exports.init = function () {
         analytics,
         trackDefaults,
         prefix,
-        sentry,
+        exceptionHandler: sentry,
         events,
         logging
     });
 
     // Listen to model events
-    modelEventsAnalytics.subscribeToModelEvents();
+    modelEventsAnalytics.subscribeToEvents();
 
     // Listen to domain events
-    subscribeToDomainEvents.subscribeToDomainEvents();
+    subscribeToDomainEvents.subscribeToEvents();
 };

ghost/core/test/unit/server/services/segment/DomainEventsAnalytics.test.js

@@ -19,13 +19,13 @@ describe('DomainEventsAnalytics', function () {
     describe('Domain events analytics service', function () {
         let domainEventsAnalytics;
         let analyticsStub;
-        let sentryStub;
+        let exceptionHandlerStub;
         let loggingStub;
         let domainEventsStub;
 
         beforeEach(function () {
             analyticsStub = sinon.stub();
-            sentryStub = sinon.stub();
+            exceptionHandlerStub = sinon.stub();
             loggingStub = sinon.stub();
             domainEventsStub = sinon.stub();
         });
@@ -42,8 +42,8 @@ describe('DomainEventsAnalytics', function () {
                     properties: {email: 'john@test.com'}
                 },
                 prefix: 'Pro: ',
-                sentry: {
-                    captureException: sentryStub
+                exceptionHandler: {
+                    captureException: exceptionHandlerStub
                 },
                 DomainEvents: {
                     subscribe: domainEventsStub
@@ -68,8 +68,8 @@ describe('DomainEventsAnalytics', function () {
                     properties: {email: 'john@test.com'}
                 },
                 prefix: 'Pro: ',
-                sentry: {
-                    captureException: sentryStub
+                exceptionHandler: {
+                    captureException: exceptionHandlerStub
                 },
                 DomainEvents,
                 logging: {
@@ -95,7 +95,7 @@ describe('DomainEventsAnalytics', function () {
             assert(analyticsStub.calledWith({
                 userId: '1234',
                 properties: {email: 'john@test.com'},
-                event: 'Pro: 100 members reached'
+                event: 'Pro: 100 Members reached'
             }));
 
             DomainEvents.dispatch(MilestoneCreatedEvent.create({
@@ -124,8 +124,8 @@ describe('DomainEventsAnalytics', function () {
                     properties: {email: 'john+arr@test.com'}
                 },
                 prefix: 'Pro: ',
-                sentry: {
-                    captureException: sentryStub
+                exceptionHandler: {
+                    captureException: exceptionHandlerStub
                 },
                 DomainEvents,
                 logging: {
@@ -181,8 +181,8 @@ describe('DomainEventsAnalytics', function () {
                 },
                 trackDefaults: {},
                 prefix: '',
-                sentry: {
-                    captureException: sentryStub
+                exceptionHandler: {
+                    captureException: exceptionHandlerStub
                 },
                 DomainEvents,
                 logging: {
@@ -207,8 +207,8 @@ describe('DomainEventsAnalytics', function () {
                 await DomainEvents.allSettled();
             } catch (err) {
                 assert(analyticsStub.callCount === 1);
-                assert(sentryStub.callCount === 1);
-                assert(sentryStub.calledWith(error));
+                assert(exceptionHandlerStub.callCount === 1);
+                assert(exceptionHandlerStub.calledWith(error));
                 assert(loggingStub.callCount === 1);
             }
         });

ghost/core/test/unit/server/services/segment/ModelEventsAnalytics.test.js

@@ -17,13 +17,13 @@ describe('ModelEventsAnalytics', function () {
     describe('Model events analytics service', function () {
         let modelEventsAnalytics;
         let analyticsStub;
-        let sentryStub;
+        let exceptionHandlerStub;
         let loggingStub;
         let eventStub;
 
         beforeEach(function () {
             analyticsStub = sinon.stub();
-            sentryStub = sinon.stub();
+            exceptionHandlerStub = sinon.stub();
             loggingStub = sinon.stub();
             eventStub = sinon.stub();
         });
@@ -40,8 +40,8 @@ describe('ModelEventsAnalytics', function () {
                     properties: {email: 'john@test.com'}
                 },
                 prefix: 'Pro: ',
-                sentry: {
-                    captureException: sentryStub
+                exceptionHandler: {
+                    captureException: exceptionHandlerStub
                 },
                 events: {
                     on: eventStub
@@ -67,8 +67,8 @@ describe('ModelEventsAnalytics', function () {
                     properties: {email: 'john@test.com'}
                 },
                 prefix: 'Pro: ',
-                sentry: {
-                    captureException: sentryStub
+                exceptionHandler: {
+                    captureException: exceptionHandlerStub
                 },
                 events,
                 logging: {
@@ -124,8 +124,8 @@ describe('ModelEventsAnalytics', function () {
                 },
                 trackDefaults: {},
                 prefix: '',
-                sentry: {
-                    captureException: sentryStub
+                exceptionHandler: {
+                    captureException: exceptionHandlerStub
                 },
                 events,
                 logging: {
@@ -139,8 +139,8 @@ describe('ModelEventsAnalytics', function () {
                 events.emit('post.published');
             } catch (err) {
                 assert(analyticsStub.callCount === 1);
-                assert(sentryStub.callCount === 1);
-                assert(sentryStub.calledWith(error));
+                assert(exceptionHandlerStub.callCount === 1);
+                assert(exceptionHandlerStub.calledWith(error));
                 assert(loggingStub.callCount === 1);
             }
         });