Skip to content

Commit e57d3f7

Browse files
waleedlatif1waleedlatif1
authored
fix(schedules): disable schedules after consecutive failures (#368)
* fix(schedules): disable schedules after consecutive failures * acknowledged PR comments
1 parent 0b05562 commit e57d3f7

16 files changed

Lines changed: 2930 additions & 420 deletions

File tree

‎apps/sim/app/api/__test-utils__/utils.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,3 +319,111 @@ export async function getMockedDependencies() {
319319
db: dbModule.db,
320320
}
321321
}
322+
323+
export function mockScheduleStatusDb({
324+
schedule = [
325+
{
326+
id: 'schedule-id',
327+
workflowId: 'workflow-id',
328+
status: 'active',
329+
failedCount: 0,
330+
lastRanAt: new Date('2024-01-01T00:00:00.000Z'),
331+
lastFailedAt: null,
332+
nextRunAt: new Date('2024-01-02T00:00:00.000Z'),
333+
},
334+
],
335+
workflow = [
336+
{
337+
userId: 'user-id',
338+
},
339+
],
340+
}: {
341+
schedule?: any[]
342+
workflow?: any[]
343+
} = {}) {
344+
vi.doMock('@/db', () => {
345+
let callCount = 0
346+
347+
const select = vi.fn().mockImplementation(() => ({
348+
from: vi.fn().mockImplementation(() => ({
349+
where: vi.fn().mockImplementation(() => ({
350+
limit: vi.fn().mockImplementation(() => {
351+
callCount += 1
352+
if (callCount === 1) return schedule
353+
if (callCount === 2) return workflow
354+
return []
355+
}),
356+
})),
357+
})),
358+
}))
359+
360+
return {
361+
db: { select },
362+
}
363+
})
364+
}
365+
366+
export function mockScheduleExecuteDb({
367+
schedules = [] as any[],
368+
workflowRecord = {
369+
id: 'workflow-id',
370+
userId: 'user-id',
371+
state: sampleWorkflowState,
372+
},
373+
envRecord = {
374+
userId: 'user-id',
375+
variables: {
376+
OPENAI_API_KEY: 'encrypted:openai-api-key',
377+
SERPER_API_KEY: 'encrypted:serper-api-key',
378+
},
379+
},
380+
}: {
381+
schedules?: any[]
382+
workflowRecord?: any
383+
envRecord?: any
384+
}): void {
385+
vi.doMock('@/db', () => {
386+
const select = vi.fn().mockImplementation(() => ({
387+
from: vi.fn().mockImplementation((table: any) => {
388+
const tbl = String(table)
389+
if (tbl === 'workflow_schedule' || tbl === 'schedule') {
390+
return {
391+
where: vi.fn().mockImplementation(() => ({
392+
limit: vi.fn().mockImplementation(() => schedules),
393+
})),
394+
}
395+
}
396+
397+
if (tbl === 'workflow') {
398+
return {
399+
where: vi.fn().mockImplementation(() => ({
400+
limit: vi.fn().mockImplementation(() => [workflowRecord]),
401+
})),
402+
}
403+
}
404+
405+
if (tbl === 'environment') {
406+
return {
407+
where: vi.fn().mockImplementation(() => ({
408+
limit: vi.fn().mockImplementation(() => [envRecord]),
409+
})),
410+
}
411+
}
412+
413+
return {
414+
where: vi.fn().mockImplementation(() => ({
415+
limit: vi.fn().mockImplementation(() => []),
416+
})),
417+
}
418+
}),
419+
}))
420+
421+
const update = vi.fn().mockImplementation(() => ({
422+
set: vi.fn().mockImplementation(() => ({
423+
where: vi.fn().mockResolvedValue([]),
424+
})),
425+
}))
426+
427+
return { db: { select, update } }
428+
})
429+
}

‎apps/sim/app/api/schedules/[id]/route.ts‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { createLogger } from '@/lib/logs/console-logger'
66
import { db } from '@/db'
77
import { workflow, workflowSchedule } from '@/db/schema'
88

9-
const logger = createLogger('ScheduleDeleteAPI')
9+
const logger = createLogger('ScheduleAPI')
1010

1111
export const dynamic = 'force-dynamic'
1212

@@ -63,3 +63,88 @@ export async function DELETE(
6363
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
6464
}
6565
}
66+
67+
/**
68+
* Update a schedule - can be used to reactivate a disabled schedule
69+
*/
70+
export async function PUT(request: NextRequest, { params }: { params: Promise<{ id: string }> }) {
71+
const requestId = crypto.randomUUID().slice(0, 8)
72+
73+
try {
74+
const { id } = await params
75+
const scheduleId = id
76+
logger.debug(`[${requestId}] Updating schedule with ID: ${scheduleId}`)
77+
78+
const session = await getSession()
79+
if (!session?.user?.id) {
80+
logger.warn(`[${requestId}] Unauthorized schedule update attempt`)
81+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
82+
}
83+
84+
const body = await request.json()
85+
const { action } = body
86+
87+
const [schedule] = await db
88+
.select({
89+
id: workflowSchedule.id,
90+
workflowId: workflowSchedule.workflowId,
91+
status: workflowSchedule.status,
92+
})
93+
.from(workflowSchedule)
94+
.where(eq(workflowSchedule.id, scheduleId))
95+
.limit(1)
96+
97+
if (!schedule) {
98+
logger.warn(`[${requestId}] Schedule not found: ${scheduleId}`)
99+
return NextResponse.json({ error: 'Schedule not found' }, { status: 404 })
100+
}
101+
102+
const [workflowRecord] = await db
103+
.select({ userId: workflow.userId })
104+
.from(workflow)
105+
.where(eq(workflow.id, schedule.workflowId))
106+
.limit(1)
107+
108+
if (!workflowRecord) {
109+
logger.warn(`[${requestId}] Workflow not found for schedule: ${scheduleId}`)
110+
return NextResponse.json({ error: 'Workflow not found' }, { status: 404 })
111+
}
112+
113+
if (workflowRecord.userId !== session.user.id) {
114+
logger.warn(`[${requestId}] User not authorized to modify this schedule: ${scheduleId}`)
115+
return NextResponse.json({ error: 'Not authorized to modify this schedule' }, { status: 403 })
116+
}
117+
118+
if (action === 'reactivate' || (body.status && body.status === 'active')) {
119+
if (schedule.status === 'active') {
120+
return NextResponse.json({ message: 'Schedule is already active' }, { status: 200 })
121+
}
122+
123+
const now = new Date()
124+
const nextRunAt = new Date(now.getTime() + 60 * 1000) // Schedule to run in 1 minute
125+
126+
await db
127+
.update(workflowSchedule)
128+
.set({
129+
status: 'active',
130+
failedCount: 0,
131+
updatedAt: now,
132+
nextRunAt,
133+
})
134+
.where(eq(workflowSchedule.id, scheduleId))
135+
136+
logger.info(`[${requestId}] Reactivated schedule: ${scheduleId}`)
137+
138+
return NextResponse.json({
139+
message: 'Schedule activated successfully',
140+
nextRunAt,
141+
})
142+
}
143+
144+
logger.warn(`[${requestId}] Unsupported update action for schedule: ${scheduleId}`)
145+
return NextResponse.json({ error: 'Unsupported update action' }, { status: 400 })
146+
} catch (error) {
147+
logger.error(`[${requestId}] Error updating schedule`, error)
148+
return NextResponse.json({ error: 'Failed to update schedule' }, { status: 500 })
149+
}
150+
}

‎apps/sim/app/api/schedules/[id]/status/route.test.ts‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/**
2+
* Integration tests for schedule status API route
3+
*
4+
* @vitest-environment node
5+
*/
6+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
7+
import { createMockRequest, mockScheduleStatusDb } from '@/app/api/__test-utils__/utils'
8+
9+
// Common mocks
10+
const mockSchedule = {
11+
id: 'schedule-id',
12+
workflowId: 'workflow-id',
13+
status: 'active',
14+
failedCount: 0,
15+
lastRanAt: new Date('2024-01-01T00:00:00.000Z'),
16+
lastFailedAt: null,
17+
nextRunAt: new Date('2024-01-02T00:00:00.000Z'),
18+
}
19+
20+
beforeEach(() => {
21+
vi.resetModules()
22+
23+
vi.doMock('@/lib/logs/console-logger', () => ({
24+
createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }),
25+
}))
26+
27+
vi.doMock('crypto', () => ({
28+
randomUUID: vi.fn(() => 'test-uuid'),
29+
default: { randomUUID: vi.fn(() => 'test-uuid') },
30+
}))
31+
})
32+
33+
afterEach(() => {
34+
vi.clearAllMocks()
35+
})
36+
37+
describe('Schedule Status API Route', () => {
38+
it('returns schedule status successfully', async () => {
39+
mockScheduleStatusDb({}) // default mocks
40+
41+
vi.doMock('@/lib/auth', () => ({
42+
getSession: vi.fn().mockResolvedValue({ user: { id: 'user-id' } }),
43+
}))
44+
45+
const req = createMockRequest('GET')
46+
47+
const { GET } = await import('./route')
48+
49+
const res = await GET(req, { params: Promise.resolve({ id: 'schedule-id' }) })
50+
51+
expect(res.status).toBe(200)
52+
const data = await res.json()
53+
54+
expect(data).toMatchObject({
55+
status: 'active',
56+
failedCount: 0,
57+
nextRunAt: mockSchedule.nextRunAt.toISOString(),
58+
isDisabled: false,
59+
})
60+
})
61+
62+
it('marks disabled schedules with isDisabled = true', async () => {
63+
mockScheduleStatusDb({ schedule: [{ ...mockSchedule, status: 'disabled' }] })
64+
65+
vi.doMock('@/lib/auth', () => ({
66+
getSession: vi.fn().mockResolvedValue({ user: { id: 'user-id' } }),
67+
}))
68+
69+
const req = createMockRequest('GET')
70+
const { GET } = await import('./route')
71+
const res = await GET(req, { params: Promise.resolve({ id: 'schedule-id' }) })
72+
73+
expect(res.status).toBe(200)
74+
const data = await res.json()
75+
expect(data).toHaveProperty('status', 'disabled')
76+
expect(data).toHaveProperty('isDisabled', true)
77+
expect(data).toHaveProperty('lastFailedAt')
78+
})
79+
80+
it('returns 404 if schedule not found', async () => {
81+
mockScheduleStatusDb({ schedule: [] })
82+
83+
vi.doMock('@/lib/auth', () => ({
84+
getSession: vi.fn().mockResolvedValue({ user: { id: 'user-id' } }),
85+
}))
86+
87+
const req = createMockRequest('GET')
88+
const { GET } = await import('./route')
89+
const res = await GET(req, { params: Promise.resolve({ id: 'missing-id' }) })
90+
91+
expect(res.status).toBe(404)
92+
const data = await res.json()
93+
expect(data).toHaveProperty('error', 'Schedule not found')
94+
})
95+
96+
it('returns 404 if related workflow not found', async () => {
97+
mockScheduleStatusDb({ workflow: [] })
98+
99+
vi.doMock('@/lib/auth', () => ({
100+
getSession: vi.fn().mockResolvedValue({ user: { id: 'user-id' } }),
101+
}))
102+
103+
const req = createMockRequest('GET')
104+
const { GET } = await import('./route')
105+
const res = await GET(req, { params: Promise.resolve({ id: 'schedule-id' }) })
106+
107+
expect(res.status).toBe(404)
108+
const data = await res.json()
109+
expect(data).toHaveProperty('error', 'Workflow not found')
110+
})
111+
112+
it('returns 403 when user is not owner of workflow', async () => {
113+
mockScheduleStatusDb({ workflow: [{ userId: 'another-user' }] })
114+
115+
vi.doMock('@/lib/auth', () => ({
116+
getSession: vi.fn().mockResolvedValue({ user: { id: 'user-id' } }),
117+
}))
118+
119+
const req = createMockRequest('GET')
120+
const { GET } = await import('./route')
121+
const res = await GET(req, { params: Promise.resolve({ id: 'schedule-id' }) })
122+
123+
expect(res.status).toBe(403)
124+
const data = await res.json()
125+
expect(data).toHaveProperty('error', 'Not authorized to view this schedule')
126+
})
127+
128+
it('returns 401 when user is not authenticated', async () => {
129+
mockScheduleStatusDb({})
130+
131+
vi.doMock('@/lib/auth', () => ({
132+
getSession: vi.fn().mockResolvedValue(null),
133+
}))
134+
135+
const req = createMockRequest('GET')
136+
const { GET } = await import('./route')
137+
const res = await GET(req, { params: Promise.resolve({ id: 'schedule-id' }) })
138+
139+
expect(res.status).toBe(401)
140+
const data = await res.json()
141+
expect(data).toHaveProperty('error', 'Unauthorized')
142+
})
143+
})

0 commit comments

Comments
 (0)