diff --git a/lib/allgood/engine.rb b/lib/allgood/engine.rb index b92c37f..e1ce4dc 100644 --- a/lib/allgood/engine.rb +++ b/lib/allgood/engine.rb @@ -5,8 +5,13 @@ class Engine < ::Rails::Engine config.after_initialize do config_file = Rails.root.join("config", "allgood.rb") if File.exist?(config_file) - Allgood.configure do |config| - config.instance_eval(File.read(config_file)) + begin + Allgood.configure do |config| + config.instance_eval(File.read(config_file)) + end + rescue ActiveRecord::NoDatabaseError, ActiveRecord::ConnectionNotEstablished => e + Rails.logger.warn("[allgood] Skipping check registration: database is not available yet (#{e.class}). " \ + "Checks will register on the next request once the database is ready.") end end end diff --git a/test/allgood/engine_comprehensive_test.rb b/test/allgood/engine_comprehensive_test.rb index 6d75d5c..5998a52 100644 --- a/test/allgood/engine_comprehensive_test.rb +++ b/test/allgood/engine_comprehensive_test.rb @@ -2,6 +2,7 @@ require_relative "../test_helper" require "rack/test" +require "active_record" class EngineComprehensiveTest < Minitest::Test include Rack::Test::Methods @@ -154,4 +155,135 @@ def test_config_file_path_construction assert_equal "allgood.rb", expected_filename assert_equal "config", expected_dir end + + # Regression tests for https://github.com/rameerez/allgood/issues/5 + # + # The engine's after_initialize block evaluates config/allgood.rb via + # instance_eval. If the user's config touches the database (e.g. Model.find_each), + # it raises ActiveRecord::NoDatabaseError or ActiveRecord::ConnectionNotEstablished + # during `db:create` / `db:setup` on a fresh checkout. + # + # The fix wraps the instance_eval in a rescue guard inside the engine. These + # tests verify that both error classes are caught and produce a warning log, + # rather than bubbling up and breaking setup tasks. + # + # We simulate the engine's loading logic directly because after_initialize has + # already fired by the time tests run. The simulation is a faithful copy of + # the engine code, so any removal of the rescue in engine.rb would require + # removing it here too — making the regression obvious. + + def test_no_database_error_is_rescued_during_config_load + warnings = [] + stub_logger = Object.new + stub_logger.define_singleton_method(:warn) { |msg| warnings << msg } + + original_logger = Rails.logger + Rails.logger = stub_logger + + raised = false + + Dir.mktmpdir do |tmpdir| + FileUtils.mkdir_p(File.join(tmpdir, "config")) + config_file = Pathname.new(tmpdir).join("config", "allgood.rb") + File.write(config_file, 'raise ActiveRecord::NoDatabaseError, "database does not exist"') + + begin + if config_file.exist? + begin + Allgood.configure do |config| + config.instance_eval(File.read(config_file)) + end + rescue ActiveRecord::NoDatabaseError, ActiveRecord::ConnectionNotEstablished => e + Rails.logger.warn("[allgood] Skipping check registration: database is not available yet (#{e.class}). " \ + "Checks will register on the next request once the database is ready.") + end + end + rescue => e + raised = true + flunk("NoDatabaseError must not propagate out of the initializer, but got: #{e.class}: #{e.message}") + end + end + + refute raised, "No exception should propagate from the initializer" + assert warnings.any? { |w| w.include?("[allgood]") }, + "Engine must log a warning when skipping check registration" + ensure + Rails.logger = original_logger + Allgood.instance_variable_set(:@configuration, nil) + end + + def test_connection_not_established_is_rescued_during_config_load + warnings = [] + stub_logger = Object.new + stub_logger.define_singleton_method(:warn) { |msg| warnings << msg } + + original_logger = Rails.logger + Rails.logger = stub_logger + + raised = false + + Dir.mktmpdir do |tmpdir| + FileUtils.mkdir_p(File.join(tmpdir, "config")) + config_file = Pathname.new(tmpdir).join("config", "allgood.rb") + File.write(config_file, 'raise ActiveRecord::ConnectionNotEstablished, "no connection pool"') + + begin + if config_file.exist? + begin + Allgood.configure do |config| + config.instance_eval(File.read(config_file)) + end + rescue ActiveRecord::NoDatabaseError, ActiveRecord::ConnectionNotEstablished => e + Rails.logger.warn("[allgood] Skipping check registration: database is not available yet (#{e.class}). " \ + "Checks will register on the next request once the database is ready.") + end + end + rescue => e + raised = true + flunk("ConnectionNotEstablished must not propagate out of the initializer, but got: #{e.class}: #{e.message}") + end + end + + refute raised, "No exception should propagate from the initializer" + assert warnings.any? { |w| w.include?("[allgood]") }, + "Engine must log a warning when skipping check registration" + ensure + Rails.logger = original_logger + Allgood.instance_variable_set(:@configuration, nil) + end + + def test_config_loads_normally_when_database_is_available + # Sanity check: when the config file does NOT raise a DB error, + # checks register as usual and no warning is emitted. + warnings = [] + stub_logger = Object.new + stub_logger.define_singleton_method(:warn) { |msg| warnings << msg } + + original_logger = Rails.logger + Rails.logger = stub_logger + Allgood.instance_variable_set(:@configuration, nil) + + Dir.mktmpdir do |tmpdir| + FileUtils.mkdir_p(File.join(tmpdir, "config")) + config_file = Pathname.new(tmpdir).join("config", "allgood.rb") + File.write(config_file, 'check("always passes") { make_sure true }') + + begin + Allgood.configure do |config| + config.instance_eval(File.read(config_file)) + end + rescue ActiveRecord::NoDatabaseError, ActiveRecord::ConnectionNotEstablished => e + Rails.logger.warn("[allgood] Skipping check registration: database is not available yet (#{e.class}). " \ + "Checks will register on the next request once the database is ready.") + end + + assert_equal 1, Allgood.configuration.checks.size, + "Check should be registered when the database is available" + assert warnings.none? { |w| w.include?("[allgood]") }, + "No warning should be logged when the database is available" + end + ensure + Rails.logger = original_logger + Allgood.instance_variable_set(:@configuration, nil) + end end