Error when recurring tasks are invalid

Resolves rails#414
evdevdev · Dec 10, 2024 · 497e1fb · 497e1fb
1 parent 012e4f8
commit 497e1fb
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 7 deletions.
diff --git a/lib/solid_queue/configuration.rb b/lib/solid_queue/configuration.rb
@@ -36,6 +36,14 @@ def configured_processes
       end
     end
 
+    def valid?
+      skip_recurring_tasks? || recurring_tasks.none? || recurring_tasks.all?(&:valid?)
+    end
+
+    def invalid_tasks
+      recurring_tasks.select(&:invalid?)
+    end
+
     def max_number_of_threads
       # At most "threads" in each worker + 1 thread for the worker + 1 thread for the heartbeat task
       workers_options.map { |options| options[:threads] }.max + 2
@@ -100,7 +108,7 @@ def dispatchers_options
       def recurring_tasks
         @recurring_tasks ||= recurring_tasks_config.map do |id, options|
           RecurringTask.from_configuration(id, **options)
-        end.select(&:valid?)
+        end
       end
 
       def processes_config

diff --git a/lib/solid_queue/supervisor.rb b/lib/solid_queue/supervisor.rb
@@ -11,11 +11,27 @@ def start(**options)
         configuration = Configuration.new(**options)
 
         if configuration.configured_processes.any?
-          new(configuration).tap(&:start)
+          if configuration.valid?
+            new(configuration).tap(&:start)
+          else
+            abort_due_to_invalid_tasks(configuration)
+          end
         else
           abort "No workers or processed configured. Exiting..."
         end
       end
+
+      private
+        def abort_due_to_invalid_tasks(configuration)
+          error_messages = configuration.invalid_tasks
+            .map do |task|
+              all_messages = task.errors.full_messages.map { |msg| "\t#{msg}" }.join("\n")
+              "#{task.key}:\n#{all_messages}"
+            end
+            .join("\n")
+
+          abort "Invalid processes configured:\n#{error_messages}"
+        end
     end
 
     def initialize(configuration)

diff --git a/test/dummy/config/recurring_with_invalid.yml b/test/dummy/config/recurring_with_invalid.yml
@@ -0,0 +1,5 @@
+periodic_store_result:
+    class: StoreResultJorrrrrrb
+    queue: default
+    args: [42, { status: "custom_status" }]
+    schedule: every second
diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -28,7 +28,7 @@ class ExpectedTestError < RuntimeError; end
 
 
 class ActiveSupport::TestCase
-  include ProcessesTestHelper, JobsTestHelper
+  include ConfigurationTestHelper, ProcessesTestHelper, JobsTestHelper
 
   setup do
     # Could be cleaner with one several minitest gems, but didn't want to add new dependency

diff --git a/test/test_helpers/configuration_test_helper.rb b/test/test_helpers/configuration_test_helper.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module ConfigurationTestHelper
+  def config_file_path(name)
+    Rails.root.join("config/#{name}.yml")
+  end
+end
diff --git a/test/test_helpers/processes_test_helper.rb b/test/test_helpers/processes_test_helper.rb
@@ -7,6 +7,16 @@ def run_supervisor_as_fork(**options)
     end
   end
 
+  def run_supervisor_as_fork_with_captured_io(**options)
+    pid = nil
+    out, err = capture_subprocess_io do
+      pid = run_supervisor_as_fork(**options)
+      wait_for_registered_processes(4)
+    end
+
+    [ pid, out, err ]
+  end
+
   def wait_for_registered_processes(count, timeout: 1.second)
     wait_while_with_timeout(timeout) { SolidQueue::Process.count != count }
   end

diff --git a/test/unit/configuration_test.rb b/test/unit/configuration_test.rb
@@ -90,6 +90,26 @@ class ConfigurationTest < ActiveSupport::TestCase
     assert_processes configuration, :dispatcher, 1, polling_interval: 0.1, recurring_tasks: nil
   end
 
+  test "detects when there are invalid recurring tasks" do
+    configuration = SolidQueue::Configuration.new(recurring_schedule_file: config_file_path(:recurring_with_invalid))
+
+    assert_not configuration.valid?
+
+    assert_equal configuration.invalid_tasks.size, 1
+  end
+
+  test "is valid when there are no recurring tasks" do
+    configuration = SolidQueue::Configuration.new(recurring_schedule_file: config_file_path(:empty_recurring))
+
+    assert configuration.valid?
+  end
+
+  test "is valid when recurring tasks are skipped" do
+    configuration = SolidQueue::Configuration.new(skip_recurring: true)
+
+    assert configuration.valid?
+  end
+
   private
     def assert_processes(configuration, kind, count, **attributes)
       processes = configuration.configured_processes.select { |p| p.kind == kind }
@@ -121,8 +141,4 @@ def assert_equal_value(expected_value, value)
         assert_equal expected_value, value
       end
     end
-
-    def config_file_path(name)
-      Rails.root.join("config/#{name}.yml")
-    end
 end
diff --git a/test/unit/supervisor_test.rb b/test/unit/supervisor_test.rb
@@ -48,6 +48,16 @@ class SupervisorTest < ActiveSupport::TestCase
     assert_not process_exists?(pid)
   end
 
+  test "start with invalid configuration" do
+    pid, _out, err = run_supervisor_as_fork_with_captured_io(recurring_schedule_file: config_file_path(:recurring_with_invalid), skip_recurring: false)
+
+    sleep(0.5)
+    assert_no_registered_processes
+
+    assert_not process_exists?(pid)
+    assert_match %r{Invalid processes configured}, err
+  end
+
   test "create and delete pidfile" do
     assert_not File.exist?(@pidfile)