Merged v5.35.1 into main

ghost/admin/package.json

@@ -1,6 +1,6 @@
 {
   "name": "ghost-admin",
-  "version": "5.35.0",
+  "version": "5.35.1",
   "description": "Ember.js admin client for Ghost",
   "author": "Ghost Foundation",
   "homepage": "http://ghost.org",

ghost/core/core/shared/labs.js

@@ -20,7 +20,8 @@ const GA_FEATURES = [
     'memberAttribution',
     'audienceFeedback',
     'themeErrorsNotification',
-    'emailStability'
+    'emailStability',
+    'emailErrors'
 ];
 
 // NOTE: this allowlist is meant to be used to filter out any unexpected
@@ -28,7 +29,6 @@ const GA_FEATURES = [
 const BETA_FEATURES = [
     'activitypub',
     'webmentions',
-    'emailErrors',
     'milestoneEmails'
 ];
 

ghost/core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "ghost",
-  "version": "5.35.0",
+  "version": "5.35.1",
   "description": "The professional publishing platform",
   "author": "Ghost Foundation",
   "homepage": "https://ghost.org",

ghost/core/test/e2e-api/admin/__snapshots__/settings.test.js.snap

@@ -654,7 +654,7 @@ exports[`Settings API Edit Can edit a setting 2: [headers] 1`] = `
 Object {
   "access-control-allow-origin": "http://127.0.0.1:2369",
   "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
-  "content-length": "3632",
+  "content-length": "3653",
   "content-type": "application/json; charset=utf-8",
   "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
   "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,

ghost/email-service/lib/batch-sending-service.js

@@ -35,6 +35,7 @@ class BatchSendingService {
     // Retry database queries happening before sending the email
     #BEFORE_RETRY_CONFIG = {maxRetries: 10, maxTime: 10 * 60 * 1000, sleep: 2000};
     #AFTER_RETRY_CONFIG = {maxRetries: 20, maxTime: 30 * 60 * 1000, sleep: 2000};
+    #MAILGUN_API_RETRY_CONFIG = {sleep: 10 * 1000, maxRetries: 6};
 
     /**
      * @param {Object} dependencies
@@ -61,7 +62,8 @@ class BatchSendingService {
         db,
         sentry,
         BEFORE_RETRY_CONFIG,
-        AFTER_RETRY_CONFIG
+        AFTER_RETRY_CONFIG,
+        MAILGUN_API_RETRY_CONFIG
     }) {
         this.#emailRenderer = emailRenderer;
         this.#sendingService = sendingService;
@@ -77,6 +79,13 @@ class BatchSendingService {
         if (AFTER_RETRY_CONFIG) {
             this.#AFTER_RETRY_CONFIG = AFTER_RETRY_CONFIG;
         }
+        if (MAILGUN_API_RETRY_CONFIG) {
+            this.#MAILGUN_API_RETRY_CONFIG = MAILGUN_API_RETRY_CONFIG;
+        } else {
+            if (process.env.NODE_ENV.startsWith('test')) {
+                this.#MAILGUN_API_RETRY_CONFIG = {maxRetries: 0};
+            }
+        }
     }
 
     #getBeforeRetryConfig(email) {
@@ -393,17 +402,19 @@ class BatchSendingService {
                 {...this.#getBeforeRetryConfig(email), description: `getBatchMembers batch ${originalBatch.id}`}
             );
 
-            const response = await this.#sendingService.send({
+            const response = await this.retryDb(async () => {
-                emailId: email.id,
+                return await this.#sendingService.send({
-                post,
+                    emailId: email.id,
-                newsletter,
+                    post,
-                segment: batch.get('member_segment'),
+                    newsletter,
-                members
+                    segment: batch.get('member_segment'),
-            }, {
+                    members
-                openTrackingEnabled: !!email.get('track_opens'),
+                }, {
-                clickTrackingEnabled: !!email.get('track_clicks'),
+                    openTrackingEnabled: !!email.get('track_opens'),
-                emailBodyCache
+                    clickTrackingEnabled: !!email.get('track_clicks'),
-            });
+                    emailBodyCache
+                });
+            }, {...this.#MAILGUN_API_RETRY_CONFIG, description: `Sending email batch ${originalBatch.id}`});
             succeeded = true;
 
             await this.retryDb(
@@ -420,8 +431,13 @@ class BatchSendingService {
                 {...this.#AFTER_RETRY_CONFIG, description: `save batch ${originalBatch.id} -> submitted`}
             );
         } catch (err) {
-            if (!err.code || err.code !== 'BULK_EMAIL_SEND_FAILED') {
+            if (err.code && err.code === 'BULK_EMAIL_SEND_FAILED') {
-                // BULK_EMAIL_SEND_FAILED are already logged in mailgun-email-provider
+                logging.error(err);
+                if (this.#sentry) {
+                    // Log the original error to Sentry
+                    this.#sentry.captureException(err);
+                }
+            } else {
                 const ghostError = new errors.EmailError({
                     err,
                     code: 'BULK_EMAIL_SEND_FAILED',
@@ -533,17 +549,18 @@ class BatchSendingService {
             return await func();
         } catch (e) {
             const retryCount = (options.retryCount ?? 0);
-            const sleep = (options.sleep ?? 0) * (retryCount + 1);
+            const sleep = (options.sleep ?? 0);
             if (retryCount >= options.maxRetries || (options.stopAfterDate && (new Date(Date.now() + sleep)) > options.stopAfterDate)) {
-                const ghostError = new errors.EmailError({
+                if (retryCount > 0) {
-                    err: e,
+                    const ghostError = new errors.EmailError({
-                    code: 'BULK_EMAIL_DB_RETRY',
+                        err: e,
-                    message: `[BULK_EMAIL_DB_RETRY] ${options.description} - Stopped retrying`,
+                        code: 'BULK_EMAIL_DB_RETRY',
-                    context: e.message
+                        message: `[BULK_EMAIL_DB_RETRY] ${options.description} - Stopped retrying`,
-                });
+                        context: e.message
-
+                    });
-                logging.error(ghostError);
 
+                    logging.error(ghostError);
+                }
                 throw e;
             }
 
@@ -561,7 +578,7 @@ class BatchSendingService {
                     setTimeout(resolve, sleep);
                 });
             }
-            return await this.retryDb(func, {...options, retryCount: retryCount + 1});
+            return await this.retryDb(func, {...options, retryCount: retryCount + 1, sleep: sleep * 2});
         }
     }
 }

ghost/email-service/lib/mailgun-email-provider.js

@@ -165,11 +165,8 @@ class MailgunEmailProvider {
                 });
             }
 
-            logging.error(ghostError);
             debug(`failed to send message (${Date.now() - startTime}ms)`);
 
-            // log error to custom error handler. ex sentry
-            this.#errorHandler(ghostError);
             throw ghostError;
         }
     }

ghost/email-service/test/batch-sending-service.test.js

@@ -661,7 +661,10 @@ describe('Batch Sending Service', function () {
             const findOne = sinon.spy(EmailBatch, 'findOne');
             const service = new BatchSendingService({
                 models: {EmailBatch, EmailRecipient},
-                sendingService
+                sendingService,
+                MAILGUN_API_RETRY_CONFIG: {
+                    sleep: 10, maxRetries: 5
+                }
             });
 
             const result = await service.sendBatch({
@@ -672,8 +675,8 @@ describe('Batch Sending Service', function () {
             });
 
             assert.equal(result, false);
-            sinon.assert.calledOnce(errorLog);
+            sinon.assert.callCount(errorLog, 7);
-            sinon.assert.calledOnce(sendingService.send);
+            sinon.assert.callCount(sendingService.send, 6);
 
             sinon.assert.calledOnce(findOne);
             const batch = await findOne.firstCall.returnValue;
@@ -701,6 +704,9 @@ describe('Batch Sending Service', function () {
                 sendingService,
                 sentry: {
                     captureException
+                },
+                MAILGUN_API_RETRY_CONFIG: {
+                    maxRetries: 0
                 }
             });
 
@@ -748,11 +754,17 @@ describe('Batch Sending Service', function () {
                     code: 'BULK_EMAIL_SEND_FAILED'
                 }))
             };
-
+            const captureException = sinon.stub();
             const findOne = sinon.spy(EmailBatch, 'findOne');
             const service = new BatchSendingService({
                 models: {EmailBatch, EmailRecipient},
-                sendingService
+                sendingService,
+                sentry: {
+                    captureException
+                },
+                MAILGUN_API_RETRY_CONFIG: {
+                    maxRetries: 0
+                }
             });
 
             const result = await service.sendBatch({
@@ -763,8 +775,11 @@ describe('Batch Sending Service', function () {
             });
 
             assert.equal(result, false);
-            sinon.assert.notCalled(errorLog);
+            sinon.assert.calledOnce(errorLog);
             sinon.assert.calledOnce(sendingService.send);
+            sinon.assert.calledOnce(captureException);
+            const sentryExeption = captureException.firstCall.args[0];
+            assert.equal(sentryExeption.message, 'Test error');
 
             sinon.assert.calledOnce(findOne);
             const batch = await findOne.firstCall.returnValue;