Skip to content

Commit 4fbd67c

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

5 files changed

Lines changed: 174 additions & 37 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: 86 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const mocks = vi.hoisted(() => ({
1515
isAsyncJobEnqueueError: vi.fn(),
1616
markFailed: vi.fn(),
1717
markPushed: vi.fn(),
18+
queueCancel: vi.fn(),
1819
queueEnqueue: vi.fn(),
1920
queueGetJob: vi.fn(),
2021
requireAttempt: vi.fn(),
@@ -31,6 +32,8 @@ vi.mock('@/lib/core/async-jobs', () => ({
3132
JOB_STATUS: {
3233
COMPLETED: 'completed',
3334
FAILED: 'failed',
35+
PENDING: 'pending',
36+
PROCESSING: 'processing',
3437
},
3538
}))
3639

@@ -70,6 +73,7 @@ describe('newsletter Resend queueing', () => {
7073
vi.clearAllMocks()
7174
mocks.getAsyncBackendType.mockReturnValue('trigger-dev')
7275
mocks.getJobQueue.mockResolvedValue({
76+
cancelJob: mocks.queueCancel,
7377
enqueue: mocks.queueEnqueue,
7478
getJob: mocks.queueGetJob,
7579
})
@@ -142,23 +146,94 @@ describe('newsletter Resend queueing', () => {
142146
)
143147
})
144148

145-
it('moves a newsletter run to failed when its persisted database job failed', async () => {
146-
mocks.getAsyncBackendType.mockReturnValue('database')
147-
mocks.claimAttempt.mockResolvedValue({
148-
attempt: 2,
149-
jobId: 'newsletter_resend_run-1_2',
150-
run,
151-
shouldEnqueue: false,
152-
})
149+
it('starts a new attempt when a persisted Trigger.dev job failed', async () => {
150+
mocks.claimAttempt
151+
.mockResolvedValueOnce({
152+
attempt: 2,
153+
jobId: 'trigger-run-failed',
154+
run,
155+
shouldEnqueue: false,
156+
})
157+
.mockResolvedValueOnce({
158+
attempt: 3,
159+
jobId: null,
160+
run: { ...run, resendSyncJobId: null },
161+
shouldEnqueue: true,
162+
})
153163
mocks.queueGetJob.mockResolvedValue({
154-
id: 'newsletter_resend_run-1_2',
164+
id: 'trigger-run-failed',
155165
status: 'failed',
156166
error: 'worker stopped',
157167
})
168+
mocks.queueEnqueue.mockResolvedValue('trigger-run-retry')
169+
mocks.setJob.mockResolvedValue({ ...run, resendSyncJobId: 'trigger-run-retry' })
170+
171+
const result = await enqueueNewsletterResendSync('run-1', 'admin-1')
158172

159-
await expect(enqueueNewsletterResendSync('run-1', 'admin-1')).rejects.toThrow('worker stopped')
160173
expect(mocks.markFailed).toHaveBeenCalledWith('run-1', 2, expect.any(Error))
161-
expect(mocks.queueEnqueue).not.toHaveBeenCalled()
174+
expect(mocks.queueEnqueue).toHaveBeenCalledWith(
175+
'newsletter-resend-sync',
176+
{ runId: 'run-1', attempt: 3, requestedById: 'admin-1' },
177+
expect.objectContaining({ jobId: 'newsletter_resend_run-1_3' })
178+
)
179+
expect(result.jobId).toBe('trigger-run-retry')
180+
})
181+
182+
it('re-enqueues when a stored Trigger.dev run no longer exists', async () => {
183+
mocks.claimAttempt
184+
.mockResolvedValueOnce({
185+
attempt: 2,
186+
jobId: 'trigger-run-missing',
187+
run,
188+
shouldEnqueue: false,
189+
})
190+
.mockResolvedValueOnce({
191+
attempt: 3,
192+
jobId: null,
193+
run,
194+
shouldEnqueue: true,
195+
})
196+
mocks.queueGetJob.mockResolvedValue(null)
197+
198+
await enqueueNewsletterResendSync('run-1', 'admin-1')
199+
200+
expect(mocks.markFailed).toHaveBeenCalledWith('run-1', 2, expect.any(Error))
201+
expect(mocks.queueEnqueue).toHaveBeenCalledWith(
202+
'newsletter-resend-sync',
203+
{ runId: 'run-1', attempt: 3, requestedById: 'admin-1' },
204+
expect.objectContaining({ jobId: 'newsletter_resend_run-1_3' })
205+
)
206+
})
207+
208+
it('cancels and replaces an active Trigger.dev run when an admin resumes it', async () => {
209+
mocks.claimAttempt
210+
.mockResolvedValueOnce({
211+
attempt: 2,
212+
jobId: 'trigger-run-active',
213+
run,
214+
shouldEnqueue: false,
215+
})
216+
.mockResolvedValueOnce({
217+
attempt: 3,
218+
jobId: null,
219+
run,
220+
shouldEnqueue: true,
221+
})
222+
mocks.queueGetJob.mockResolvedValue({
223+
id: 'trigger-run-active',
224+
status: 'processing',
225+
})
226+
227+
const result = await enqueueNewsletterResendSync('run-1', 'admin-1')
228+
229+
expect(mocks.queueCancel).toHaveBeenCalledWith('trigger-run-active')
230+
expect(mocks.markFailed).toHaveBeenCalledWith('run-1', 2, expect.any(Error))
231+
expect(mocks.queueEnqueue).toHaveBeenCalledWith(
232+
'newsletter-resend-sync',
233+
{ runId: 'run-1', attempt: 3, requestedById: 'admin-1' },
234+
expect.objectContaining({ jobId: 'newsletter_resend_run-1_3' })
235+
)
236+
expect(result.jobId).toBe('trigger-run-123')
162237
})
163238

164239
it('resets failed recipients before a task retry', async () => {

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

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -174,25 +174,37 @@ 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
}
188-
if (persistedJob?.status === JOB_STATUS.FAILED) {
189-
const error = new Error(persistedJob.error ?? 'Newsletter sync database job failed')
185+
if (backendType !== 'database') {
186+
if (
187+
persistedJob?.status === JOB_STATUS.PENDING ||
188+
persistedJob?.status === JOB_STATUS.PROCESSING
189+
) {
190+
await queue.cancelJob(claim.jobId)
191+
}
192+
const error = new Error(
193+
persistedJob?.error ?? 'Newsletter sync was resumed with a fresh background job'
194+
)
195+
await markNewsletterRunPushFailed(runId, claim.attempt, error)
196+
claim = await claimNewsletterRunResendAttempt(runId)
197+
} else if (persistedJob?.status === JOB_STATUS.FAILED) {
198+
const error = new Error(persistedJob.error ?? 'Newsletter sync job failed')
190199
await markNewsletterRunPushFailed(runId, claim.attempt, error)
191-
throw error
200+
claim = await claimNewsletterRunResendAttempt(runId)
192201
}
193202
}
194203

195-
const enqueueKey = claim.jobId ?? `newsletter_resend_${runId}_${claim.attempt}`
204+
const enqueueKey =
205+
backendType === 'database' && claim.jobId
206+
? claim.jobId
207+
: `newsletter_resend_${runId}_${claim.attempt}`
196208
let jobId: string
197209
try {
198210
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)