Skip to content

Commit 7724a9b

Browse files
committed
cleanup, edge cases
1 parent 1738bcc commit 7724a9b

File tree

10 files changed

+815
-209
lines changed

10 files changed

+815
-209
lines changed

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

Lines changed: 376 additions & 10 deletions
Large diffs are not rendered by default.

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { z } from 'zod'
66
import { getSession } from '@/lib/auth'
77
import { generateRequestId } from '@/lib/core/utils/request'
88
import { createLogger } from '@/lib/logs/console/logger'
9+
import { validateCronExpression } from '@/lib/workflows/schedules/utils'
910
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1011

1112
const logger = createLogger('ScheduleAPI')
@@ -44,6 +45,8 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{
4445
id: workflowSchedule.id,
4546
workflowId: workflowSchedule.workflowId,
4647
status: workflowSchedule.status,
48+
cronExpression: workflowSchedule.cronExpression,
49+
timezone: workflowSchedule.timezone,
4750
})
4851
.from(workflowSchedule)
4952
.where(eq(workflowSchedule.id, scheduleId))
@@ -85,8 +88,19 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{
8588
return NextResponse.json({ message: 'Schedule is already active' }, { status: 200 })
8689
}
8790

91+
if (!schedule.cronExpression) {
92+
logger.error(`[${requestId}] Schedule has no cron expression: ${scheduleId}`)
93+
return NextResponse.json({ error: 'Schedule has no cron expression' }, { status: 400 })
94+
}
95+
96+
const cronResult = validateCronExpression(schedule.cronExpression, schedule.timezone || 'UTC')
97+
if (!cronResult.isValid || !cronResult.nextRun) {
98+
logger.error(`[${requestId}] Invalid cron expression for schedule: ${scheduleId}`)
99+
return NextResponse.json({ error: 'Schedule has invalid cron expression' }, { status: 400 })
100+
}
101+
88102
const now = new Date()
89-
const nextRunAt = new Date(now.getTime() + 60 * 1000) // Schedule to run in 1 minute
103+
const nextRunAt = cronResult.nextRun
90104

91105
await db
92106
.update(workflowSchedule)

apps/sim/app/api/workflows/[id]/deploy/route.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
createSchedulesForDeploy,
99
deleteSchedulesForWorkflow,
1010
validateWorkflowSchedules,
11-
} from '@/lib/workflows/schedules/deploy'
11+
} from '@/lib/workflows/schedules'
1212
import { validateWorkflowPermissions } from '@/lib/workflows/utils'
1313
import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
1414

@@ -103,20 +103,17 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
103103
return createErrorResponse(error.message, error.status)
104104
}
105105

106-
// Attribution: this route is UI-only; require session user as actor
107106
const actorUserId: string | null = session?.user?.id ?? null
108107
if (!actorUserId) {
109108
logger.warn(`[${requestId}] Unable to resolve actor user for workflow deployment: ${id}`)
110109
return createErrorResponse('Unable to determine deploying user', 400)
111110
}
112111

113-
// Load current workflow state to validate schedule blocks before deployment
114112
const normalizedData = await loadWorkflowFromNormalizedTables(id)
115113
if (!normalizedData) {
116114
return createErrorResponse('Failed to load workflow state', 500)
117115
}
118116

119-
// Validate schedule blocks before deploying
120117
const scheduleValidation = validateWorkflowSchedules(normalizedData.blocks)
121118
if (!scheduleValidation.isValid) {
122119
logger.warn(
@@ -137,11 +134,9 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
137134

138135
const deployedAt = deployResult.deployedAt!
139136

140-
// Create or update schedules for the deployed workflow
141137
let scheduleInfo: { scheduleId?: string; cronExpression?: string; nextRunAt?: Date } = {}
142138
const scheduleResult = await createSchedulesForDeploy(id, normalizedData.blocks, db)
143139
if (!scheduleResult.success) {
144-
// Log but don't fail deployment - schedule creation is secondary
145140
logger.error(
146141
`[${requestId}] Failed to create schedule for workflow ${id}: ${scheduleResult.error}`
147142
)
@@ -202,7 +197,6 @@ export async function DELETE(
202197
}
203198

204199
await db.transaction(async (tx) => {
205-
// Delete all schedules for this workflow
206200
await deleteSchedulesForWorkflow(id, tx)
207201

208202
await tx
@@ -218,7 +212,6 @@ export async function DELETE(
218212

219213
logger.info(`[${requestId}] Workflow undeployed successfully: ${id}`)
220214

221-
// Track workflow undeployment
222215
try {
223216
const { trackPlatformEvent } = await import('@/lib/core/telemetry')
224217
trackPlatformEvent('platform.workflow.undeployed', {

apps/sim/background/schedule-execution.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,7 @@ export async function executeScheduleJob(payload: ScheduleExecutionPayload) {
573573
updatedAt: now,
574574
nextRunAt,
575575
failedCount: 0,
576+
lastQueuedAt: null,
576577
},
577578
requestId,
578579
`Error updating schedule ${payload.scheduleId} after success`,

apps/sim/lib/workflows/schedules/deploy.test.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,19 +61,13 @@ vi.mock('./utils', async (importOriginal) => {
6161
}
6262
})
6363

64-
// Mock crypto.randomUUID
6564
vi.stubGlobal('crypto', {
6665
randomUUID: mockRandomUUID,
6766
})
6867

69-
import {
70-
createSchedulesForDeploy,
71-
deleteSchedulesForWorkflow,
72-
findScheduleBlocks,
73-
validateScheduleBlock,
74-
validateWorkflowSchedules,
75-
} from './deploy'
68+
import { createSchedulesForDeploy, deleteSchedulesForWorkflow } from './deploy'
7669
import type { BlockState } from './utils'
70+
import { findScheduleBlocks, validateScheduleBlock, validateWorkflowSchedules } from './validation'
7771

7872
describe('Schedule Deploy Utilities', () => {
7973
beforeEach(() => {

apps/sim/lib/workflows/schedules/deploy.ts

Lines changed: 2 additions & 180 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,8 @@ import { eq } from 'drizzle-orm'
55
import type { PgTransaction } from 'drizzle-orm/pg-core'
66
import type { PostgresJsQueryResultHKT } from 'drizzle-orm/postgres-js'
77
import { createLogger } from '@/lib/logs/console/logger'
8-
import {
9-
type BlockState,
10-
calculateNextRunTime,
11-
generateCronExpression,
12-
getScheduleTimeValues,
13-
getSubBlockValue,
14-
validateCronExpression,
15-
} from './utils'
8+
import type { BlockState } from '@/lib/workflows/schedules/utils'
9+
import { findScheduleBlocks, validateScheduleBlock } from '@/lib/workflows/schedules/validation'
1610

1711
const logger = createLogger('ScheduleDeployUtils')
1812

@@ -28,18 +22,6 @@ type DbOrTx =
2822
ExtractTablesWithRelations<typeof schema>
2923
>
3024

31-
/**
32-
* Result of schedule validation
33-
*/
34-
export interface ScheduleValidationResult {
35-
isValid: boolean
36-
error?: string
37-
scheduleType?: string
38-
cronExpression?: string
39-
nextRunAt?: Date
40-
timezone?: string
41-
}
42-
4325
/**
4426
* Result of schedule creation during deploy
4527
*/
@@ -52,166 +34,6 @@ export interface ScheduleDeployResult {
5234
timezone?: string
5335
}
5436

55-
/**
56-
* Check if a time value is valid (not empty/null/undefined)
57-
*/
58-
function isValidTimeValue(value: string | null | undefined): boolean {
59-
return !!value && value.trim() !== '' && value.includes(':')
60-
}
61-
62-
/**
63-
* Check if a schedule type has valid configuration
64-
* Uses raw block values to avoid false positives from default parsing
65-
*/
66-
function hasValidScheduleConfig(scheduleType: string | undefined, block: BlockState): boolean {
67-
switch (scheduleType) {
68-
case 'minutes': {
69-
const rawValue = getSubBlockValue(block, 'minutesInterval')
70-
const numValue = Number(rawValue)
71-
return !!rawValue && !Number.isNaN(numValue) && numValue >= 1 && numValue <= 1440
72-
}
73-
case 'hourly': {
74-
const rawValue = getSubBlockValue(block, 'hourlyMinute')
75-
const numValue = Number(rawValue)
76-
return rawValue !== '' && !Number.isNaN(numValue) && numValue >= 0 && numValue <= 59
77-
}
78-
case 'daily': {
79-
const rawTime = getSubBlockValue(block, 'dailyTime')
80-
return isValidTimeValue(rawTime)
81-
}
82-
case 'weekly': {
83-
const rawDay = getSubBlockValue(block, 'weeklyDay')
84-
const rawTime = getSubBlockValue(block, 'weeklyDayTime')
85-
return !!rawDay && isValidTimeValue(rawTime)
86-
}
87-
case 'monthly': {
88-
const rawDay = getSubBlockValue(block, 'monthlyDay')
89-
const rawTime = getSubBlockValue(block, 'monthlyTime')
90-
const dayNum = Number(rawDay)
91-
return (
92-
!!rawDay &&
93-
!Number.isNaN(dayNum) &&
94-
dayNum >= 1 &&
95-
dayNum <= 31 &&
96-
isValidTimeValue(rawTime)
97-
)
98-
}
99-
case 'custom':
100-
return !!getSubBlockValue(block, 'cronExpression')
101-
default:
102-
return false
103-
}
104-
}
105-
106-
/**
107-
* Get human-readable error message for missing schedule configuration
108-
*/
109-
function getMissingConfigError(scheduleType: string): string {
110-
switch (scheduleType) {
111-
case 'minutes':
112-
return 'Minutes interval is required for minute-based schedules'
113-
case 'hourly':
114-
return 'Minute value is required for hourly schedules'
115-
case 'daily':
116-
return 'Time is required for daily schedules'
117-
case 'weekly':
118-
return 'Day and time are required for weekly schedules'
119-
case 'monthly':
120-
return 'Day and time are required for monthly schedules'
121-
case 'custom':
122-
return 'Cron expression is required for custom schedules'
123-
default:
124-
return 'Schedule type is required'
125-
}
126-
}
127-
128-
/**
129-
* Find schedule blocks in a workflow's blocks
130-
*/
131-
export function findScheduleBlocks(blocks: Record<string, BlockState>): BlockState[] {
132-
return Object.values(blocks).filter((block) => block.type === 'schedule')
133-
}
134-
135-
/**
136-
* Validate a schedule block's configuration
137-
* Returns validation result with error details if invalid
138-
*/
139-
export function validateScheduleBlock(block: BlockState): ScheduleValidationResult {
140-
const scheduleType = getSubBlockValue(block, 'scheduleType')
141-
142-
if (!scheduleType) {
143-
return {
144-
isValid: false,
145-
error: 'Schedule type is required',
146-
}
147-
}
148-
149-
const hasValidConfig = hasValidScheduleConfig(scheduleType, block)
150-
151-
if (!hasValidConfig) {
152-
return {
153-
isValid: false,
154-
error: getMissingConfigError(scheduleType),
155-
scheduleType,
156-
}
157-
}
158-
159-
const timezone = getSubBlockValue(block, 'timezone') || 'UTC'
160-
161-
try {
162-
// Get parsed schedule values (safe to use after validation passes)
163-
const scheduleValues = getScheduleTimeValues(block)
164-
const sanitizedScheduleValues =
165-
scheduleType !== 'custom' ? { ...scheduleValues, cronExpression: null } : scheduleValues
166-
167-
const cronExpression = generateCronExpression(scheduleType, sanitizedScheduleValues)
168-
169-
const validation = validateCronExpression(cronExpression, timezone)
170-
if (!validation.isValid) {
171-
return {
172-
isValid: false,
173-
error: `Invalid cron expression: ${validation.error}`,
174-
scheduleType,
175-
}
176-
}
177-
178-
const nextRunAt = calculateNextRunTime(scheduleType, sanitizedScheduleValues)
179-
180-
return {
181-
isValid: true,
182-
scheduleType,
183-
cronExpression,
184-
nextRunAt,
185-
timezone,
186-
}
187-
} catch (error) {
188-
return {
189-
isValid: false,
190-
error: error instanceof Error ? error.message : 'Failed to generate schedule',
191-
scheduleType,
192-
}
193-
}
194-
}
195-
196-
/**
197-
* Validate all schedule blocks in a workflow
198-
* Returns the first validation error found, or success if all are valid
199-
*/
200-
export function validateWorkflowSchedules(
201-
blocks: Record<string, BlockState>
202-
): ScheduleValidationResult {
203-
const scheduleBlocks = findScheduleBlocks(blocks)
204-
205-
for (const block of scheduleBlocks) {
206-
const result = validateScheduleBlock(block)
207-
if (!result.isValid) {
208-
return result
209-
}
210-
}
211-
212-
return { isValid: true }
213-
}
214-
21537
/**
21638
* Create or update schedule records for a workflow during deployment
21739
* This should be called within a database transaction
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
export {
2+
createSchedulesForDeploy,
3+
deleteSchedulesForWorkflow,
4+
type ScheduleDeployResult,
5+
} from './deploy'
6+
export {
7+
type BlockState,
8+
calculateNextRunTime,
9+
DAY_MAP,
10+
generateCronExpression,
11+
getScheduleInfo,
12+
getScheduleTimeValues,
13+
getSubBlockValue,
14+
parseCronToHumanReadable,
15+
parseTimeString,
16+
validateCronExpression,
17+
} from './utils'
18+
export {
19+
findScheduleBlocks,
20+
type ScheduleValidationResult,
21+
validateScheduleBlock,
22+
validateWorkflowSchedules,
23+
} from './validation'

0 commit comments

Comments
 (0)