Skip to content

Commit f8c79bf

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(newsletters): propagate sync cancellation
1 parent 4e522cd commit f8c79bf

6 files changed

Lines changed: 186 additions & 12 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({
7+
runSync: vi.fn(),
8+
task: vi.fn((definition) => definition),
9+
}))
10+
11+
vi.mock('@trigger.dev/sdk', () => ({
12+
task: mocks.task,
13+
}))
14+
15+
vi.mock('@/lib/newsletters/push-resend', () => ({
16+
NEWSLETTER_RESEND_SYNC_CONCURRENCY_LIMIT: 1,
17+
NEWSLETTER_RESEND_SYNC_MAX_ATTEMPTS: 3,
18+
runNewsletterResendSync: mocks.runSync,
19+
}))
20+
21+
import type { NewsletterResendSyncPayload } from '@/lib/newsletters/push-resend'
22+
import { newsletterResendSyncTask } from '@/background/newsletter-resend-sync'
23+
24+
interface NewsletterTaskDefinition {
25+
run: (payload: NewsletterResendSyncPayload, context: { signal: AbortSignal }) => Promise<unknown>
26+
}
27+
28+
it('forwards the Trigger.dev cancellation signal to the sync service', async () => {
29+
const payload = {
30+
runId: 'run-1',
31+
attempt: 1,
32+
requestedById: 'admin-1',
33+
}
34+
const controller = new AbortController()
35+
const definition = newsletterResendSyncTask as unknown as NewsletterTaskDefinition
36+
37+
await definition.run(payload, { signal: controller.signal })
38+
39+
expect(mocks.runSync).toHaveBeenCalledWith(payload, controller.signal)
40+
})

apps/sim/background/newsletter-resend-sync.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,6 @@ export const newsletterResendSyncTask = task({
1515
queue: {
1616
concurrencyLimit: NEWSLETTER_RESEND_SYNC_CONCURRENCY_LIMIT,
1717
},
18-
run: async (payload: NewsletterResendSyncPayload) => runNewsletterResendSync(payload),
18+
run: async (payload: NewsletterResendSyncPayload, { signal }) =>
19+
runNewsletterResendSync(payload, signal),
1920
})

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

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
66
const mocks = vi.hoisted(() => ({
77
claimAttempt: vi.fn(),
88
countByStatus: vi.fn(),
9+
createContact: vi.fn(),
910
createSegment: vi.fn(),
1011
ensureProperties: vi.fn(),
1112
getAsyncBackendType: vi.fn(),
@@ -39,7 +40,7 @@ vi.mock('@/lib/core/async-jobs', () => ({
3940

4041
vi.mock('@/lib/newsletters/resend', () => ({
4142
createNewsletterSegment: mocks.createSegment,
42-
createOrSegmentNewsletterContact: vi.fn(),
43+
createOrSegmentNewsletterContact: mocks.createContact,
4344
ensureNewsletterContactProperties: mocks.ensureProperties,
4445
getResendExcludedEmails: mocks.getExcludedEmails,
4546
}))
@@ -387,6 +388,46 @@ describe('newsletter Resend queueing', () => {
387388
expect(mocks.markFailed).not.toHaveBeenCalled()
388389
})
389390

391+
it('forwards cancellation to every Resend operation', async () => {
392+
const controller = new AbortController()
393+
const recipient = {
394+
currentUserEligible: true,
395+
email: 'user@example.com',
396+
name: 'Ada Lovelace',
397+
simUnsubscribed: false,
398+
snapshotVersion: 1,
399+
userId: 'user-1',
400+
}
401+
mocks.requireAttempt.mockResolvedValue({
402+
...run,
403+
resendSegmentId: null,
404+
resendSegmentName: null,
405+
})
406+
mocks.createSegment.mockResolvedValue({ id: 'segment-new', name: 'Segment new' })
407+
mocks.getExcludedEmails.mockResolvedValue(new Set())
408+
mocks.getPendingRecipients.mockResolvedValueOnce([recipient]).mockResolvedValueOnce([])
409+
mocks.createContact.mockResolvedValue({ status: 'created', contactId: 'contact-1' })
410+
mocks.countByStatus.mockResolvedValue({})
411+
412+
await runNewsletterResendSync(
413+
{
414+
runId: 'run-1',
415+
attempt: 2,
416+
requestedById: 'admin-1',
417+
},
418+
controller.signal
419+
)
420+
421+
expect(mocks.createSegment).toHaveBeenCalledWith(expect.any(String), {
422+
signal: controller.signal,
423+
})
424+
expect(mocks.ensureProperties).toHaveBeenCalledWith({ signal: controller.signal })
425+
expect(mocks.getExcludedEmails).toHaveBeenCalledWith({ signal: controller.signal })
426+
expect(mocks.createContact).toHaveBeenCalledWith(
427+
expect.objectContaining({ signal: controller.signal })
428+
)
429+
})
430+
390431
it('does not mark a run pushed after ownership is lost during status aggregation', async () => {
391432
const controller = new AbortController()
392433
mocks.requireAttempt.mockResolvedValue(run)

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ async function runWithConcurrency<T>(
5555
for (let index = 0; index < items.length; index += limit) {
5656
signal?.throwIfAborted()
5757
await Promise.all(items.slice(index, index + limit).map(worker))
58+
signal?.throwIfAborted()
5859
}
5960
}
6061

@@ -79,17 +80,17 @@ export async function runNewsletterResendSync(
7980
const segment =
8081
run.resendSegmentId && run.resendSegmentName
8182
? { id: run.resendSegmentId, name: run.resendSegmentName }
82-
: await createNewsletterSegment(segmentNameForRun(run.name))
83+
: await createNewsletterSegment(segmentNameForRun(run.name), { signal })
8384

8485
signal?.throwIfAborted()
8586
if (!run.resendSegmentId) {
8687
await setNewsletterRunResendSegment(runId, attempt, segment.id, segment.name)
8788
}
8889

8990
signal?.throwIfAborted()
90-
await ensureNewsletterContactProperties()
91+
await ensureNewsletterContactProperties({ signal })
9192
signal?.throwIfAborted()
92-
const suppressedEmails = await getResendExcludedEmails()
93+
const suppressedEmails = await getResendExcludedEmails({ signal })
9394
let processed = 0
9495

9596
while (true) {
@@ -128,6 +129,7 @@ export async function runNewsletterResendSync(
128129
userId: recipient.userId,
129130
runId,
130131
segmentId: segment.id,
132+
signal,
131133
})
132134
signal?.throwIfAborted()
133135
await updateRecipientSyncStatus(

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

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,21 @@
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { fetchMock } = vi.hoisted(() => ({
6+
const { fetchMock, sleepMock } = vi.hoisted(() => ({
77
fetchMock: vi.fn(),
8+
sleepMock: vi.fn(),
89
}))
910

1011
vi.mock('@/lib/core/config/env', () => ({
1112
env: { RESEND_API_KEY: 're_test' },
1213
}))
1314

15+
vi.mock('@sim/utils/helpers', () => ({
16+
sleep: sleepMock,
17+
}))
18+
1419
import {
20+
createNewsletterSegment,
1521
createOrSegmentNewsletterContact,
1622
ensureNewsletterContactProperties,
1723
getResendExcludedEmails,
@@ -29,6 +35,42 @@ describe('newsletter Resend service', () => {
2935
beforeEach(() => {
3036
vi.clearAllMocks()
3137
vi.stubGlobal('fetch', fetchMock)
38+
sleepMock.mockResolvedValue(undefined)
39+
})
40+
41+
it('forwards cancellation to Resend requests', async () => {
42+
const controller = new AbortController()
43+
fetchMock.mockResolvedValueOnce(jsonResponse({ id: 'segment-1', name: 'Segment 1' }, 201))
44+
45+
await createNewsletterSegment('Segment 1', { signal: controller.signal })
46+
47+
expect(fetchMock).toHaveBeenCalledWith(
48+
'https://api.resend.com/segments',
49+
expect.objectContaining({ signal: controller.signal })
50+
)
51+
})
52+
53+
it('does not make a Resend request when already aborted', async () => {
54+
const controller = new AbortController()
55+
controller.abort('cancelled')
56+
57+
await expect(createNewsletterSegment('Segment 1', { signal: controller.signal })).rejects.toBe(
58+
'cancelled'
59+
)
60+
expect(fetchMock).not.toHaveBeenCalled()
61+
})
62+
63+
it('does not retry after cancellation during backoff', async () => {
64+
const controller = new AbortController()
65+
fetchMock.mockResolvedValueOnce(jsonResponse({ message: 'retry' }, 429))
66+
sleepMock.mockImplementationOnce(async () => {
67+
controller.abort('cancelled')
68+
})
69+
70+
await expect(createNewsletterSegment('Segment 1', { signal: controller.signal })).rejects.toBe(
71+
'cancelled'
72+
)
73+
expect(fetchMock).toHaveBeenCalledTimes(1)
3274
})
3375

3476
it('creates missing newsletter contact properties', async () => {
@@ -124,6 +166,28 @@ describe('newsletter Resend service', () => {
124166
)
125167
})
126168

169+
it('does not add segment membership after cancellation', async () => {
170+
const controller = new AbortController()
171+
fetchMock
172+
.mockResolvedValueOnce(jsonResponse({ message: 'Contact already exists' }, 409))
173+
.mockImplementationOnce(async () => {
174+
controller.abort('cancelled')
175+
return jsonResponse({ id: 'contact-1' })
176+
})
177+
178+
await expect(
179+
createOrSegmentNewsletterContact({
180+
email: 'user@example.com',
181+
name: 'Ada Lovelace',
182+
userId: 'user-1',
183+
runId: 'run-1',
184+
segmentId: 'segment-1',
185+
signal: controller.signal,
186+
})
187+
).rejects.toBe('cancelled')
188+
expect(fetchMock).toHaveBeenCalledTimes(2)
189+
})
190+
127191
it('normalizes suppressed email addresses', async () => {
128192
fetchMock.mockResolvedValueOnce(
129193
jsonResponse({

apps/sim/lib/newsletters/resend.ts

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ interface ResendRequestOptions {
4242
body?: Record<string, unknown>
4343
required?: boolean
4444
maxResponseBytes?: number
45+
signal?: AbortSignal
4546
}
4647

4748
const resendSuppressionListSchema = z.object({
@@ -88,13 +89,15 @@ async function resendRequest<T>(path: string, options: ResendRequestOptions = {}
8889
if (!key) return undefined as T
8990

9091
for (let attempt = 0; attempt < RESEND_MAX_ATTEMPTS; attempt++) {
92+
options.signal?.throwIfAborted()
9193
const response = await fetch(`${RESEND_API_BASE}${path}`, {
9294
method: options.method ?? 'GET',
9395
headers: {
9496
Authorization: `Bearer ${key}`,
9597
'Content-Type': 'application/json',
9698
},
9799
body: options.body ? JSON.stringify(options.body) : undefined,
100+
signal: options.signal,
98101
})
99102

100103
if (response.ok) {
@@ -108,6 +111,7 @@ async function resendRequest<T>(path: string, options: ResendRequestOptions = {}
108111
if (response.status === 429 || response.status >= 500) {
109112
const retryAfterMs = parseRetryAfter(response.headers.get('retry-after'))
110113
await sleep(backoffWithJitter(attempt + 1, retryAfterMs, { baseMs: 250, maxMs: 5000 }))
114+
options.signal?.throwIfAborted()
111115
continue
112116
}
113117

@@ -117,7 +121,10 @@ async function resendRequest<T>(path: string, options: ResendRequestOptions = {}
117121
throw new Error('Resend request failed after retries')
118122
}
119123

120-
export async function getResendSuppressedEmails(options?: { required?: boolean }) {
124+
export async function getResendSuppressedEmails(options?: {
125+
required?: boolean
126+
signal?: AbortSignal
127+
}) {
121128
const emails = new Set<string>()
122129
let after: string | null = null
123130
let hasMore = false
@@ -127,6 +134,7 @@ export async function getResendSuppressedEmails(options?: { required?: boolean }
127134
if (after) query.set('after', after)
128135
const rawResponse = await resendRequest<unknown>(`/suppressions?${query.toString()}`, {
129136
required: options?.required ?? true,
137+
signal: options?.signal,
130138
})
131139
if (rawResponse === undefined) return emails
132140
const parsedResponse = resendSuppressionListSchema.safeParse(rawResponse)
@@ -152,8 +160,13 @@ export async function getResendSuppressedEmails(options?: { required?: boolean }
152160
return emails
153161
}
154162

155-
export async function getResendExcludedEmails(): Promise<Set<string>> {
156-
const excludedEmails = await getResendSuppressedEmails({ required: true })
163+
export async function getResendExcludedEmails(options?: {
164+
signal?: AbortSignal
165+
}): Promise<Set<string>> {
166+
const excludedEmails = await getResendSuppressedEmails({
167+
required: true,
168+
signal: options?.signal,
169+
})
157170
let after: string | null = null
158171
let hasMore = false
159172

@@ -162,6 +175,7 @@ export async function getResendExcludedEmails(): Promise<Set<string>> {
162175
if (after) query.set('after', after)
163176
const rawContacts = await resendRequest<unknown>(`/contacts?${query.toString()}`, {
164177
required: true,
178+
signal: options?.signal,
165179
})
166180
const parsedContacts = resendContactListSchema.safeParse(rawContacts)
167181
if (!parsedContacts.success) {
@@ -188,14 +202,20 @@ export async function getResendExcludedEmails(): Promise<Set<string>> {
188202
return excludedEmails
189203
}
190204

191-
export async function createNewsletterSegment(name: string): Promise<ResendSegmentResponse> {
205+
export async function createNewsletterSegment(
206+
name: string,
207+
options?: { signal?: AbortSignal }
208+
): Promise<ResendSegmentResponse> {
192209
return resendRequest<ResendSegmentResponse>('/segments', {
193210
method: 'POST',
194211
body: { name },
212+
signal: options?.signal,
195213
})
196214
}
197215

198-
export async function ensureNewsletterContactProperties(): Promise<void> {
216+
export async function ensureNewsletterContactProperties(options?: {
217+
signal?: AbortSignal
218+
}): Promise<void> {
199219
const existingKeys = new Set<string>()
200220
let after: string | null = null
201221
let hasMore = false
@@ -204,7 +224,8 @@ export async function ensureNewsletterContactProperties(): Promise<void> {
204224
const query = new URLSearchParams({ limit: String(RESEND_CONTACT_PROPERTY_PAGE_LIMIT) })
205225
if (after) query.set('after', after)
206226
const response = await resendRequest<ResendContactPropertyListResponse>(
207-
`/contact-properties?${query.toString()}`
227+
`/contact-properties?${query.toString()}`,
228+
{ signal: options?.signal }
208229
)
209230

210231
for (const property of response.data ?? []) {
@@ -228,6 +249,7 @@ export async function ensureNewsletterContactProperties(): Promise<void> {
228249
await resendRequest('/contact-properties', {
229250
method: 'POST',
230251
body: { key, type: 'string' },
252+
signal: options?.signal,
231253
})
232254
} catch (error) {
233255
const message = getErrorMessage(error)
@@ -252,6 +274,7 @@ export async function createOrSegmentNewsletterContact(input: {
252274
userId: string | null
253275
runId: string
254276
segmentId: string
277+
signal?: AbortSignal
255278
}): Promise<{ status: 'created' | 'updated' | 'segment_added'; contactId?: string }> {
256279
const email = normalizeEmail(input.email)
257280
const name = splitName(input.name)
@@ -269,6 +292,7 @@ export async function createOrSegmentNewsletterContact(input: {
269292
},
270293
segments: [{ id: input.segmentId }],
271294
},
295+
signal: input.signal,
272296
})
273297
return { status: 'created', contactId: contact.id }
274298
} catch (error) {
@@ -286,10 +310,12 @@ export async function createOrSegmentNewsletterContact(input: {
286310
newsletter_run_id: input.runId,
287311
},
288312
},
313+
signal: input.signal,
289314
}
290315
)
291316
await resendRequest(`/contacts/${encodeURIComponent(email)}/segments/${input.segmentId}`, {
292317
method: 'POST',
318+
signal: input.signal,
293319
})
294320
return { status: 'updated', contactId: contact.id }
295321
}

0 commit comments

Comments
 (0)