From d23e35409a2cdb68645f1afa5a1d13bc58e8c7c2 Mon Sep 17 00:00:00 2001 From: Himanshu Kale Date: Thu, 15 Jan 2026 23:15:45 +0530 Subject: [PATCH] Fix: Detect and warn when schedules generate multiple CRONs Fixes #703 - Use Fugit::Nat.parse with multi: :fail to detect schedules that generate multiple CRONs - Raise validation error with clear message when multiple CRONs detected - Add tests for single CRON, multiple times (same/different minutes) - Prevents silent failures where only first CRON was used --- app/models/solid_queue/recurring_task.rb | 15 +++++++++++++-- .../models/solid_queue/recurring_task_test.rb | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/app/models/solid_queue/recurring_task.rb b/app/models/solid_queue/recurring_task.rb index 2a776c8a..f2f6fbac 100644 --- a/app/models/solid_queue/recurring_task.rb +++ b/app/models/solid_queue/recurring_task.rb @@ -103,7 +103,10 @@ def attributes_for_upsert private def supported_schedule - unless parsed_schedule.instance_of?(Fugit::Cron) + if parsed_schedule == :multiple_crons_detected + errors.add :schedule, :multiple_crons, + message: "generates multiple cron schedules. Please use separate recurring tasks for each schedule, or use explicit cron syntax (e.g., '40 0,15 * * *' for multiple times with the same minutes)" + elsif !parsed_schedule.instance_of?(Fugit::Cron) errors.add :schedule, :unsupported, message: "is not a supported recurring schedule" end end @@ -152,7 +155,15 @@ def arguments_with_kwargs def parsed_schedule - @parsed_schedule ||= Fugit.parse(schedule) + @parsed_schedule ||= begin + Fugit::Nat.parse(schedule, multi: :fail) || Fugit.parse(schedule) + rescue ArgumentError => e + if e.message.include?("multiple crons") + :multiple_crons_detected + else + raise + end + end end def job_class diff --git a/test/models/solid_queue/recurring_task_test.rb b/test/models/solid_queue/recurring_task_test.rb index 29e946c5..2de91d8c 100644 --- a/test/models/solid_queue/recurring_task_test.rb +++ b/test/models/solid_queue/recurring_task_test.rb @@ -144,6 +144,25 @@ def perform assert_not SolidQueue::RecurringTask.new(key: "task-id", class_name: "SolidQueue::RecurringTaskTest::JobWithoutArguments").valid? end + test "schedules with multiple cron detection" do + task = recurring_task_with(class_name: "JobWithoutArguments", schedule: "every day at 00:40") + assert task.valid? + assert task.to_s.ends_with? "[ 40 0 * * * ]" + + task = recurring_task_with(class_name: "JobWithoutArguments", schedule: "every day at 00:40 and 15:40") + assert task.valid? + assert task.to_s.ends_with? "[ 40 0,15 * * * ]" + + task = recurring_task_with(class_name: "JobWithoutArguments", schedule: "every day at 00:40 and 15:20") + assert_not task.valid? + error_message = task.errors[:schedule].first + assert_includes error_message, "generates multiple cron schedules" + assert_includes error_message, "Please use separate recurring tasks" + + task = recurring_task_with(class_name: "JobWithoutArguments", schedule: "40 0 * * *") + assert task.valid? + end + test "valid and invalid job class and command" do # Command assert recurring_task_with(command: "puts '¡hola!'").valid?