fix(notifications): keep partial recipients when one lookup fails

Author: TheodoreSpeaks

apps/sim/lib/workflows/schedules/disable-notifications.test.ts

@@ -2,6 +2,7 @@
  * @vitest-environment node
  */
 import {
+  dbChainMockFns,
   queueTableRows,
   resetDbChainMock,
   resetUrlsMock,
@@ -194,14 +195,33 @@ describe('notifyScheduleAutoDisabled', () => {
     expect(sendEmailSpy).toHaveBeenCalledTimes(2)
   })
 
-  it('never throws when recipient resolution fails', async () => {
+  it('still emails the creator when the admin lookup fails', async () => {
     queueTableRows(schemaMock.workflowSchedule, [WORKFLOW_SCHEDULE_ROW])
     queueTableRows(schemaMock.user, [CREATOR])
     getUsersWithPermissionsMock.mockRejectedValue(new Error('db down'))
 
     await expect(
       notifyScheduleAutoDisabled({ scheduleId: 's-1', reason: 'consecutive_failures' })
     ).resolves.toBeUndefined()
-    expect(sendEmailSpy).not.toHaveBeenCalled()
+
+    expect(sendEmailSpy).toHaveBeenCalledTimes(1)
+    expect(sendEmailSpy).toHaveBeenCalledWith(
+      expect.objectContaining({ to: 'creator@example.com' })
+    )
+  })
+
+  it('still emails the admins when the creator lookup fails', async () => {
+    // First .limit() is the schedule row, second is the creator lookup.
+    dbChainMockFns.limit
+      .mockResolvedValueOnce([WORKFLOW_SCHEDULE_ROW])
+      .mockRejectedValueOnce(new Error('db down'))
+    getUsersWithPermissionsMock.mockResolvedValue([admin('admin-a@example.com')])
+
+    await notifyScheduleAutoDisabled({ scheduleId: 's-1', reason: 'consecutive_failures' })
+
+    expect(sendEmailSpy).toHaveBeenCalledTimes(1)
+    expect(sendEmailSpy).toHaveBeenCalledWith(
+      expect.objectContaining({ to: 'admin-a@example.com' })
+    )
   })
 })

apps/sim/lib/workflows/schedules/disable-notifications.ts

@@ -117,6 +117,10 @@ export async function notifyScheduleAutoDisabled(params: {
 /**
  * Creator first, then workspace admins. Deduped on lowercased email so someone
  * who is both only receives one message.
+ *
+ * The two lookups are independent and each failure is contained: losing one
+ * must not zero out the other, or a blip in either turns an auto-disable back
+ * into the silent event this notification exists to prevent.
  */
 async function resolveRecipients(
   ownerUserId: string | null,
@@ -134,19 +138,27 @@ async function resolveRecipients(
   }
 
   if (ownerUserId) {
-    const ownerRows = await db
-      .select({ email: user.email, name: user.name })
-      .from(user)
-      .where(eq(user.id, ownerUserId))
-      .limit(1)
-    add(ownerRows[0]?.email ?? null, ownerRows[0]?.name ?? null)
+    try {
+      const ownerRows = await db
+        .select({ email: user.email, name: user.name })
+        .from(user)
+        .where(eq(user.id, ownerUserId))
+        .limit(1)
+      add(ownerRows[0]?.email ?? null, ownerRows[0]?.name ?? null)
+    } catch (error) {
+      logger.error('Failed to resolve schedule creator for disable email', error, { ownerUserId })
+    }
   }
 
   if (workspaceId) {
-    const members = await getUsersWithPermissions(workspaceId)
-    for (const member of members) {
-      if (member.permissionType !== 'admin') continue
-      add(member.email, member.name)
+    try {
+      const members = await getUsersWithPermissions(workspaceId)
+      for (const member of members) {
+        if (member.permissionType !== 'admin') continue
+        add(member.email, member.name)
+      }
+    } catch (error) {
+      logger.error('Failed to resolve workspace admins for disable email', error, { workspaceId })
     }
   }