From 54637da8683e61dff5b902b8ec92d2dce78558f8 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Mon, 10 Aug 2026 16:00:57 +0100 Subject: [PATCH 1/3] Added latest gift delivery provider outcomes ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions Mailgun telemetry is useful for diagnosis after acceptance, but it must remain separate from the durable sent fact and never trigger another delivery. --- ghost/core/core/boot.js | 11 +++- .../start-gift-email-analytics-job-event.ts | 11 ++++ .../gift-email-analytics-batch-processor.ts | 66 +++++++++++++++++++ .../server/services/email-analytics/index.ts | 37 +++++++++++ .../jobs/email-analytics-job-scheduler.ts | 29 ++++++++ .../jobs/gift-fetch-latest/index.js | 7 ++ .../services/email-analytics/jobs/index.js | 10 +++ .../services/gifts/gift-service-wrapper.js | 3 +- .../email-analytics-job-scheduler.test.ts | 29 +++++++- ...ft-email-analytics-batch-processor.test.ts | 50 ++++++++++++++ 10 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 ghost/core/core/server/services/email-analytics/events/start-gift-email-analytics-job-event.ts create mode 100644 ghost/core/core/server/services/email-analytics/gift-email-analytics-batch-processor.ts create mode 100644 ghost/core/core/server/services/email-analytics/jobs/gift-fetch-latest/index.js create mode 100644 ghost/core/test/unit/server/services/email-analytics/gift-email-analytics-batch-processor.test.ts diff --git a/ghost/core/core/boot.js b/ghost/core/core/boot.js index 7a54eaa613d..7dd9b6a005b 100644 --- a/ghost/core/core/boot.js +++ b/ghost/core/core/boot.js @@ -450,10 +450,17 @@ async function initBackgroundServices({config}) { // Load email analytics recurring jobs if (config.get('backgroundJobs:emailAnalytics')) { const emailAnalyticsJobs = require('./server/services/email-analytics/jobs'); - await Promise.all([ + const analyticsJobs = [ emailAnalyticsJobs.scheduleRecurringNewslettersJob(), emailAnalyticsJobs.scheduleRecurringAutomationsJob() - ]); + ]; + + const labs = require('./shared/labs'); + if (labs.isSet('giftSubCustomization')) { + analyticsJobs.push(emailAnalyticsJobs.scheduleRecurringGiftDeliveriesJob()); + } + + await Promise.all(analyticsJobs); } const updateCheck = require('./server/services/update-check'); diff --git a/ghost/core/core/server/services/email-analytics/events/start-gift-email-analytics-job-event.ts b/ghost/core/core/server/services/email-analytics/events/start-gift-email-analytics-job-event.ts new file mode 100644 index 00000000000..2270cc06bbc --- /dev/null +++ b/ghost/core/core/server/services/email-analytics/events/start-gift-email-analytics-job-event.ts @@ -0,0 +1,11 @@ +export class StartGiftEmailAnalyticsJobEvent { + readonly timestamp: Date; + + constructor(timestamp: Date) { + this.timestamp = timestamp; + } + + static create(timestamp = new Date()) { + return new StartGiftEmailAnalyticsJobEvent(timestamp); + } +} diff --git a/ghost/core/core/server/services/email-analytics/gift-email-analytics-batch-processor.ts b/ghost/core/core/server/services/email-analytics/gift-email-analytics-batch-processor.ts new file mode 100644 index 00000000000..0973fa1dd0b --- /dev/null +++ b/ghost/core/core/server/services/email-analytics/gift-email-analytics-batch-processor.ts @@ -0,0 +1,66 @@ +import type {BatchEventProcessor} from './batch-event-processor'; +import {EventProcessingResult} from './event-processing-result'; + +type GiftService = { + recordDeliveryOutcome(data: { + providerMessageId: string; + outcome: 'delivered' | 'temporary_failed' | 'permanent_failed'; + timestamp: Date; + error: string | null; + }): Promise; +}; + +type EmailAnalyticsEvent = { + type: string; + severity?: string; + providerId: string; + timestamp: Date; + error?: {code?: unknown; message?: unknown; enhancedCode?: unknown} | null; +}; + +const normalizeMessageId = (value: string): string => value.trim().replace(/^<|>$/g, ''); + +export class GiftEmailAnalyticsBatchProcessor implements BatchEventProcessor { + readonly #giftService: GiftService; + + constructor({giftService}: {giftService: GiftService}) { + this.#giftService = giftService; + } + + async processBatch(events: ReadonlyArray, result: EventProcessingResult, fetchData: {lastEventTimestamp?: Date}): Promise { + for (const event of events) { + if (!fetchData.lastEventTimestamp || event.timestamp > fetchData.lastEventTimestamp) { + fetchData.lastEventTimestamp = event.timestamp; + } + + let outcome: 'delivered' | 'temporary_failed' | 'permanent_failed' | null = null; + if (event.type === 'delivered') { + outcome = 'delivered'; + } else if (event.type === 'failed') { + outcome = event.severity === 'temporary' ? 'temporary_failed' : 'permanent_failed'; + } + + if (!outcome) { + result.merge(new EventProcessingResult({unhandled: 1})); + continue; + } + + const updated = await this.#giftService.recordDeliveryOutcome({ + providerMessageId: normalizeMessageId(event.providerId), + outcome, + timestamp: event.timestamp, + error: event.error ? JSON.stringify(event.error) : null + }); + + if (!updated) { + result.merge(new EventProcessingResult({unprocessable: 1})); + } else if (outcome === 'delivered') { + result.merge(new EventProcessingResult({delivered: 1})); + } else if (outcome === 'temporary_failed') { + result.merge(new EventProcessingResult({temporaryFailed: 1})); + } else { + result.merge(new EventProcessingResult({permanentFailed: 1})); + } + } + } +} diff --git a/ghost/core/core/server/services/email-analytics/index.ts b/ghost/core/core/server/services/email-analytics/index.ts index 34da53958d3..0e408fc86d7 100644 --- a/ghost/core/core/server/services/email-analytics/index.ts +++ b/ghost/core/core/server/services/email-analytics/index.ts @@ -24,6 +24,12 @@ import {StartAutomationEmailAnalyticsJobEvent} from './events/start-automation-e import {AUTOMATION_EMAIL_TAG} from '../member-welcome-emails/constants'; import * as automationsApi from '../automations/automations-api'; import {AutomationEmailAnalyticsBatchProcessor} from './automation-email-analytics-batch-processor'; +import {GiftEmailAnalyticsBatchProcessor} from './gift-email-analytics-batch-processor'; +import {StartGiftEmailAnalyticsJobEvent} from './events/start-gift-email-analytics-job-event'; +// @ts-expect-error This CommonJS service wrapper lacks type declarations. +import giftsService from '../gifts'; +// @ts-expect-error This CommonJS service lacks type declarations. +import labs from '../../../shared/labs'; export const newsletters = new EmailAnalyticsServiceWrapper({ logName: 'newsletters' @@ -33,6 +39,10 @@ export const automations = new EmailAnalyticsServiceWrapper({ logName: 'automations', }); +export const gifts = new EmailAnalyticsServiceWrapper({ + logName: 'gifts' +}); + export const init = () => { const newsletterEmailEventProcessor = new EmailEventProcessor({ domainEvents, @@ -106,4 +116,31 @@ export const init = () => { }) ) }); + + if (labs.isSet('giftSubCustomization')) { + gifts.init({ + event: StartGiftEmailAnalyticsJobEvent, + mailgunTags: ['gift-delivery'], + jobNames: { + latestNonOpened: 'email-analytics-gifts-latest-others', + missing: 'email-analytics-gifts-missing', + latestOpened: 'email-analytics-gifts-latest-opened', + scheduled: 'email-analytics-gifts-scheduled' + }, + cursorSeed: { + tableName: 'gift_deliveries', + eventColumns: { + delivered: 'outcome_at', + failed: 'outcome_at' + } + }, + createEventProcessor: () => ( + new GiftEmailAnalyticsBatchProcessor({ + giftService: { + recordDeliveryOutcome: data => giftsService.service.recordDeliveryOutcome(data) + } + }) + ) + }); + } }; diff --git a/ghost/core/core/server/services/email-analytics/jobs/email-analytics-job-scheduler.ts b/ghost/core/core/server/services/email-analytics/jobs/email-analytics-job-scheduler.ts index b0b56cd9650..edd78199825 100644 --- a/ghost/core/core/server/services/email-analytics/jobs/email-analytics-job-scheduler.ts +++ b/ghost/core/core/server/services/email-analytics/jobs/email-analytics-job-scheduler.ts @@ -17,6 +17,9 @@ type Models = { AutomatedEmailRecipient: { query(): ExistingRecipientQuery; }; + GiftDelivery?: { + query(): ExistingRecipientQuery; + }; }; type Config = {get(key: string): unknown}; type JobManager = { @@ -39,6 +42,7 @@ function randomFiveMinuteCron(): string { export class EmailAnalyticsJobScheduler { #hasScheduledNewslettersJob = false; #hasScheduledAutomationsJob = false; + #hasScheduledGiftDeliveriesJob = false; readonly #models: Models; readonly #config: Config; readonly #jobManager: JobManager; @@ -123,4 +127,29 @@ export class EmailAnalyticsJobScheduler { this.#hasScheduledAutomationsJob = true; } + + async scheduleRecurringGiftDeliveriesJob(skipGiftDeliveryCheck: boolean = false): Promise { + if (this.#hasScheduledGiftDeliveriesJob || !this.#isConfigured()) { + return; + } + + const hasGiftDelivery = skipGiftDeliveryCheck || Boolean( + this.#models.GiftDelivery && await this.#models.GiftDelivery + .query() + .where('email_sent_at', '>', moment.utc().subtract(30, 'days').toDate()) + .whereNotNull('email_provider_message_id') + .first('id') + ); + + if (!hasGiftDelivery || this.#hasScheduledGiftDeliveriesJob) { + return; + } + + this.#jobManager.addJob({ + at: randomFiveMinuteCron(), + job: path.resolve(__dirname, 'gift-fetch-latest/index.js'), + name: 'email-analytics-gift-fetch-latest' + }); + this.#hasScheduledGiftDeliveriesJob = true; + } } diff --git a/ghost/core/core/server/services/email-analytics/jobs/gift-fetch-latest/index.js b/ghost/core/core/server/services/email-analytics/jobs/gift-fetch-latest/index.js new file mode 100644 index 00000000000..bdd506e8d4a --- /dev/null +++ b/ghost/core/core/server/services/email-analytics/jobs/gift-fetch-latest/index.js @@ -0,0 +1,7 @@ +const {run} = require('../fetch-latest-job'); +const {StartGiftEmailAnalyticsJobEvent} = require('../../events/start-gift-email-analytics-job-event'); + +run({ + event: StartGiftEmailAnalyticsJobEvent, + logName: 'gifts' +}); diff --git a/ghost/core/core/server/services/email-analytics/jobs/index.js b/ghost/core/core/server/services/email-analytics/jobs/index.js index 485ae9411d7..a8ab9713e70 100644 --- a/ghost/core/core/server/services/email-analytics/jobs/index.js +++ b/ghost/core/core/server/services/email-analytics/jobs/index.js @@ -28,3 +28,13 @@ exports.scheduleRecurringAutomationsJob = async (...args) => { await emailAnalyticsJobScheduler.scheduleRecurringAutomationsJob(...args); } }; + +/** + * @param {Parameters} args + * @returns {Promise} + */ +exports.scheduleRecurringGiftDeliveriesJob = async (...args) => { + if (!process.env.NODE_ENV.startsWith('test')) { + await emailAnalyticsJobScheduler.scheduleRecurringGiftDeliveriesJob(...args); + } +}; diff --git a/ghost/core/core/server/services/gifts/gift-service-wrapper.js b/ghost/core/core/server/services/gifts/gift-service-wrapper.js index 487888aba32..6828bb1a772 100644 --- a/ghost/core/core/server/services/gifts/gift-service-wrapper.js +++ b/ghost/core/core/server/services/gifts/gift-service-wrapper.js @@ -42,6 +42,7 @@ class GiftServiceWrapper { const StartGiftDeliveryFlushEvent = require('./events/start-gift-delivery-flush-event'); const StartGiftCleanupEvent = require('./events/start-gift-cleanup-event'); const jobs = require('./jobs'); + const emailAnalyticsJobs = require('../email-analytics/jobs'); const {GhostMailer} = require('../mail'); const MailgunClient = require('../lib/mailgun-client'); @@ -104,7 +105,7 @@ class GiftServiceWrapper { giftReminderScheduler, giftDeliveryScheduler, giftEmailAnalytics: { - schedule: () => Promise.resolve() + schedule: () => emailAnalyticsJobs.scheduleRecurringGiftDeliveriesJob(true) }, checkoutAdapter, labsService, diff --git a/ghost/core/test/unit/server/services/email-analytics/email-analytics-job-scheduler.test.ts b/ghost/core/test/unit/server/services/email-analytics/email-analytics-job-scheduler.test.ts index 69ce2356d72..b2298728d86 100644 --- a/ghost/core/test/unit/server/services/email-analytics/email-analytics-job-scheduler.test.ts +++ b/ghost/core/test/unit/server/services/email-analytics/email-analytics-job-scheduler.test.ts @@ -28,21 +28,27 @@ function buildScheduler({ emailAnalyticsEnabled = true, backgroundJobEnabled = true, emailCount = 1, - automatedEmailRecipient = null + automatedEmailRecipient = null, + giftDelivery = null }: { emailAnalyticsEnabled?: boolean; backgroundJobEnabled?: boolean; emailCount?: string | number; automatedEmailRecipient?: unknown; + giftDelivery?: unknown; } = {}) { const newsletterQuery = buildNewsletterQuery(emailCount); const automationsQuery = buildAutomationsQuery(automatedEmailRecipient); + const giftQuery = buildAutomationsQuery(giftDelivery); const models = { Email: { where: newsletterQuery.where }, AutomatedEmailRecipient: { query: sinon.stub().returns(automationsQuery) + }, + GiftDelivery: { + query: sinon.stub().returns(giftQuery) } }; const config = { @@ -65,6 +71,7 @@ function buildScheduler({ jobManager, newsletterQuery, automationsQuery, + giftQuery, models }; } @@ -281,4 +288,24 @@ describe('EmailAnalyticsJobScheduler', function () { sinon.assert.notCalled(jobManager.addJob); sinon.assert.calledOnce(automationsQuery.first); }); + + it('adds the existing recurring analytics collector for accepted gift email telemetry', async function () { + const {scheduler, jobManager, giftQuery, models} = buildScheduler({ + emailCount: 0, + giftDelivery: {id: 'gift-id'} + }); + + await scheduler.scheduleRecurringGiftDeliveriesJob(); + + sinon.assert.calledOnceWithMatch(jobManager.addJob, { + job: sinon.match((value: unknown) => ( + typeof value === 'string' && value.endsWith('gift-fetch-latest/index.js') + )), + name: 'email-analytics-gift-fetch-latest' + }); + sinon.assert.calledOnce(models.GiftDelivery.query); + sinon.assert.calledOnceWithExactly(giftQuery.where, 'email_sent_at', '>', sinon.match.date); + sinon.assert.calledOnceWithExactly(giftQuery.whereNotNull, 'email_provider_message_id'); + sinon.assert.calledOnceWithExactly(giftQuery.first, 'id'); + }); }); diff --git a/ghost/core/test/unit/server/services/email-analytics/gift-email-analytics-batch-processor.test.ts b/ghost/core/test/unit/server/services/email-analytics/gift-email-analytics-batch-processor.test.ts new file mode 100644 index 00000000000..7d8d154ec04 --- /dev/null +++ b/ghost/core/test/unit/server/services/email-analytics/gift-email-analytics-batch-processor.test.ts @@ -0,0 +1,50 @@ +import assert from 'node:assert/strict'; +import sinon from 'sinon'; +import {GiftEmailAnalyticsBatchProcessor} from '../../../../../core/server/services/email-analytics/gift-email-analytics-batch-processor'; +import {EventProcessingResult} from '../../../../../core/server/services/email-analytics/event-processing-result'; + +describe('GiftEmailAnalyticsBatchProcessor', function () { + it('maps Mailgun delivery and failure events to latest gift outcomes without opens', async function () { + const giftService = {recordDeliveryOutcome: sinon.stub().resolves(true)}; + const processor = new GiftEmailAnalyticsBatchProcessor({giftService}); + const result = new EventProcessingResult(); + const fetchData: {lastEventTimestamp?: Date} = {}; + const deliveredAt = new Date('2026-08-05T12:00:00.000Z'); + const failedAt = new Date('2026-08-05T12:10:00.000Z'); + + await processor.processBatch([ + {type: 'delivered', providerId: '', timestamp: deliveredAt}, + {type: 'failed', severity: 'temporary', providerId: 'provider-123', timestamp: failedAt, error: {code: 421, message: 'try later'}}, + {type: 'opened', providerId: 'provider-123', timestamp: new Date('2026-08-05T12:20:00.000Z')} + ], result, fetchData); + + sinon.assert.calledWithExactly(giftService.recordDeliveryOutcome, sinon.match({ + providerMessageId: 'provider-123', + outcome: 'delivered', + timestamp: deliveredAt, + error: null + })); + assert.deepEqual(giftService.recordDeliveryOutcome.secondCall.firstArg, { + providerMessageId: 'provider-123', + outcome: 'temporary_failed', + timestamp: failedAt, + error: JSON.stringify({code: 421, message: 'try later'}) + }); + assert.equal(result.delivered, 1); + assert.equal(result.temporaryFailed, 1); + assert.equal(result.unhandled, 1); + }); + + it('marks events for unknown message IDs unprocessable', async function () { + const giftService = {recordDeliveryOutcome: sinon.stub().resolves(false)}; + const processor = new GiftEmailAnalyticsBatchProcessor({giftService}); + const result = new EventProcessingResult(); + + await processor.processBatch([ + {type: 'failed', severity: 'permanent', providerId: 'unknown', timestamp: new Date()} + ], result, {}); + + assert.equal(result.unprocessable, 1); + assert.equal(result.permanentFailed, 0); + }); +}); From 8bcc704ef7cf71f942a2653f868ffe926d08eacf Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 13 Aug 2026 10:16:59 +0100 Subject: [PATCH 2/3] Fixed gift delivery outcome collection ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions Gift outcomes need the transactional Mailgun account and full provider timestamp ordering so telemetry remains authoritative across split email configurations and replayed events. --- ...-add-gift-delivery-outcome-milliseconds.js | 8 ++ ghost/core/core/server/data/schema/schema.js | 1 + .../core/core/server/models/gift-delivery.js | 4 +- .../email-analytics-service-wrapper.js | 4 +- .../email-analytics/fetch-mailgun-events.js | 31 +++++- .../server/services/email-analytics/index.ts | 3 + .../gift-delivery-bookshelf-repository.ts | 54 ++++++--- .../services/gifts/gift-delivery-codec.ts | 13 ++- .../services/gifts/gift-delivery-schema.ts | 10 +- .../server/services/lib/mailgun-client.js | 10 +- ...gift-delivery-bookshelf-repository.test.ts | 105 +++++++++++++----- .../unit/server/data/schema/integrity.test.js | 2 +- .../fetch-mailgun-events.test.js | 34 +++++- .../services/gifts/gift-delivery.test.ts | 3 +- .../services/lib/mailgun-client.test.js | 37 ++++++ 15 files changed, 266 insertions(+), 53 deletions(-) create mode 100644 ghost/core/core/server/data/migrations/versions/6.58/2026-08-13-09-04-19-add-gift-delivery-outcome-milliseconds.js diff --git a/ghost/core/core/server/data/migrations/versions/6.58/2026-08-13-09-04-19-add-gift-delivery-outcome-milliseconds.js b/ghost/core/core/server/data/migrations/versions/6.58/2026-08-13-09-04-19-add-gift-delivery-outcome-milliseconds.js new file mode 100644 index 00000000000..1f7c56943dd --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/6.58/2026-08-13-09-04-19-add-gift-delivery-outcome-milliseconds.js @@ -0,0 +1,8 @@ +const {createAddColumnMigration} = require('../../utils'); + +module.exports = createAddColumnMigration('gift_deliveries', 'outcome_at_ms', { + type: 'integer', + nullable: false, + unsigned: true, + defaultTo: 0 +}); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index fd4c576d71b..dc09403a200 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -1464,6 +1464,7 @@ module.exports = { } }, outcome_at: {type: 'dateTime', nullable: true}, + outcome_at_ms: {type: 'integer', nullable: false, unsigned: true, defaultTo: 0}, outcome_error: {type: 'text', maxlength: 65535, nullable: true}, '@@INDEXES@@': [ {columns: ['email_provider_message_id'], length: 31}, diff --git a/ghost/core/core/server/models/gift-delivery.js b/ghost/core/core/server/models/gift-delivery.js index 1d82e0401f0..8b7b3de1294 100644 --- a/ghost/core/core/server/models/gift-delivery.js +++ b/ghost/core/core/server/models/gift-delivery.js @@ -7,7 +7,9 @@ const GiftDelivery = ghostBookshelf.Model.extend({ defaults: { status: 'pending', - outcome: 'unknown' + attempts: 0, + outcome: 'unknown', + outcome_at_ms: 0 }, gift() { diff --git a/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js b/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js index a735bd22497..bf1aa325cb6 100644 --- a/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js +++ b/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js @@ -29,6 +29,7 @@ class EmailAnalyticsServiceWrapper { * @param {object} options * @param {Parameters[0]} options.event * @param {string[]} options.mailgunTags + * @param {() => {apiKey: string, domain: string, baseUrl: string}|null} [options.getMailgunConfig] * @param {JobNames} options.jobNames * @param {CursorSeed} options.cursorSeed * @param {() => BatchEventProcessor} options.createEventProcessor @@ -37,6 +38,7 @@ class EmailAnalyticsServiceWrapper { init({ event, mailgunTags, + getMailgunConfig, jobNames, cursorSeed, createEventProcessor, @@ -52,7 +54,7 @@ class EmailAnalyticsServiceWrapper { const {queries} = require('./lib/queries'); this.service = new EmailAnalyticsService({ - fetchEvents: (options) => fetchMailgunEvents({...options, config, settings, tags: mailgunTags}), + fetchEvents: (options) => fetchMailgunEvents({...options, config, settings, tags: mailgunTags, getMailgunConfig}), queries, prometheusClient, jobNames, diff --git a/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.js b/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.js index 82ac305acb3..d2cf6bcbd68 100644 --- a/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.js +++ b/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.js @@ -10,14 +10,15 @@ const PAGE_LIMIT = 300; * @param {object} options.config * @param {object} options.settings * @param {string[]} options.tags + * @param {() => {apiKey: string, domain: string, baseUrl: string}|null} [options.getMailgunConfig] * @param {Function} options.batchHandler * @param {number} [options.maxEvents] Not a strict maximum. We stop fetching after we reached the maximum AND received at least one event after begin (not equal) to prevent deadlocks. * @param {Date} [options.begin] * @param {Date} [options.end] * @param {string[]} [options.events] */ -async function fetchMailgunEvents({config, settings, tags, batchHandler, maxEvents, begin, end, events}) { - const mailgunClient = new MailgunClient({config, settings}); +async function fetchMailgunEvents({config, settings, tags, getMailgunConfig, batchHandler, maxEvents, begin, end, events}) { + const mailgunClient = new MailgunClient({config, settings, getMailgunConfig}); const mailgunOptions = { limit: PAGE_LIMIT, event: events ? events.join(' OR ') : DEFAULT_EVENT_FILTER, @@ -29,4 +30,30 @@ async function fetchMailgunEvents({config, settings, tags, batchHandler, maxEven return await mailgunClient.fetchEvents(mailgunOptions, batchHandler, {maxEvents}); } +function getTransactionalMailgunConfig(config) { + const mail = config.get('mail'); + if (mail?.transport?.toLowerCase() !== 'mailgun') { + return null; + } + + const options = mail.options ?? {}; + const apiKey = options.auth?.api_key ?? options.auth?.apiKey; + const domain = options.auth?.domain; + if (!apiKey || !domain) { + return null; + } + + let baseUrl = options.url; + if (!baseUrl) { + const protocolOption = options.protocol ?? 'https:'; + const protocol = protocolOption.endsWith(':') ? protocolOption : `${protocolOption}:`; + const host = options.host ?? 'api.mailgun.net'; + const port = options.port ? `:${options.port}` : ''; + baseUrl = `${protocol}//${host}${port}`; + } + + return {apiKey, domain, baseUrl}; +} + module.exports.fetchMailgunEvents = fetchMailgunEvents; +module.exports.getTransactionalMailgunConfig = getTransactionalMailgunConfig; diff --git a/ghost/core/core/server/services/email-analytics/index.ts b/ghost/core/core/server/services/email-analytics/index.ts index 0e408fc86d7..b3074d62941 100644 --- a/ghost/core/core/server/services/email-analytics/index.ts +++ b/ghost/core/core/server/services/email-analytics/index.ts @@ -26,6 +26,8 @@ import * as automationsApi from '../automations/automations-api'; import {AutomationEmailAnalyticsBatchProcessor} from './automation-email-analytics-batch-processor'; import {GiftEmailAnalyticsBatchProcessor} from './gift-email-analytics-batch-processor'; import {StartGiftEmailAnalyticsJobEvent} from './events/start-gift-email-analytics-job-event'; +// @ts-expect-error This CommonJS helper lacks type declarations. +import {getTransactionalMailgunConfig} from './fetch-mailgun-events'; // @ts-expect-error This CommonJS service wrapper lacks type declarations. import giftsService from '../gifts'; // @ts-expect-error This CommonJS service lacks type declarations. @@ -121,6 +123,7 @@ export const init = () => { gifts.init({ event: StartGiftEmailAnalyticsJobEvent, mailgunTags: ['gift-delivery'], + getMailgunConfig: () => getTransactionalMailgunConfig(config), jobNames: { latestNonOpened: 'email-analytics-gifts-latest-others', missing: 'email-analytics-gifts-missing', diff --git a/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts b/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts index c9b4ce5cfb4..fe05eb188f3 100644 --- a/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts +++ b/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts @@ -16,8 +16,9 @@ export interface GiftDeliveryRepository { findDue(now: Date, limit: number): Promise; findPending(): Promise; countStuck(before: Date): Promise; - tryStartDelivery(id: string, now: Date): Promise; + tryStartAttempt(id: string, now: Date, maxAttempts: number): Promise; markSent(id: string, sentAt: Date, providerMessageId: string | null): Promise; + markForRetry(id: string, nextAttemptAt: Date): Promise; markFailed(id: string): Promise; markCancelled(id: string): Promise; cancelPendingForGift(token: string, options?: RepositoryTransactionOptions): Promise; @@ -72,7 +73,10 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { .where('gift.status', 'purchased') .whereRaw('COALESCE(gift.available_at, gift.purchased_at) <= ?', [dueAt]) .where('delivery.status', 'pending') - .orderByRaw('COALESCE(gift.available_at, gift.purchased_at) ASC') + .where((builder) => { + builder.whereNull('delivery.attempt_at').orWhere('delivery.attempt_at', '<=', dueAt); + }) + .orderByRaw('COALESCE(delivery.attempt_at, gift.available_at, gift.purchased_at) ASC') .limit(limit) .select('delivery.*', 'gift.available_at', 'gift.purchased_at'); @@ -89,7 +93,7 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { .innerJoin('gifts as gift', 'gift.id', 'delivery.gift_id') .where('gift.status', 'purchased') .where('delivery.status', 'pending') - .orderByRaw('COALESCE(gift.available_at, gift.purchased_at) ASC') + .orderByRaw('COALESCE(delivery.attempt_at, gift.available_at, gift.purchased_at) ASC') .select('delivery.*', 'gift.available_at', 'gift.purchased_at'); return rows.map(row => ({ @@ -105,7 +109,7 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { .innerJoin('gifts as gift', 'gift.id', 'delivery.gift_id') .where('gift.status', 'purchased') .where('delivery.status', 'sending') - .where('delivery.started_at', '<=', toDatabaseDate(before)) + .where('delivery.attempt_at', '<=', toDatabaseDate(before)) .count({count: 'delivery.id'}) .first(); @@ -113,21 +117,26 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { }); } - async tryStartDelivery(id: string, now: Date): Promise { + async tryStartAttempt(id: string, now: Date, maxAttempts: number): Promise { return this.transaction(async (transacting) => { - const startedAt = toDatabaseDate(now); + const claimAt = toDatabaseDate(now); const eligibleGifts = transacting('gifts') .select('id') .where('status', 'purchased') - .whereRaw('COALESCE(available_at, purchased_at) <= ?', [startedAt]); + .whereRaw('COALESCE(available_at, purchased_at) <= ?', [claimAt]); const updated = await transacting('gift_deliveries') .where({id, status: 'pending'}) .whereIn('gift_id', eligibleGifts) + .where('attempts', '<', maxAttempts) + .where((builder) => { + builder.whereNull('attempt_at').orWhere('attempt_at', '<=', claimAt); + }) .update({ status: 'sending', - started_at: startedAt - }); + attempt_at: claimAt + }) + .increment('attempts', 1); if (updated !== 1) { return null; @@ -142,21 +151,28 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { status: 'sent', email_sent_at: toDatabaseDate(sentAt), email_provider_message_id: providerMessageId, - started_at: null + attempt_at: null + }); + } + + async markForRetry(id: string, nextAttemptAt: Date): Promise { + return this.updateState(id, 'sending', { + status: 'pending', + attempt_at: toDatabaseDate(nextAttemptAt) }); } async markFailed(id: string): Promise { return this.updateState(id, 'sending', { status: 'failed', - started_at: null + attempt_at: null }); } async markCancelled(id: string): Promise { return this.updateState(id, 'sending', { status: 'cancelled', - started_at: null + attempt_at: null }); } @@ -166,7 +182,7 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { const updated = await transacting('gift_deliveries') .where({status: 'pending'}) .whereIn('gift_id', gift) - .update({status: 'cancelled', started_at: null}); + .update({status: 'cancelled', attempt_at: null}); return updated === 1; }; @@ -177,14 +193,23 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { async recordOutcome({providerMessageId, outcome, timestamp, error}: {providerMessageId: string; outcome: GiftDeliveryOutcome; timestamp: Date; error: string | null}): Promise { return this.transaction(async (transacting) => { const outcomeAt = toDatabaseDate(timestamp); + const outcomeAtMilliseconds = timestamp.getUTCMilliseconds(); const updated = await transacting('gift_deliveries') .where({email_provider_message_id: providerMessageId}) .where((builder) => { - builder.whereNull('outcome_at').orWhere('outcome_at', '<', outcomeAt); + builder + .whereNull('outcome_at') + .orWhere('outcome_at', '<', outcomeAt) + .orWhere((sameSecond) => { + sameSecond + .where('outcome_at', '=', outcomeAt) + .where('outcome_at_ms', '<', outcomeAtMilliseconds); + }); }) .update({ outcome, outcome_at: outcomeAt, + outcome_at_ms: outcomeAtMilliseconds, outcome_error: error }); @@ -209,5 +234,4 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { return updated === 1; }); } - } diff --git a/ghost/core/core/server/services/gifts/gift-delivery-codec.ts b/ghost/core/core/server/services/gifts/gift-delivery-codec.ts index 361c9dae83b..d94d3e6e12b 100644 --- a/ghost/core/core/server/services/gifts/gift-delivery-codec.ts +++ b/ghost/core/core/server/services/gifts/gift-delivery-codec.ts @@ -2,6 +2,16 @@ import {z} from 'zod'; import {GiftDelivery} from './gift-delivery'; import {DbGiftDelivery} from './gift-delivery-schema'; +function withMilliseconds(date: Date | null, milliseconds: number): Date | null { + if (!date) { + return date; + } + + const preciseDate = new Date(date); + preciseDate.setUTCMilliseconds(milliseconds); + return preciseDate; +} + export const giftDeliveryCodec = z.codec(DbGiftDelivery, z.instanceof(GiftDelivery), { decode: row => new GiftDelivery({ id: row.id, @@ -12,7 +22,7 @@ export const giftDeliveryCodec = z.codec(DbGiftDelivery, z.instanceof(GiftDelive emailSentAt: row.email_sent_at, emailProviderMessageId: row.email_provider_message_id, outcome: row.outcome, - outcomeAt: row.outcome_at, + outcomeAt: withMilliseconds(row.outcome_at, row.outcome_at_ms), outcomeError: row.outcome_error }), encode: delivery => ({ @@ -25,6 +35,7 @@ export const giftDeliveryCodec = z.codec(DbGiftDelivery, z.instanceof(GiftDelive email_provider_message_id: delivery.emailProviderMessageId, outcome: delivery.outcome, outcome_at: delivery.outcomeAt, + outcome_at_ms: delivery.outcomeAt?.getUTCMilliseconds() ?? 0, outcome_error: delivery.outcomeError }) }); diff --git a/ghost/core/core/server/services/gifts/gift-delivery-schema.ts b/ghost/core/core/server/services/gifts/gift-delivery-schema.ts index 45279cfd9d4..51f26b5079d 100644 --- a/ghost/core/core/server/services/gifts/gift-delivery-schema.ts +++ b/ghost/core/core/server/services/gifts/gift-delivery-schema.ts @@ -9,7 +9,7 @@ export const GiftDeliveryOutcomeSchema = z.enum(['unknown', 'delivered', 'tempor export type GiftDeliveryStatus = z.infer; export type GiftDeliveryOutcome = z.infer; -export const DbGiftDelivery = z.object({ +const DbGiftDeliveryData = z.object({ id: z.string(), gift_id: z.string(), recipient_email: z.string().email(), @@ -22,7 +22,11 @@ export const DbGiftDelivery = z.object({ outcome_error: z.string().nullable().default(null) }); -type GiftDeliveryInputRow = z.input; +export const DbGiftDelivery = DbGiftDeliveryData.extend({ + outcome_at_ms: z.number().int().min(0).max(999).default(0) +}); + +type GiftDeliveryInputRow = z.input; export type GiftDeliveryRow = z.output; -export type GiftDeliveryData = CamelKeys; +export type GiftDeliveryData = CamelKeys>; export type GiftDeliveryDataInput = SetOptional>>; diff --git a/ghost/core/core/server/services/lib/mailgun-client.js b/ghost/core/core/server/services/lib/mailgun-client.js index 917590d295b..5682efbb43a 100644 --- a/ghost/core/core/server/services/lib/mailgun-client.js +++ b/ghost/core/core/server/services/lib/mailgun-client.js @@ -9,10 +9,12 @@ const DEFAULT_BATCH_SIZE = 1000; module.exports = class MailgunClient { #config; #settings; + #getMailgunConfig; - constructor({config, settings}) { + constructor({config, settings, getMailgunConfig}) { this.#config = config; this.#settings = settings; + this.#getMailgunConfig = getMailgunConfig; } /** @@ -195,7 +197,7 @@ module.exports = class MailgunClient { #getDomainsToFetch(mailgunConfig) { const domains = [mailgunConfig.domain]; - const fallbackDomain = this.#config.get('hostSettings:managedEmail:fallbackDomain'); + const fallbackDomain = this.#getMailgunConfig ? null : this.#config.get('hostSettings:managedEmail:fallbackDomain'); if (fallbackDomain && fallbackDomain !== mailgunConfig.domain) { domains.push(fallbackDomain); logging.info(`[MailgunClient] Domain warming enabled, fetching from both primary (${mailgunConfig.domain}) and fallback (${fallbackDomain}) domains`); @@ -331,6 +333,10 @@ module.exports = class MailgunClient { } #getConfig() { + if (this.#getMailgunConfig) { + return this.#getMailgunConfig(); + } + const bulkEmailConfig = this.#config.get('bulkEmail'); const bulkEmailSetting = { apiKey: this.#settings.get('mailgun_api_key'), diff --git a/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts b/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts index bbf18b7384b..bce45eea63a 100644 --- a/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts +++ b/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts @@ -28,17 +28,17 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { async function createPendingEmailGift({ availableAt, - startedAt = null, + attemptAt = null, giftStatus = 'purchased' }: { availableAt: Date; - startedAt?: Date | null; + attemptAt?: Date | null; giftStatus?: string; }) { giftSequence += 1; const now = new Date(); const gift = await models.Gift.add({ - token: `delivery-start-test-token-${giftSequence}`, + token: `delivery-attempt-test-token-${giftSequence}`, buyer_email: `buyer-${giftSequence}@example.com`, buyer_member_id: null, buyer_name: 'Gift Buyer', @@ -67,7 +67,8 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { gift_id: gift.id, recipient_email: `recipient-${giftSequence}@example.com`, status: 'pending', - started_at: startedAt, + attempts: 0, + attempt_at: attemptAt, email_sent_at: null, email_provider_message_id: null, outcome: 'unknown', @@ -109,34 +110,42 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { assert.equal(await deliveryRepository.getByGiftId(gift.id), null); }); - it('allows exactly one concurrent caller to start a due delivery', async function () { - const startedAt = new Date(); - startedAt.setMilliseconds(0); + it('allows exactly one concurrent caller to start a due delivery attempt', async function () { + const attemptAt = new Date(); + attemptAt.setMilliseconds(0); const {delivery} = await createPendingEmailGift({ - availableAt: new Date(startedAt.getTime() - 60_000) + availableAt: new Date(attemptAt.getTime() - 60_000), + attemptAt: new Date(attemptAt.getTime() - 30_000) }); - const starts = await Promise.all([ - deliveryRepository.tryStartDelivery(delivery.id, startedAt), - deliveryRepository.tryStartDelivery(delivery.id, startedAt) + const attempts = await Promise.all([ + deliveryRepository.tryStartAttempt(delivery.id, attemptAt, 10), + deliveryRepository.tryStartAttempt(delivery.id, attemptAt, 10) ]); const reloaded = await deliveryRepository.getById(delivery.id); - assert.equal(starts.filter(Boolean).length, 1); - assert.equal(starts.filter(start => start === null).length, 1); + assert.equal(attempts.filter(Boolean).length, 1); + assert.equal(attempts.filter(attempt => attempt === null).length, 1); assert.equal(reloaded?.status, 'sending'); - assert.equal(reloaded?.startedAt?.toISOString(), startedAt.toISOString()); + assert.equal(reloaded?.attempts, 1); + assert.equal(reloaded?.attemptAt?.toISOString(), attemptAt.toISOString()); }); - it('does not start a delivery before gift availability', async function () { - const startedAt = new Date(); - startedAt.setMilliseconds(0); + it('does not start an attempt before gift availability or the retry time', async function () { + const attemptAt = new Date(); + attemptAt.setMilliseconds(0); const futureAvailability = await createPendingEmailGift({ - availableAt: new Date(startedAt.getTime() + 60_000) + availableAt: new Date(attemptAt.getTime() + 60_000) + }); + const futureRetry = await createPendingEmailGift({ + availableAt: new Date(attemptAt.getTime() - 60_000), + attemptAt: new Date(attemptAt.getTime() + 60_000) }); - assert.equal(await deliveryRepository.tryStartDelivery(futureAvailability.delivery.id, startedAt), null); - assert.equal((await deliveryRepository.getById(futureAvailability.delivery.id))?.status, 'pending'); + assert.equal(await deliveryRepository.tryStartAttempt(futureAvailability.delivery.id, attemptAt, 10), null); + assert.equal(await deliveryRepository.tryStartAttempt(futureRetry.delivery.id, attemptAt, 10), null); + assert.equal((await deliveryRepository.getById(futureAvailability.delivery.id))?.attempts, 0); + assert.equal((await deliveryRepository.getById(futureRetry.delivery.id))?.attempts, 0); }); it('finds only the oldest due deliveries within the requested batch size', async function () { @@ -151,27 +160,44 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { await createPendingEmailGift({ availableAt: new Date(now.getTime() + 60_000) }); + await createPendingEmailGift({ + availableAt: new Date(now.getTime() - 120_000), + attemptAt: new Date(now.getTime() + 60_000) + }); + const due = await deliveryRepository.findDue(now, 1); assert.equal(due.length, 1); assert.equal(due[0]?.delivery.id, oldestDue.delivery.id); }); - it('does not complete a delivery that is not sending', async function () { + it('does not start an attempt once the attempt cap is reached', async function () { + const attemptAt = new Date(); + const {delivery} = await createPendingEmailGift({ + availableAt: new Date(attemptAt.getTime() - 60_000) + }); + await delivery.save({attempts: 10}, {patch: true}); + + assert.equal(await deliveryRepository.tryStartAttempt(delivery.id, attemptAt, 10), null); + assert.equal((await deliveryRepository.getById(delivery.id))?.attempts, 10); + }); + + it('does not complete or retry a delivery that is not sending', async function () { const {delivery} = await createPendingEmailGift({availableAt: new Date()}); assert.equal(await deliveryRepository.markSent(delivery.id, new Date(), 'provider-1'), false); + assert.equal(await deliveryRepository.markForRetry(delivery.id, new Date()), false); assert.equal((await deliveryRepository.getById(delivery.id))?.status, 'pending'); }); - it('does not start a delivery when the parent gift is no longer purchased', async function () { - const startedAt = new Date(); + it('does not start an attempt when the parent gift is no longer purchased', async function () { + const attemptAt = new Date(); const {delivery} = await createPendingEmailGift({ - availableAt: new Date(startedAt.getTime() - 60_000), + availableAt: new Date(attemptAt.getTime() - 60_000), giftStatus: 'refunded' }); - assert.equal(await deliveryRepository.tryStartDelivery(delivery.id, startedAt), null); + assert.equal(await deliveryRepository.tryStartAttempt(delivery.id, attemptAt, 10), null); }); it('cancels a pending delivery by gift token', async function () { @@ -210,4 +236,33 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { assert.equal(reloaded?.outcomeError, null); }); + it('orders provider outcomes within the same second', async function () { + const {delivery} = await createPendingEmailGift({availableAt: new Date()}); + await delivery.save({ + email_provider_message_id: 'provider-123', + outcome: 'temporary_failed', + outcome_at: new Date('2026-08-11T10:00:00.000Z'), + outcome_at_ms: 900, + outcome_error: 'temporary rejection' + }, {patch: true}); + + assert.equal(await deliveryRepository.recordOutcome({ + providerMessageId: 'provider-123', + outcome: 'permanent_failed', + timestamp: new Date('2026-08-11T10:00:00.100Z'), + error: 'older rejection' + }), false); + + assert.equal(await deliveryRepository.recordOutcome({ + providerMessageId: 'provider-123', + outcome: 'delivered', + timestamp: new Date('2026-08-11T10:00:00.950Z'), + error: null + }), true); + + const reloaded = await deliveryRepository.getById(delivery.id); + assert.equal(reloaded?.outcome, 'delivered'); + assert.equal(reloaded?.outcomeAt?.toISOString(), '2026-08-11T10:00:00.950Z'); + assert.equal(reloaded?.outcomeError, null); + }); }); diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index 72a06ec6fcc..fdebf2c3b92 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -35,7 +35,7 @@ const parseYaml = require('../../../../../core/server/services/route-settings/ya */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = '738dec6a7263d292731a49b0fa5dfdff'; + const currentSchemaHash = 'ecce293efc721e93b51b66303a3a472e'; const currentFixturesHash = 'a268b8ff06b240df97eb6060a2a478f5'; const currentSettingsHash = '8650db85b9a61afe4797ad6333066c62'; const currentRoutesHash = 'd8c25fa01bf6d22a2bcb05ba0de70dc1'; diff --git a/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.js b/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.js index f7c9c24d610..795ba82fd79 100644 --- a/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.js +++ b/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.js @@ -1,7 +1,8 @@ +const assert = require('node:assert/strict'); const sinon = require('sinon'); const MailgunClient = require('../../../../../core/server/services/lib/mailgun-client'); -const {fetchMailgunEvents} = require('../../../../../core/server/services/email-analytics/fetch-mailgun-events'); +const {fetchMailgunEvents, getTransactionalMailgunConfig} = require('../../../../../core/server/services/email-analytics/fetch-mailgun-events'); const DEFAULT_TAGS = ['bulk-email']; const LATEST_TIMESTAMP = new Date('Thu Feb 25 2021 12:00:00 GMT+0000'); @@ -96,4 +97,35 @@ describe('fetchMailgunEvents', function () { event: 'delivered' }, batchHandler, {maxEvents: undefined}); }); + + it('maps the transactional Mailgun transport configuration', function () { + const transactionalConfig = { + get: sinon.stub().withArgs('mail').returns({ + transport: 'Mailgun', + options: { + auth: { + api_key: 'apiKey', + domain: 'transactional.example.com' + }, + host: 'api.eu.mailgun.net' + } + }) + }; + + assert.deepEqual(getTransactionalMailgunConfig(transactionalConfig), { + apiKey: 'apiKey', + domain: 'transactional.example.com', + baseUrl: 'https://api.eu.mailgun.net' + }); + }); + + it('does not fall back to bulk Mailgun configuration for other transactional transports', function () { + const transactionalConfig = { + get: sinon.stub().withArgs('mail').returns({ + transport: 'SMTP' + }) + }; + + assert.equal(getTransactionalMailgunConfig(transactionalConfig), null); + }); }); diff --git a/ghost/core/test/unit/server/services/gifts/gift-delivery.test.ts b/ghost/core/test/unit/server/services/gifts/gift-delivery.test.ts index b6ce66a60b9..4db8d67b8c9 100644 --- a/ghost/core/test/unit/server/services/gifts/gift-delivery.test.ts +++ b/ghost/core/test/unit/server/services/gifts/gift-delivery.test.ts @@ -29,6 +29,7 @@ describe('GiftDelivery', function () { email_provider_message_id: 'provider-123', outcome: 'delivered', outcome_at: '2026-08-11T11:01:00.000Z', + outcome_at_ms: 123, outcome_error: null } as const; @@ -39,7 +40,7 @@ describe('GiftDelivery', function () { assert.deepEqual(encodeGiftDelivery(delivery), { ...row, email_sent_at: new Date(row.email_sent_at), - outcome_at: new Date(row.outcome_at) + outcome_at: new Date('2026-08-11T11:01:00.123Z') }); }); diff --git a/ghost/core/test/unit/server/services/lib/mailgun-client.test.js b/ghost/core/test/unit/server/services/lib/mailgun-client.test.js index c5202ecef58..a76431c2a2a 100644 --- a/ghost/core/test/unit/server/services/lib/mailgun-client.test.js +++ b/ghost/core/test/unit/server/services/lib/mailgun-client.test.js @@ -165,6 +165,43 @@ describe('MailgunClient', function () { assert.equal(mailgunClient.isConfigured(), false); }); + it('uses an explicit Mailgun configuration source without the managed bulk fallback domain', async function () { + const configStub = sinon.stub(config, 'get'); + configStub.withArgs('bulkEmail').returns({ + mailgun: { + apiKey: 'bulk-api-key', + domain: 'bulk.example.com', + baseUrl: 'https://api.mailgun.net' + } + }); + configStub.withArgs('hostSettings:managedEmail:fallbackDomain').returns('fallback.example.com'); + const getMailgunConfig = sinon.stub().returns({ + apiKey: 'apiKey', + domain: 'transactional.example.com', + baseUrl: 'https://api.mailgun.net' + }); + const transactionalApiMock = nock('https://api.mailgun.net') + .get('/v3/transactional.example.com/events') + .query(MAILGUN_OPTIONS) + .replyWithFile(200, `${__dirname}/fixtures/empty.json`, { + 'Content-Type': 'application/json' + }); + const fallbackApiMock = nock('https://api.mailgun.net') + .get('/v3/fallback.example.com/events') + .query(MAILGUN_OPTIONS) + .replyWithFile(200, `${__dirname}/fixtures/empty.json`, { + 'Content-Type': 'application/json' + }); + + const mailgunClient = new MailgunClient({config, settings, getMailgunConfig}); + await mailgunClient.fetchEvents(MAILGUN_OPTIONS, () => {}); + + assert.equal(transactionalApiMock.isDone(), true); + assert.equal(fallbackApiMock.isDone(), false); + sinon.assert.neverCalledWith(configStub, 'bulkEmail'); + sinon.assert.neverCalledWith(configStub, 'hostSettings:managedEmail:fallbackDomain'); + }); + it('respects changes in settings', async function () { const settingsStub = sinon.stub(settings, 'get'); settingsStub.withArgs('mailgun_api_key').returns('settingsApiKey'); From 75c8ec8c4169290fd531966247f89b8ba6eaeed7 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 13 Aug 2026 11:47:28 +0100 Subject: [PATCH 3/3] Simplified gift delivery outcome handling ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions Only temporary and permanent provider failures affect product behavior: permanent failures send one best-effort buyer notice, while outcome storage keeps second-level ordering and bulk Mailgun analytics. --- ...-add-gift-delivery-outcome-milliseconds.js | 8 -- ghost/core/core/server/data/schema/schema.js | 1 - .../core/core/server/models/gift-delivery.js | 4 +- .../email-analytics-service-wrapper.js | 4 +- .../email-analytics/fetch-mailgun-events.js | 31 +---- .../server/services/email-analytics/index.ts | 3 - .../core/core/server/services/gifts/README.md | 8 +- .../gift-delivery-bookshelf-repository.ts | 61 ++++------ .../services/gifts/gift-delivery-codec.ts | 13 +-- .../services/gifts/gift-delivery-schema.ts | 10 +- .../server/services/gifts/gift-service.ts | 47 +++++++- .../server/services/lib/mailgun-client.js | 10 +- ...gift-delivery-bookshelf-repository.test.ts | 106 +++++------------- .../unit/server/data/schema/integrity.test.js | 2 +- .../fetch-mailgun-events.test.js | 34 +----- .../services/gifts/gift-delivery.test.ts | 3 +- .../services/gifts/gift-service.test.ts | 76 ++++++++++++- .../services/lib/mailgun-client.test.js | 37 ------ 18 files changed, 188 insertions(+), 270 deletions(-) delete mode 100644 ghost/core/core/server/data/migrations/versions/6.58/2026-08-13-09-04-19-add-gift-delivery-outcome-milliseconds.js diff --git a/ghost/core/core/server/data/migrations/versions/6.58/2026-08-13-09-04-19-add-gift-delivery-outcome-milliseconds.js b/ghost/core/core/server/data/migrations/versions/6.58/2026-08-13-09-04-19-add-gift-delivery-outcome-milliseconds.js deleted file mode 100644 index 1f7c56943dd..00000000000 --- a/ghost/core/core/server/data/migrations/versions/6.58/2026-08-13-09-04-19-add-gift-delivery-outcome-milliseconds.js +++ /dev/null @@ -1,8 +0,0 @@ -const {createAddColumnMigration} = require('../../utils'); - -module.exports = createAddColumnMigration('gift_deliveries', 'outcome_at_ms', { - type: 'integer', - nullable: false, - unsigned: true, - defaultTo: 0 -}); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index dc09403a200..fd4c576d71b 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -1464,7 +1464,6 @@ module.exports = { } }, outcome_at: {type: 'dateTime', nullable: true}, - outcome_at_ms: {type: 'integer', nullable: false, unsigned: true, defaultTo: 0}, outcome_error: {type: 'text', maxlength: 65535, nullable: true}, '@@INDEXES@@': [ {columns: ['email_provider_message_id'], length: 31}, diff --git a/ghost/core/core/server/models/gift-delivery.js b/ghost/core/core/server/models/gift-delivery.js index 8b7b3de1294..1d82e0401f0 100644 --- a/ghost/core/core/server/models/gift-delivery.js +++ b/ghost/core/core/server/models/gift-delivery.js @@ -7,9 +7,7 @@ const GiftDelivery = ghostBookshelf.Model.extend({ defaults: { status: 'pending', - attempts: 0, - outcome: 'unknown', - outcome_at_ms: 0 + outcome: 'unknown' }, gift() { diff --git a/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js b/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js index bf1aa325cb6..a735bd22497 100644 --- a/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js +++ b/ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js @@ -29,7 +29,6 @@ class EmailAnalyticsServiceWrapper { * @param {object} options * @param {Parameters[0]} options.event * @param {string[]} options.mailgunTags - * @param {() => {apiKey: string, domain: string, baseUrl: string}|null} [options.getMailgunConfig] * @param {JobNames} options.jobNames * @param {CursorSeed} options.cursorSeed * @param {() => BatchEventProcessor} options.createEventProcessor @@ -38,7 +37,6 @@ class EmailAnalyticsServiceWrapper { init({ event, mailgunTags, - getMailgunConfig, jobNames, cursorSeed, createEventProcessor, @@ -54,7 +52,7 @@ class EmailAnalyticsServiceWrapper { const {queries} = require('./lib/queries'); this.service = new EmailAnalyticsService({ - fetchEvents: (options) => fetchMailgunEvents({...options, config, settings, tags: mailgunTags, getMailgunConfig}), + fetchEvents: (options) => fetchMailgunEvents({...options, config, settings, tags: mailgunTags}), queries, prometheusClient, jobNames, diff --git a/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.js b/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.js index d2cf6bcbd68..82ac305acb3 100644 --- a/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.js +++ b/ghost/core/core/server/services/email-analytics/fetch-mailgun-events.js @@ -10,15 +10,14 @@ const PAGE_LIMIT = 300; * @param {object} options.config * @param {object} options.settings * @param {string[]} options.tags - * @param {() => {apiKey: string, domain: string, baseUrl: string}|null} [options.getMailgunConfig] * @param {Function} options.batchHandler * @param {number} [options.maxEvents] Not a strict maximum. We stop fetching after we reached the maximum AND received at least one event after begin (not equal) to prevent deadlocks. * @param {Date} [options.begin] * @param {Date} [options.end] * @param {string[]} [options.events] */ -async function fetchMailgunEvents({config, settings, tags, getMailgunConfig, batchHandler, maxEvents, begin, end, events}) { - const mailgunClient = new MailgunClient({config, settings, getMailgunConfig}); +async function fetchMailgunEvents({config, settings, tags, batchHandler, maxEvents, begin, end, events}) { + const mailgunClient = new MailgunClient({config, settings}); const mailgunOptions = { limit: PAGE_LIMIT, event: events ? events.join(' OR ') : DEFAULT_EVENT_FILTER, @@ -30,30 +29,4 @@ async function fetchMailgunEvents({config, settings, tags, getMailgunConfig, bat return await mailgunClient.fetchEvents(mailgunOptions, batchHandler, {maxEvents}); } -function getTransactionalMailgunConfig(config) { - const mail = config.get('mail'); - if (mail?.transport?.toLowerCase() !== 'mailgun') { - return null; - } - - const options = mail.options ?? {}; - const apiKey = options.auth?.api_key ?? options.auth?.apiKey; - const domain = options.auth?.domain; - if (!apiKey || !domain) { - return null; - } - - let baseUrl = options.url; - if (!baseUrl) { - const protocolOption = options.protocol ?? 'https:'; - const protocol = protocolOption.endsWith(':') ? protocolOption : `${protocolOption}:`; - const host = options.host ?? 'api.mailgun.net'; - const port = options.port ? `:${options.port}` : ''; - baseUrl = `${protocol}//${host}${port}`; - } - - return {apiKey, domain, baseUrl}; -} - module.exports.fetchMailgunEvents = fetchMailgunEvents; -module.exports.getTransactionalMailgunConfig = getTransactionalMailgunConfig; diff --git a/ghost/core/core/server/services/email-analytics/index.ts b/ghost/core/core/server/services/email-analytics/index.ts index b3074d62941..0e408fc86d7 100644 --- a/ghost/core/core/server/services/email-analytics/index.ts +++ b/ghost/core/core/server/services/email-analytics/index.ts @@ -26,8 +26,6 @@ import * as automationsApi from '../automations/automations-api'; import {AutomationEmailAnalyticsBatchProcessor} from './automation-email-analytics-batch-processor'; import {GiftEmailAnalyticsBatchProcessor} from './gift-email-analytics-batch-processor'; import {StartGiftEmailAnalyticsJobEvent} from './events/start-gift-email-analytics-job-event'; -// @ts-expect-error This CommonJS helper lacks type declarations. -import {getTransactionalMailgunConfig} from './fetch-mailgun-events'; // @ts-expect-error This CommonJS service wrapper lacks type declarations. import giftsService from '../gifts'; // @ts-expect-error This CommonJS service lacks type declarations. @@ -123,7 +121,6 @@ export const init = () => { gifts.init({ event: StartGiftEmailAnalyticsJobEvent, mailgunTags: ['gift-delivery'], - getMailgunConfig: () => getTransactionalMailgunConfig(config), jobNames: { latestNonOpened: 'email-analytics-gifts-latest-others', missing: 'email-analytics-gifts-missing', diff --git a/ghost/core/core/server/services/gifts/README.md b/ghost/core/core/server/services/gifts/README.md index d7f97aa5350..911445c6c47 100644 --- a/ghost/core/core/server/services/gifts/README.md +++ b/ghost/core/core/server/services/gifts/README.md @@ -25,9 +25,13 @@ transactions, or Stripe objects. adapters. Delivery claims are atomic and each delivery makes one Mailgun acceptance attempt. - `recordDeliveryOutcome(...)` retains only the newest Mailgun delivery outcome; - mail transport acceptance remains the authoritative sent fact. + mail transport acceptance remains the authoritative sent fact. A newly + recorded permanent provider failure sends the buyer a best-effort + transactional notification containing the gift link for manual sharing. - `reassignRedeemer(...)` is the import capability. The `Gift` and `GiftDelivery` models, their repositories, Bookshelf queries, Stripe checkout, email, scheduling, and notification collaborators are internal -adapters. +adapters. Recipient delivery uses the bulk Mailgun account so delivery outcomes +are observable; buyer-facing confirmations and failure notices use the +provider-agnostic transactional mailer. diff --git a/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts b/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts index fe05eb188f3..8a3ed9417d4 100644 --- a/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts +++ b/ghost/core/core/server/services/gifts/gift-delivery-bookshelf-repository.ts @@ -13,12 +13,12 @@ export interface GiftDeliverySchedule { export interface GiftDeliveryRepository { getById(id: string, options?: RepositoryTransactionOptions): Promise; getByGiftId(giftId: string, options?: RepositoryTransactionOptions): Promise; + getByProviderMessageId(providerMessageId: string): Promise; findDue(now: Date, limit: number): Promise; findPending(): Promise; countStuck(before: Date): Promise; - tryStartAttempt(id: string, now: Date, maxAttempts: number): Promise; + tryStartDelivery(id: string, now: Date): Promise; markSent(id: string, sentAt: Date, providerMessageId: string | null): Promise; - markForRetry(id: string, nextAttemptAt: Date): Promise; markFailed(id: string): Promise; markCancelled(id: string): Promise; cancelPendingForGift(token: string, options?: RepositoryTransactionOptions): Promise; @@ -65,6 +65,12 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { return model ? decodeGiftDeliveryRow(model.toJSON()) : null; } + async getByProviderMessageId(providerMessageId: string): Promise { + const model = await this.model.findOne({email_provider_message_id: providerMessageId}, {require: false}); + + return model ? decodeGiftDeliveryRow(model.toJSON()) : null; + } + async findDue(now: Date, limit: number): Promise { return this.transaction(async (transacting) => { const dueAt = toDatabaseDate(now); @@ -73,10 +79,7 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { .where('gift.status', 'purchased') .whereRaw('COALESCE(gift.available_at, gift.purchased_at) <= ?', [dueAt]) .where('delivery.status', 'pending') - .where((builder) => { - builder.whereNull('delivery.attempt_at').orWhere('delivery.attempt_at', '<=', dueAt); - }) - .orderByRaw('COALESCE(delivery.attempt_at, gift.available_at, gift.purchased_at) ASC') + .orderByRaw('COALESCE(gift.available_at, gift.purchased_at) ASC') .limit(limit) .select('delivery.*', 'gift.available_at', 'gift.purchased_at'); @@ -93,7 +96,7 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { .innerJoin('gifts as gift', 'gift.id', 'delivery.gift_id') .where('gift.status', 'purchased') .where('delivery.status', 'pending') - .orderByRaw('COALESCE(delivery.attempt_at, gift.available_at, gift.purchased_at) ASC') + .orderByRaw('COALESCE(gift.available_at, gift.purchased_at) ASC') .select('delivery.*', 'gift.available_at', 'gift.purchased_at'); return rows.map(row => ({ @@ -109,7 +112,7 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { .innerJoin('gifts as gift', 'gift.id', 'delivery.gift_id') .where('gift.status', 'purchased') .where('delivery.status', 'sending') - .where('delivery.attempt_at', '<=', toDatabaseDate(before)) + .where('delivery.started_at', '<=', toDatabaseDate(before)) .count({count: 'delivery.id'}) .first(); @@ -117,26 +120,21 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { }); } - async tryStartAttempt(id: string, now: Date, maxAttempts: number): Promise { + async tryStartDelivery(id: string, now: Date): Promise { return this.transaction(async (transacting) => { - const claimAt = toDatabaseDate(now); + const startedAt = toDatabaseDate(now); const eligibleGifts = transacting('gifts') .select('id') .where('status', 'purchased') - .whereRaw('COALESCE(available_at, purchased_at) <= ?', [claimAt]); + .whereRaw('COALESCE(available_at, purchased_at) <= ?', [startedAt]); const updated = await transacting('gift_deliveries') .where({id, status: 'pending'}) .whereIn('gift_id', eligibleGifts) - .where('attempts', '<', maxAttempts) - .where((builder) => { - builder.whereNull('attempt_at').orWhere('attempt_at', '<=', claimAt); - }) .update({ status: 'sending', - attempt_at: claimAt - }) - .increment('attempts', 1); + started_at: startedAt + }); if (updated !== 1) { return null; @@ -151,28 +149,21 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { status: 'sent', email_sent_at: toDatabaseDate(sentAt), email_provider_message_id: providerMessageId, - attempt_at: null - }); - } - - async markForRetry(id: string, nextAttemptAt: Date): Promise { - return this.updateState(id, 'sending', { - status: 'pending', - attempt_at: toDatabaseDate(nextAttemptAt) + started_at: null }); } async markFailed(id: string): Promise { return this.updateState(id, 'sending', { status: 'failed', - attempt_at: null + started_at: null }); } async markCancelled(id: string): Promise { return this.updateState(id, 'sending', { status: 'cancelled', - attempt_at: null + started_at: null }); } @@ -182,7 +173,7 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { const updated = await transacting('gift_deliveries') .where({status: 'pending'}) .whereIn('gift_id', gift) - .update({status: 'cancelled', attempt_at: null}); + .update({status: 'cancelled', started_at: null}); return updated === 1; }; @@ -193,23 +184,14 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { async recordOutcome({providerMessageId, outcome, timestamp, error}: {providerMessageId: string; outcome: GiftDeliveryOutcome; timestamp: Date; error: string | null}): Promise { return this.transaction(async (transacting) => { const outcomeAt = toDatabaseDate(timestamp); - const outcomeAtMilliseconds = timestamp.getUTCMilliseconds(); const updated = await transacting('gift_deliveries') .where({email_provider_message_id: providerMessageId}) .where((builder) => { - builder - .whereNull('outcome_at') - .orWhere('outcome_at', '<', outcomeAt) - .orWhere((sameSecond) => { - sameSecond - .where('outcome_at', '=', outcomeAt) - .where('outcome_at_ms', '<', outcomeAtMilliseconds); - }); + builder.whereNull('outcome_at').orWhere('outcome_at', '<', outcomeAt); }) .update({ outcome, outcome_at: outcomeAt, - outcome_at_ms: outcomeAtMilliseconds, outcome_error: error }); @@ -234,4 +216,5 @@ export class GiftDeliveryBookshelfRepository implements GiftDeliveryRepository { return updated === 1; }); } + } diff --git a/ghost/core/core/server/services/gifts/gift-delivery-codec.ts b/ghost/core/core/server/services/gifts/gift-delivery-codec.ts index d94d3e6e12b..361c9dae83b 100644 --- a/ghost/core/core/server/services/gifts/gift-delivery-codec.ts +++ b/ghost/core/core/server/services/gifts/gift-delivery-codec.ts @@ -2,16 +2,6 @@ import {z} from 'zod'; import {GiftDelivery} from './gift-delivery'; import {DbGiftDelivery} from './gift-delivery-schema'; -function withMilliseconds(date: Date | null, milliseconds: number): Date | null { - if (!date) { - return date; - } - - const preciseDate = new Date(date); - preciseDate.setUTCMilliseconds(milliseconds); - return preciseDate; -} - export const giftDeliveryCodec = z.codec(DbGiftDelivery, z.instanceof(GiftDelivery), { decode: row => new GiftDelivery({ id: row.id, @@ -22,7 +12,7 @@ export const giftDeliveryCodec = z.codec(DbGiftDelivery, z.instanceof(GiftDelive emailSentAt: row.email_sent_at, emailProviderMessageId: row.email_provider_message_id, outcome: row.outcome, - outcomeAt: withMilliseconds(row.outcome_at, row.outcome_at_ms), + outcomeAt: row.outcome_at, outcomeError: row.outcome_error }), encode: delivery => ({ @@ -35,7 +25,6 @@ export const giftDeliveryCodec = z.codec(DbGiftDelivery, z.instanceof(GiftDelive email_provider_message_id: delivery.emailProviderMessageId, outcome: delivery.outcome, outcome_at: delivery.outcomeAt, - outcome_at_ms: delivery.outcomeAt?.getUTCMilliseconds() ?? 0, outcome_error: delivery.outcomeError }) }); diff --git a/ghost/core/core/server/services/gifts/gift-delivery-schema.ts b/ghost/core/core/server/services/gifts/gift-delivery-schema.ts index 51f26b5079d..45279cfd9d4 100644 --- a/ghost/core/core/server/services/gifts/gift-delivery-schema.ts +++ b/ghost/core/core/server/services/gifts/gift-delivery-schema.ts @@ -9,7 +9,7 @@ export const GiftDeliveryOutcomeSchema = z.enum(['unknown', 'delivered', 'tempor export type GiftDeliveryStatus = z.infer; export type GiftDeliveryOutcome = z.infer; -const DbGiftDeliveryData = z.object({ +export const DbGiftDelivery = z.object({ id: z.string(), gift_id: z.string(), recipient_email: z.string().email(), @@ -22,11 +22,7 @@ const DbGiftDeliveryData = z.object({ outcome_error: z.string().nullable().default(null) }); -export const DbGiftDelivery = DbGiftDeliveryData.extend({ - outcome_at_ms: z.number().int().min(0).max(999).default(0) -}); - -type GiftDeliveryInputRow = z.input; +type GiftDeliveryInputRow = z.input; export type GiftDeliveryRow = z.output; -export type GiftDeliveryData = CamelKeys>; +export type GiftDeliveryData = CamelKeys; export type GiftDeliveryDataInput = SetOptional>>; diff --git a/ghost/core/core/server/services/gifts/gift-service.ts b/ghost/core/core/server/services/gifts/gift-service.ts index 635c847693a..31c8e462b57 100644 --- a/ghost/core/core/server/services/gifts/gift-service.ts +++ b/ghost/core/core/server/services/gifts/gift-service.ts @@ -111,6 +111,12 @@ interface GiftEmailService { duration: number; expiresAt: Date; }): Promise<{providerMessageId: string}>; + sendDeliveryFailureNotification(data: { + buyerEmail: string; + recipientEmail: string; + token: string; + expiresAt: Date; + }): Promise; } interface StaffServiceEmails { @@ -1180,7 +1186,46 @@ export class GiftService { } async recordDeliveryOutcome(data: {providerMessageId: string; outcome: 'delivered' | 'temporary_failed' | 'permanent_failed'; timestamp: Date; error: string | null}): Promise { - return this.deps.giftDeliveryRepository.recordOutcome(data); + const recorded = await this.deps.giftDeliveryRepository.recordOutcome(data); + + if (recorded && data.outcome === 'permanent_failed') { + await this.notifyBuyerOfDeliveryFailure(data.providerMessageId); + } + + return recorded; + } + + private async notifyBuyerOfDeliveryFailure(providerMessageId: string): Promise { + try { + const delivery = await this.deps.giftDeliveryRepository.getByProviderMessageId(providerMessageId); + if (!delivery) { + return; + } + + const gift = await this.deps.giftRepository.getById(delivery.giftId); + if (!gift || gift.status !== 'purchased' || gift.expiresAt.getTime() <= Date.now()) { + return; + } + + await this.deps.giftEmailService.sendDeliveryFailureNotification({ + buyerEmail: gift.buyerEmail, + recipientEmail: delivery.recipientEmail, + token: gift.token, + expiresAt: gift.expiresAt + }); + + logging.info({ + event: {name: 'gift_delivery.failure_notification.sent'}, + giftId: delivery.giftId, + deliveryId: delivery.id + }, 'Sent gift delivery failure notification to buyer'); + } catch (err) { + logging.error({ + event: {name: 'gift_delivery.failure_notification.failed'}, + err, + providerMessageId + }, 'Failed to send gift delivery failure notification to buyer'); + } } async processReminders(): Promise<{remindedCount: number; skippedCount: number; failedCount: number}> { diff --git a/ghost/core/core/server/services/lib/mailgun-client.js b/ghost/core/core/server/services/lib/mailgun-client.js index 5682efbb43a..917590d295b 100644 --- a/ghost/core/core/server/services/lib/mailgun-client.js +++ b/ghost/core/core/server/services/lib/mailgun-client.js @@ -9,12 +9,10 @@ const DEFAULT_BATCH_SIZE = 1000; module.exports = class MailgunClient { #config; #settings; - #getMailgunConfig; - constructor({config, settings, getMailgunConfig}) { + constructor({config, settings}) { this.#config = config; this.#settings = settings; - this.#getMailgunConfig = getMailgunConfig; } /** @@ -197,7 +195,7 @@ module.exports = class MailgunClient { #getDomainsToFetch(mailgunConfig) { const domains = [mailgunConfig.domain]; - const fallbackDomain = this.#getMailgunConfig ? null : this.#config.get('hostSettings:managedEmail:fallbackDomain'); + const fallbackDomain = this.#config.get('hostSettings:managedEmail:fallbackDomain'); if (fallbackDomain && fallbackDomain !== mailgunConfig.domain) { domains.push(fallbackDomain); logging.info(`[MailgunClient] Domain warming enabled, fetching from both primary (${mailgunConfig.domain}) and fallback (${fallbackDomain}) domains`); @@ -333,10 +331,6 @@ module.exports = class MailgunClient { } #getConfig() { - if (this.#getMailgunConfig) { - return this.#getMailgunConfig(); - } - const bulkEmailConfig = this.#config.get('bulkEmail'); const bulkEmailSetting = { apiKey: this.#settings.get('mailgun_api_key'), diff --git a/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts b/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts index bce45eea63a..a657e18e73b 100644 --- a/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts +++ b/ghost/core/test/integration/services/gifts/gift-delivery-bookshelf-repository.test.ts @@ -28,17 +28,17 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { async function createPendingEmailGift({ availableAt, - attemptAt = null, + startedAt = null, giftStatus = 'purchased' }: { availableAt: Date; - attemptAt?: Date | null; + startedAt?: Date | null; giftStatus?: string; }) { giftSequence += 1; const now = new Date(); const gift = await models.Gift.add({ - token: `delivery-attempt-test-token-${giftSequence}`, + token: `delivery-start-test-token-${giftSequence}`, buyer_email: `buyer-${giftSequence}@example.com`, buyer_member_id: null, buyer_name: 'Gift Buyer', @@ -67,8 +67,7 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { gift_id: gift.id, recipient_email: `recipient-${giftSequence}@example.com`, status: 'pending', - attempts: 0, - attempt_at: attemptAt, + started_at: startedAt, email_sent_at: null, email_provider_message_id: null, outcome: 'unknown', @@ -110,42 +109,34 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { assert.equal(await deliveryRepository.getByGiftId(gift.id), null); }); - it('allows exactly one concurrent caller to start a due delivery attempt', async function () { - const attemptAt = new Date(); - attemptAt.setMilliseconds(0); + it('allows exactly one concurrent caller to start a due delivery', async function () { + const startedAt = new Date(); + startedAt.setMilliseconds(0); const {delivery} = await createPendingEmailGift({ - availableAt: new Date(attemptAt.getTime() - 60_000), - attemptAt: new Date(attemptAt.getTime() - 30_000) + availableAt: new Date(startedAt.getTime() - 60_000) }); - const attempts = await Promise.all([ - deliveryRepository.tryStartAttempt(delivery.id, attemptAt, 10), - deliveryRepository.tryStartAttempt(delivery.id, attemptAt, 10) + const starts = await Promise.all([ + deliveryRepository.tryStartDelivery(delivery.id, startedAt), + deliveryRepository.tryStartDelivery(delivery.id, startedAt) ]); const reloaded = await deliveryRepository.getById(delivery.id); - assert.equal(attempts.filter(Boolean).length, 1); - assert.equal(attempts.filter(attempt => attempt === null).length, 1); + assert.equal(starts.filter(Boolean).length, 1); + assert.equal(starts.filter(start => start === null).length, 1); assert.equal(reloaded?.status, 'sending'); - assert.equal(reloaded?.attempts, 1); - assert.equal(reloaded?.attemptAt?.toISOString(), attemptAt.toISOString()); + assert.equal(reloaded?.startedAt?.toISOString(), startedAt.toISOString()); }); - it('does not start an attempt before gift availability or the retry time', async function () { - const attemptAt = new Date(); - attemptAt.setMilliseconds(0); + it('does not start a delivery before gift availability', async function () { + const startedAt = new Date(); + startedAt.setMilliseconds(0); const futureAvailability = await createPendingEmailGift({ - availableAt: new Date(attemptAt.getTime() + 60_000) - }); - const futureRetry = await createPendingEmailGift({ - availableAt: new Date(attemptAt.getTime() - 60_000), - attemptAt: new Date(attemptAt.getTime() + 60_000) + availableAt: new Date(startedAt.getTime() + 60_000) }); - assert.equal(await deliveryRepository.tryStartAttempt(futureAvailability.delivery.id, attemptAt, 10), null); - assert.equal(await deliveryRepository.tryStartAttempt(futureRetry.delivery.id, attemptAt, 10), null); - assert.equal((await deliveryRepository.getById(futureAvailability.delivery.id))?.attempts, 0); - assert.equal((await deliveryRepository.getById(futureRetry.delivery.id))?.attempts, 0); + assert.equal(await deliveryRepository.tryStartDelivery(futureAvailability.delivery.id, startedAt), null); + assert.equal((await deliveryRepository.getById(futureAvailability.delivery.id))?.status, 'pending'); }); it('finds only the oldest due deliveries within the requested batch size', async function () { @@ -160,44 +151,27 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { await createPendingEmailGift({ availableAt: new Date(now.getTime() + 60_000) }); - await createPendingEmailGift({ - availableAt: new Date(now.getTime() - 120_000), - attemptAt: new Date(now.getTime() + 60_000) - }); - const due = await deliveryRepository.findDue(now, 1); assert.equal(due.length, 1); assert.equal(due[0]?.delivery.id, oldestDue.delivery.id); }); - it('does not start an attempt once the attempt cap is reached', async function () { - const attemptAt = new Date(); - const {delivery} = await createPendingEmailGift({ - availableAt: new Date(attemptAt.getTime() - 60_000) - }); - await delivery.save({attempts: 10}, {patch: true}); - - assert.equal(await deliveryRepository.tryStartAttempt(delivery.id, attemptAt, 10), null); - assert.equal((await deliveryRepository.getById(delivery.id))?.attempts, 10); - }); - - it('does not complete or retry a delivery that is not sending', async function () { + it('does not complete a delivery that is not sending', async function () { const {delivery} = await createPendingEmailGift({availableAt: new Date()}); assert.equal(await deliveryRepository.markSent(delivery.id, new Date(), 'provider-1'), false); - assert.equal(await deliveryRepository.markForRetry(delivery.id, new Date()), false); assert.equal((await deliveryRepository.getById(delivery.id))?.status, 'pending'); }); - it('does not start an attempt when the parent gift is no longer purchased', async function () { - const attemptAt = new Date(); + it('does not start a delivery when the parent gift is no longer purchased', async function () { + const startedAt = new Date(); const {delivery} = await createPendingEmailGift({ - availableAt: new Date(attemptAt.getTime() - 60_000), + availableAt: new Date(startedAt.getTime() - 60_000), giftStatus: 'refunded' }); - assert.equal(await deliveryRepository.tryStartAttempt(delivery.id, attemptAt, 10), null); + assert.equal(await deliveryRepository.tryStartDelivery(delivery.id, startedAt), null); }); it('cancels a pending delivery by gift token', async function () { @@ -234,35 +208,7 @@ describe('GiftDeliveryBookshelfRepository (integration)', function () { assert.equal(reloaded?.outcome, 'delivered'); assert.equal(reloaded?.outcomeAt?.toISOString(), '2026-08-11T11:00:00.000Z'); assert.equal(reloaded?.outcomeError, null); + assert.equal((await deliveryRepository.getByProviderMessageId('provider-123'))?.id, delivery.id); }); - it('orders provider outcomes within the same second', async function () { - const {delivery} = await createPendingEmailGift({availableAt: new Date()}); - await delivery.save({ - email_provider_message_id: 'provider-123', - outcome: 'temporary_failed', - outcome_at: new Date('2026-08-11T10:00:00.000Z'), - outcome_at_ms: 900, - outcome_error: 'temporary rejection' - }, {patch: true}); - - assert.equal(await deliveryRepository.recordOutcome({ - providerMessageId: 'provider-123', - outcome: 'permanent_failed', - timestamp: new Date('2026-08-11T10:00:00.100Z'), - error: 'older rejection' - }), false); - - assert.equal(await deliveryRepository.recordOutcome({ - providerMessageId: 'provider-123', - outcome: 'delivered', - timestamp: new Date('2026-08-11T10:00:00.950Z'), - error: null - }), true); - - const reloaded = await deliveryRepository.getById(delivery.id); - assert.equal(reloaded?.outcome, 'delivered'); - assert.equal(reloaded?.outcomeAt?.toISOString(), '2026-08-11T10:00:00.950Z'); - assert.equal(reloaded?.outcomeError, null); - }); }); diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index fdebf2c3b92..72a06ec6fcc 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -35,7 +35,7 @@ const parseYaml = require('../../../../../core/server/services/route-settings/ya */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = 'ecce293efc721e93b51b66303a3a472e'; + const currentSchemaHash = '738dec6a7263d292731a49b0fa5dfdff'; const currentFixturesHash = 'a268b8ff06b240df97eb6060a2a478f5'; const currentSettingsHash = '8650db85b9a61afe4797ad6333066c62'; const currentRoutesHash = 'd8c25fa01bf6d22a2bcb05ba0de70dc1'; diff --git a/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.js b/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.js index 795ba82fd79..f7c9c24d610 100644 --- a/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.js +++ b/ghost/core/test/unit/server/services/email-analytics/fetch-mailgun-events.test.js @@ -1,8 +1,7 @@ -const assert = require('node:assert/strict'); const sinon = require('sinon'); const MailgunClient = require('../../../../../core/server/services/lib/mailgun-client'); -const {fetchMailgunEvents, getTransactionalMailgunConfig} = require('../../../../../core/server/services/email-analytics/fetch-mailgun-events'); +const {fetchMailgunEvents} = require('../../../../../core/server/services/email-analytics/fetch-mailgun-events'); const DEFAULT_TAGS = ['bulk-email']; const LATEST_TIMESTAMP = new Date('Thu Feb 25 2021 12:00:00 GMT+0000'); @@ -97,35 +96,4 @@ describe('fetchMailgunEvents', function () { event: 'delivered' }, batchHandler, {maxEvents: undefined}); }); - - it('maps the transactional Mailgun transport configuration', function () { - const transactionalConfig = { - get: sinon.stub().withArgs('mail').returns({ - transport: 'Mailgun', - options: { - auth: { - api_key: 'apiKey', - domain: 'transactional.example.com' - }, - host: 'api.eu.mailgun.net' - } - }) - }; - - assert.deepEqual(getTransactionalMailgunConfig(transactionalConfig), { - apiKey: 'apiKey', - domain: 'transactional.example.com', - baseUrl: 'https://api.eu.mailgun.net' - }); - }); - - it('does not fall back to bulk Mailgun configuration for other transactional transports', function () { - const transactionalConfig = { - get: sinon.stub().withArgs('mail').returns({ - transport: 'SMTP' - }) - }; - - assert.equal(getTransactionalMailgunConfig(transactionalConfig), null); - }); }); diff --git a/ghost/core/test/unit/server/services/gifts/gift-delivery.test.ts b/ghost/core/test/unit/server/services/gifts/gift-delivery.test.ts index 4db8d67b8c9..b6ce66a60b9 100644 --- a/ghost/core/test/unit/server/services/gifts/gift-delivery.test.ts +++ b/ghost/core/test/unit/server/services/gifts/gift-delivery.test.ts @@ -29,7 +29,6 @@ describe('GiftDelivery', function () { email_provider_message_id: 'provider-123', outcome: 'delivered', outcome_at: '2026-08-11T11:01:00.000Z', - outcome_at_ms: 123, outcome_error: null } as const; @@ -40,7 +39,7 @@ describe('GiftDelivery', function () { assert.deepEqual(encodeGiftDelivery(delivery), { ...row, email_sent_at: new Date(row.email_sent_at), - outcome_at: new Date('2026-08-11T11:01:00.123Z') + outcome_at: new Date(row.outcome_at) }); }); diff --git a/ghost/core/test/unit/server/services/gifts/gift-service.test.ts b/ghost/core/test/unit/server/services/gifts/gift-service.test.ts index d6d59fdb5d3..42571e73379 100644 --- a/ghost/core/test/unit/server/services/gifts/gift-service.test.ts +++ b/ghost/core/test/unit/server/services/gifts/gift-service.test.ts @@ -75,6 +75,7 @@ describe('GiftService', function () { sendPurchaseConfirmation: sinon.SinonStub; sendReminder: sinon.SinonStub; sendGiftDelivery: sinon.SinonStub; + sendDeliveryFailureNotification: sinon.SinonStub; }; let tiersService: { api: { @@ -117,6 +118,7 @@ describe('GiftService', function () { giftDeliveryRepository = { getById: sinon.stub().resolves(null), getByGiftId: sinon.stub().resolves(null), + getByProviderMessageId: sinon.stub().resolves(null), findDue: sinon.stub().resolves([]), findPending: sinon.stub().resolves([]), countStuck: sinon.stub().resolves(0), @@ -145,7 +147,8 @@ describe('GiftService', function () { giftEmailService = { sendPurchaseConfirmation: sinon.stub().resolves(undefined), sendReminder: sinon.stub().resolves(undefined), - sendGiftDelivery: sinon.stub().resolves({providerMessageId: null}) + sendGiftDelivery: sinon.stub().resolves({providerMessageId: null}), + sendDeliveryFailureNotification: sinon.stub().resolves(undefined) }; tiersService = { api: { @@ -922,6 +925,77 @@ describe('GiftService', function () { }); }); + describe('recordDeliveryOutcome', function () { + const permanentFailure = { + providerMessageId: 'provider-123', + outcome: 'permanent_failed' as const, + timestamp: new Date('2026-08-13T10:00:00.000Z'), + error: 'recipient rejected' + }; + + it('sends the buyer the gift link after recording a permanent delivery failure', async function () { + const delivery = buildGiftDelivery({status: 'sent', outcome: 'permanent_failed'}); + const gift = buildGift(); + giftDeliveryRepository.getByProviderMessageId.resolves(delivery); + giftRepository.getById.resolves(gift); + const service = createService(); + + assert.equal(await service.recordDeliveryOutcome(permanentFailure), true); + + sinon.assert.calledOnceWithExactly(giftDeliveryRepository.recordOutcome, permanentFailure); + sinon.assert.calledOnceWithExactly(giftDeliveryRepository.getByProviderMessageId, 'provider-123'); + sinon.assert.calledOnceWithExactly(giftEmailService.sendDeliveryFailureNotification, { + buyerEmail: gift.buyerEmail, + recipientEmail: delivery.recipientEmail, + token: gift.token, + expiresAt: gift.expiresAt + }); + }); + + it('does not notify again when the provider outcome was not newly recorded', async function () { + giftDeliveryRepository.recordOutcome.resolves(false); + const service = createService(); + + assert.equal(await service.recordDeliveryOutcome(permanentFailure), false); + + sinon.assert.notCalled(giftDeliveryRepository.getByProviderMessageId); + sinon.assert.notCalled(giftEmailService.sendDeliveryFailureNotification); + }); + + it('does not notify the buyer for temporary provider failures', async function () { + const service = createService(); + + await service.recordDeliveryOutcome({...permanentFailure, outcome: 'temporary_failed'}); + + sinon.assert.notCalled(giftDeliveryRepository.getByProviderMessageId); + sinon.assert.notCalled(giftEmailService.sendDeliveryFailureNotification); + }); + + it('does not notify for a gift that can no longer be redeemed', async function () { + const delivery = buildGiftDelivery(); + giftDeliveryRepository.getByProviderMessageId.resolves(delivery); + giftRepository.getById.resolves(buildGift({ + status: 'refunded', + refundedAt: new Date() + })); + const service = createService(); + + await service.recordDeliveryOutcome(permanentFailure); + + sinon.assert.notCalled(giftEmailService.sendDeliveryFailureNotification); + }); + + it('logs and ignores a buyer notification failure', async function () { + const delivery = buildGiftDelivery(); + giftDeliveryRepository.getByProviderMessageId.resolves(delivery); + giftRepository.getById.resolves(buildGift()); + giftEmailService.sendDeliveryFailureNotification.rejects({responseCode: 550}); + const service = createService(); + + assert.equal(await service.recordDeliveryOutcome(permanentFailure), true); + }); + }); + describe('processReminders', function () { const MS_PER_DAY = 24 * 60 * 60 * 1000; diff --git a/ghost/core/test/unit/server/services/lib/mailgun-client.test.js b/ghost/core/test/unit/server/services/lib/mailgun-client.test.js index a76431c2a2a..c5202ecef58 100644 --- a/ghost/core/test/unit/server/services/lib/mailgun-client.test.js +++ b/ghost/core/test/unit/server/services/lib/mailgun-client.test.js @@ -165,43 +165,6 @@ describe('MailgunClient', function () { assert.equal(mailgunClient.isConfigured(), false); }); - it('uses an explicit Mailgun configuration source without the managed bulk fallback domain', async function () { - const configStub = sinon.stub(config, 'get'); - configStub.withArgs('bulkEmail').returns({ - mailgun: { - apiKey: 'bulk-api-key', - domain: 'bulk.example.com', - baseUrl: 'https://api.mailgun.net' - } - }); - configStub.withArgs('hostSettings:managedEmail:fallbackDomain').returns('fallback.example.com'); - const getMailgunConfig = sinon.stub().returns({ - apiKey: 'apiKey', - domain: 'transactional.example.com', - baseUrl: 'https://api.mailgun.net' - }); - const transactionalApiMock = nock('https://api.mailgun.net') - .get('/v3/transactional.example.com/events') - .query(MAILGUN_OPTIONS) - .replyWithFile(200, `${__dirname}/fixtures/empty.json`, { - 'Content-Type': 'application/json' - }); - const fallbackApiMock = nock('https://api.mailgun.net') - .get('/v3/fallback.example.com/events') - .query(MAILGUN_OPTIONS) - .replyWithFile(200, `${__dirname}/fixtures/empty.json`, { - 'Content-Type': 'application/json' - }); - - const mailgunClient = new MailgunClient({config, settings, getMailgunConfig}); - await mailgunClient.fetchEvents(MAILGUN_OPTIONS, () => {}); - - assert.equal(transactionalApiMock.isDone(), true); - assert.equal(fallbackApiMock.isDone(), false); - sinon.assert.neverCalledWith(configStub, 'bulkEmail'); - sinon.assert.neverCalledWith(configStub, 'hostSettings:managedEmail:fallbackDomain'); - }); - it('respects changes in settings', async function () { const settingsStub = sinon.stub(settings, 'get'); settingsStub.withArgs('mailgun_api_key').returns('settingsApiKey');