Skip to content

Commit 59bdf0b

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(newsletters): handle review recovery cases
1 parent 85ec0dd commit 59bdf0b

5 files changed

Lines changed: 142 additions & 33 deletions

File tree

apps/sim/components/settings/account-settings-renderer.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,5 +49,6 @@ export function AccountSettingsRenderer({ section }: AccountSettingsRendererProp
4949
if (section === 'api-keys') return <ApiKeys scope='personal' />
5050
if (section === 'admin') return <Admin />
5151
if (section === 'mothership') return <Mothership />
52-
return <Newsletters />
52+
if (section === 'newsletters') return <Newsletters />
53+
return null
5354
}

apps/sim/lib/newsletters/push-resend.test.ts

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ vi.mock('@/lib/core/async-jobs', () => ({
3131
JOB_STATUS: {
3232
COMPLETED: 'completed',
3333
FAILED: 'failed',
34+
PENDING: 'pending',
35+
PROCESSING: 'processing',
3436
},
3537
}))
3638

@@ -142,22 +144,72 @@ describe('newsletter Resend queueing', () => {
142144
)
143145
})
144146

145-
it('moves a newsletter run to failed when its persisted database job failed', async () => {
146-
mocks.getAsyncBackendType.mockReturnValue('database')
147+
it('starts a new attempt when a persisted Trigger.dev job failed', async () => {
148+
mocks.claimAttempt
149+
.mockResolvedValueOnce({
150+
attempt: 2,
151+
jobId: 'trigger-run-failed',
152+
run,
153+
shouldEnqueue: false,
154+
})
155+
.mockResolvedValueOnce({
156+
attempt: 3,
157+
jobId: null,
158+
run: { ...run, resendSyncJobId: null },
159+
shouldEnqueue: true,
160+
})
161+
mocks.queueGetJob.mockResolvedValue({
162+
id: 'trigger-run-failed',
163+
status: 'failed',
164+
error: 'worker stopped',
165+
})
166+
mocks.queueEnqueue.mockResolvedValue('trigger-run-retry')
167+
mocks.setJob.mockResolvedValue({ ...run, resendSyncJobId: 'trigger-run-retry' })
168+
169+
const result = await enqueueNewsletterResendSync('run-1', 'admin-1')
170+
171+
expect(mocks.markFailed).toHaveBeenCalledWith('run-1', 2, expect.any(Error))
172+
expect(mocks.queueEnqueue).toHaveBeenCalledWith(
173+
'newsletter-resend-sync',
174+
{ runId: 'run-1', attempt: 3, requestedById: 'admin-1' },
175+
expect.objectContaining({ jobId: 'newsletter_resend_run-1_3' })
176+
)
177+
expect(result.jobId).toBe('trigger-run-retry')
178+
})
179+
180+
it('re-enqueues when a stored Trigger.dev run no longer exists', async () => {
147181
mocks.claimAttempt.mockResolvedValue({
148182
attempt: 2,
149-
jobId: 'newsletter_resend_run-1_2',
183+
jobId: 'trigger-run-missing',
184+
run,
185+
shouldEnqueue: false,
186+
})
187+
mocks.queueGetJob.mockResolvedValue(null)
188+
189+
await enqueueNewsletterResendSync('run-1', 'admin-1')
190+
191+
expect(mocks.queueEnqueue).toHaveBeenCalledWith(
192+
'newsletter-resend-sync',
193+
{ runId: 'run-1', attempt: 2, requestedById: 'admin-1' },
194+
expect.objectContaining({ jobId: 'newsletter_resend_run-1_2' })
195+
)
196+
})
197+
198+
it('does not duplicate an active Trigger.dev run', async () => {
199+
mocks.claimAttempt.mockResolvedValue({
200+
attempt: 2,
201+
jobId: 'trigger-run-active',
150202
run,
151203
shouldEnqueue: false,
152204
})
153205
mocks.queueGetJob.mockResolvedValue({
154-
id: 'newsletter_resend_run-1_2',
155-
status: 'failed',
156-
error: 'worker stopped',
206+
id: 'trigger-run-active',
207+
status: 'processing',
157208
})
158209

159-
await expect(enqueueNewsletterResendSync('run-1', 'admin-1')).rejects.toThrow('worker stopped')
160-
expect(mocks.markFailed).toHaveBeenCalledWith('run-1', 2, expect.any(Error))
210+
const result = await enqueueNewsletterResendSync('run-1', 'admin-1')
211+
212+
expect(result.jobId).toBe('trigger-run-active')
161213
expect(mocks.queueEnqueue).not.toHaveBeenCalled()
162214
})
163215

apps/sim/lib/newsletters/push-resend.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,25 +174,32 @@ export async function runNewsletterResendSync(
174174
}
175175

176176
export async function enqueueNewsletterResendSync(runId: string, requestedById: string) {
177-
const claim = await claimNewsletterRunResendAttempt(runId)
177+
let claim = await claimNewsletterRunResendAttempt(runId)
178178
const queue = await getJobQueue()
179+
const backendType = getAsyncBackendType()
179180
if (!claim.shouldEnqueue && claim.jobId) {
180-
if (getAsyncBackendType() !== 'database') {
181-
return { run: claim.run, jobId: claim.jobId }
182-
}
183-
184181
const persistedJob = await queue.getJob(claim.jobId)
185182
if (persistedJob?.status === JOB_STATUS.COMPLETED) {
186183
return { run: claim.run, jobId: claim.jobId }
187184
}
185+
if (
186+
persistedJob &&
187+
backendType !== 'database' &&
188+
(persistedJob.status === JOB_STATUS.PENDING || persistedJob.status === JOB_STATUS.PROCESSING)
189+
) {
190+
return { run: claim.run, jobId: claim.jobId }
191+
}
188192
if (persistedJob?.status === JOB_STATUS.FAILED) {
189-
const error = new Error(persistedJob.error ?? 'Newsletter sync database job failed')
193+
const error = new Error(persistedJob.error ?? 'Newsletter sync job failed')
190194
await markNewsletterRunPushFailed(runId, claim.attempt, error)
191-
throw error
195+
claim = await claimNewsletterRunResendAttempt(runId)
192196
}
193197
}
194198

195-
const enqueueKey = claim.jobId ?? `newsletter_resend_${runId}_${claim.attempt}`
199+
const enqueueKey =
200+
backendType === 'database' && claim.jobId
201+
? claim.jobId
202+
: `newsletter_resend_${runId}_${claim.attempt}`
196203
let jobId: string
197204
try {
198205
await resetFailedNewsletterRecipients(runId)

apps/sim/lib/newsletters/resend.test.ts

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,21 +127,53 @@ describe('newsletter Resend service', () => {
127127
it('normalizes suppressed email addresses', async () => {
128128
fetchMock.mockResolvedValueOnce(
129129
jsonResponse({
130-
data: [{ email: ' First@Example.com ' }, { email: 'second@example.com' }],
130+
data: [
131+
{ id: 'suppression-1', email: ' First@Example.com ' },
132+
{ id: 'suppression-2', email: 'second@example.com' },
133+
],
131134
has_more: false,
132135
})
133136
)
134137

135138
const emails = await getResendSuppressedEmails()
136139

137140
expect(emails).toEqual(new Set(['first@example.com', 'second@example.com']))
141+
expect(fetchMock).toHaveBeenCalledWith(
142+
'https://api.resend.com/suppressions?limit=100',
143+
expect.objectContaining({ method: 'GET' })
144+
)
145+
})
146+
147+
it('paginates through all suppressed email addresses', async () => {
148+
fetchMock
149+
.mockResolvedValueOnce(
150+
jsonResponse({
151+
data: [{ id: 'suppression-1', email: 'first@example.com' }],
152+
has_more: true,
153+
})
154+
)
155+
.mockResolvedValueOnce(
156+
jsonResponse({
157+
data: [{ id: 'suppression-2', email: 'second@example.com' }],
158+
has_more: false,
159+
})
160+
)
161+
162+
const emails = await getResendSuppressedEmails()
163+
164+
expect(emails).toEqual(new Set(['first@example.com', 'second@example.com']))
165+
expect(fetchMock).toHaveBeenNthCalledWith(
166+
2,
167+
'https://api.resend.com/suppressions?limit=100&after=suppression-1',
168+
expect.objectContaining({ method: 'GET' })
169+
)
138170
})
139171

140172
it('combines suppressions with globally unsubscribed contacts', async () => {
141173
fetchMock
142174
.mockResolvedValueOnce(
143175
jsonResponse({
144-
data: [{ email: 'suppressed@example.com' }],
176+
data: [{ id: 'suppression-1', email: 'suppressed@example.com' }],
145177
has_more: false,
146178
})
147179
)

apps/sim/lib/newsletters/resend.ts

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ const RESEND_CONTACT_PROPERTY_PAGE_LIMIT = 100
1313
const RESEND_CONTACT_PROPERTY_MAX_PAGES = 10
1414
const RESEND_CONTACT_PAGE_LIMIT = 100
1515
const RESEND_CONTACT_MAX_PAGES = 1000
16+
const RESEND_SUPPRESSION_PAGE_LIMIT = 100
17+
const RESEND_SUPPRESSION_MAX_PAGES = 1000
1618
const NEWSLETTER_CONTACT_PROPERTY_KEYS = ['sim_user_id', 'newsletter_run_id'] as const
1719

1820
interface ResendErrorBody {
@@ -43,7 +45,7 @@ interface ResendRequestOptions {
4345
}
4446

4547
const resendSuppressionListSchema = z.object({
46-
data: z.array(z.object({ email: z.string().min(1) })),
48+
data: z.array(z.object({ id: z.string().min(1), email: z.string().min(1) })),
4749
has_more: z.boolean(),
4850
})
4951

@@ -116,22 +118,37 @@ async function resendRequest<T>(path: string, options: ResendRequestOptions = {}
116118
}
117119

118120
export async function getResendSuppressedEmails(options?: { required?: boolean }) {
119-
const rawResponse = await resendRequest<unknown>('/suppressions', {
120-
required: options?.required ?? true,
121-
})
122-
if (rawResponse === undefined) return new Set<string>()
123-
const parsedResponse = resendSuppressionListSchema.safeParse(rawResponse)
124-
if (!parsedResponse.success) {
125-
throw new Error('Resend suppression list response was malformed')
126-
}
127-
const response = parsedResponse.data
128121
const emails = new Set<string>()
129-
if (response?.has_more) {
130-
throw new Error('Resend suppression list was incomplete')
122+
let after: string | null = null
123+
let hasMore = false
124+
125+
for (let page = 0; page < RESEND_SUPPRESSION_MAX_PAGES; page++) {
126+
const query = new URLSearchParams({ limit: String(RESEND_SUPPRESSION_PAGE_LIMIT) })
127+
if (after) query.set('after', after)
128+
const rawResponse = await resendRequest<unknown>(`/suppressions?${query.toString()}`, {
129+
required: options?.required ?? true,
130+
})
131+
if (rawResponse === undefined) return emails
132+
const parsedResponse = resendSuppressionListSchema.safeParse(rawResponse)
133+
if (!parsedResponse.success) {
134+
throw new Error('Resend suppression list response was malformed')
135+
}
136+
const response = parsedResponse.data
137+
138+
for (const suppression of response.data) {
139+
emails.add(normalizeEmail(suppression.email))
140+
}
141+
142+
hasMore = response.has_more
143+
if (!hasMore) break
144+
after = response.data.at(-1)?.id ?? null
145+
if (!after) throw new Error('Resend suppression pagination returned no cursor')
131146
}
132-
for (const suppression of response.data) {
133-
emails.add(normalizeEmail(suppression.email))
147+
148+
if (hasMore) {
149+
throw new Error('Resend suppression list exceeded the pagination safety limit')
134150
}
151+
135152
return emails
136153
}
137154

0 commit comments

Comments
 (0)