Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 35 additions & 6 deletions packages/features/ee/workflows/api/scheduleSMSReminders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,19 @@ async function handler(req: NextApiRequest, res: NextApiResponse) {
//delete all scheduled sms reminders where scheduled date is past current date
await prisma.workflowReminder.deleteMany({
where: {
method: WorkflowMethods.SMS,
scheduledDate: {
lte: dayjs().toISOString(),
},
OR: [
{
method: WorkflowMethods.SMS,
scheduledDate: {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 CRITICAL

The new deleteMany predicate is too broad: retryCount > 1 is ORed with the SMS/scheduledDate filter, so any reminder of any method with a retry count above 1 will be deleted, even if it is still scheduled in the future. The root cause is that the retry cleanup condition was added at the top level instead of being scoped to the SMS reminder criteria.


re-entry.ai

lte: dayjs().toISOString(),
},
},
{
retryCount: {
gt: 1,
},
},
],
},
});

Expand All @@ -44,8 +53,11 @@ async function handler(req: NextApiRequest, res: NextApiResponse) {
lte: dayjs().add(7, "day").toISOString(),
},
},
select,
})) as PartialWorkflowReminder[];
select: {
...select,
retryCount: true,
},
})) as (PartialWorkflowReminder & { retryCount: number })[];

if (!unscheduledReminders.length) {
res.json({ ok: true });
Expand Down Expand Up @@ -163,9 +175,26 @@ async function handler(req: NextApiRequest, res: NextApiResponse) {
referenceId: scheduledSMS.sid,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ WARNING

retryCount is incremented both in the normal else branch and again in the catch block. If the code path reaches the else branch and then later throws during the same reminder iteration, the same reminder can be counted twice, which will prematurely trip the new cleanup rule. This is a correctness bug in the retry accounting flow.


re-entry.ai

},
});
} else {
await prisma.workflowReminder.update({
where: {
id: reminder.id,
},
data: {
retryCount: reminder.retryCount + 1,
},
});
}
}
} catch (error) {
await prisma.workflowReminder.update({
where: {
id: reminder.id,
},
data: {
retryCount: reminder.retryCount + 1,
},
});
console.log(`Error scheduling SMS with error ${error}`);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- AlterTable
ALTER TABLE "WorkflowReminder" ADD COLUMN "retryCount" INTEGER NOT NULL DEFAULT 0;
1 change: 1 addition & 0 deletions packages/prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,7 @@ model WorkflowReminder {
cancelled Boolean?
seatReferenceId String?
isMandatoryReminder Boolean? @default(false)
retryCount Int @default(0)

@@index([bookingUid])
@@index([workflowStepId])
Expand Down