Zeitwerk, omniauth and multiple DBs (connects_to)

[fragkakis]

Hello and thank you for zeitwerk.

I am upgrading our Rails 6.1 application to use zeitwerk and am running into a strange issue that won't let me run specific migrations. The involved parts as far as I can tell are zeitwerk, multiple connections with connects_to and omniauth (for which I have read #143, but don't see how it's related).

Any suggestions are welcome.

The classes

In omniauth, we are using a custom strategy:

# config/initializers/omniauth.rb
provider(OmniAuth::Strategies::ShieldOIDC, 
  ...
})

The OmniAuth::Strategies::ShieldOIDC then uses constants defined in the following module:

# lib/shield_settings.rb
module ShieldSettings
  SHIELD_TO_ADDITIONAL_ROLES_MAPPING = {
    ROLE_HRIS_EMPLOYEE => Member::HRIS_ROLE_EMPLOYEE,
    ROLE_HRIS_ADMIN => Member::HRIS_ROLE_ADMIN
  }
  ...
end

Member is a model, which inherits from ApplicationRecord, which contains the connects_to:

# app/models/application_record.rb
class ApplicationRecord < ActiveRecord::Base
  include FineGrainedValidations
  include PolicyScopeSupport

  self.abstract_class = true
  self.ignored_columns = [:__fake_column__]

  connects_to database: {writing: :primary, reporting: :reporting}
end

The problem

When I run the migrations, all migrations run successfully, but I get:

2023-09-15 10:54:18.004987 W [69836:10220] Rails -- DEPRECATION WARNING: Initialization autoloaded the constants FineGrainedValidations, PolicyScopeSupport, ApplicationRecord, Sanitizable, FavoriteJobs, MemberJobs, WorkableUtils, WorkableUtils::InitialsExtractor, DefaultSettings, RecruitingAdmin, Syncable, Emailable, Sampleable, EmailValidator, and Member.

In my understanding, these are from lib/shield_settings.rb and in particular the Member:: prefix, which cases the Member to be loaded during initialization, which is deprecated.

Instead, I defined the constants I was retrieving from Member within lib/shield_settings.rb, so that I am not loading the model during initialization. Now ~30 migrations pass and the following fails:

class AddLogoToAccounts < ActiveRecord::Migration[4.2]
  def up
    add_column :accounts, :logo, :string
    Account.all.each do |account|
      account.remote_logo_url = account["logo_url"] if account["logo_url"].present?
      account.save
    end
    remove_column :accounts, :logo_url
  end

with:

== 20120829175352 AddLogoToAccounts: migrating ================================
2023-09-15 11:10:00.999778 I [71225:10220] ActiveRecord -- Migrating to AddLogoToAccounts (20120829175352)
-- add_column(:accounts, :logo, :string)
   -> 0.0008s
-- remove_column(:accounts, :logo_url)
rails aborted!
StandardError: An error has occurred, this and all later migrations canceled:

connection is closed
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'

Caused by:
ActiveRecord::ConnectionNotEstablished: connection is closed
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'

Caused by:
PG::ConnectionBad: connection is closed
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'

Caused by:
ActiveRecord::ConnectionNotEstablished: connection is closed
/Users/markosfragkakis/dev/workable/db/migrate/20120829175352_add_logo_to_accounts.rb:8:in `up'
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'

Caused by:
PG::ConnectionBad: connection is closed
/Users/markosfragkakis/dev/workable/db/migrate/20120829175352_add_logo_to_accounts.rb:8:in `up'
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'
Tasks: TOP => db:migrate
(See full trace by running task with --trace)

Observations

Observation 1

All the migrations will pass if I comment out the connects_to from ApplicationRecord.

Observation 2

Furthermore, all the migrations pass (with the DEPRECATION WARNING shown above if Member or ApplicationRecord or any other model are referenced in the lib/shield_settings.rb.

Observation 3

If I comment out the code snippet from the offending migration, the migration passes. The strange thing is that the iteration is empty, so the loop will not run. Now another ~20 migrations pass, and then the following migration will fail:

class AddShortcodeToJobs < ActiveRecord::Migration[4.2]
  def up
    add_column :jobs, :shortcode, :string
    add_index :jobs, :shortcode, unique: true

    Job.reset_column_information

    Job.unscoped.where(shortcode: nil).each do |job|
      begin
        job.shortcode = SecureRandom.hex(5).upcase
      end while Job.unscoped.exists?(shortcode: job.shortcode)
      job.save
    end
  end

the error now is:

StandardError: An error has occurred, this and all later migrations canceled:

connection is closed
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'

Caused by:
ActiveRecord::ConnectionNotEstablished: connection is closed
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'

Caused by:
PG::ConnectionBad: connection is closed
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'

Caused by:
ActiveRecord::StatementInvalid: PG::UndefinedColumn: ERROR:  column jobs.shortcode does not exist
LINE 1: ...reated_at", "jobs"."updated_at" FROM "jobs" WHERE "jobs"."sh...
                                                             ^
/Users/markosfragkakis/dev/workable/db/migrate/20121003213728_add_shortcode_to_jobs.rb:8:in `up'
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'

Caused by:
PG::UndefinedColumn: ERROR:  column jobs.shortcode does not exist
LINE 1: ...reated_at", "jobs"."updated_at" FROM "jobs" WHERE "jobs"."sh...
                                                             ^
/Users/markosfragkakis/dev/workable/db/migrate/20121003213728_add_shortcode_to_jobs.rb:8:in `up'
/Users/markosfragkakis/dev/workable/bin/rails:5:in `<top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:10:in `block in <top (required)>'
/Users/markosfragkakis/dev/workable/bin/spring:7:in `<top (required)>'
Tasks: TOP => db:migrate
(See full trace by running task with --trace)

[fxn]

I need to read this with some time, but let me ask a couple of questions:

1. Who defines OmniAuth::Strategies::ShieldOIDC?
2. The provider call in the initializer is setting up Rack middleware, right?

[fragkakis]

Hello Xavier, thank you.

1. It is a custom strategy within our repository.
2. Yes, this is my understanding of how omniauth works.

[fxn]

Is OmniAuth::Strategies::ShieldOIDC defined in a directory that belongs to the autoload paths?

[fragkakis]

It is defined in lib/omni_auth/strategies/shield_oidc.rb, and the autoload paths from application.rb are the following:

    config.autoload_paths += %W[
      #{config.root}/app/models/serializable
      #{config.root}/app/models/concerns
      #{config.root}/lib
      #{config.root}/demo
      #{config.root}/qa
      #{config.root}/app/mailers/concerns
    ]

[fxn]

OK, that is not good because middleware is not reloaded. In Rails 7 you won't be able to configure the provider (precisely, to avoid these situations). You should tell the autoloader to ignore lib/omni_auth:

Rails.autoloaders.main.ignore("#{config.root}/lib/omni_auth")

and issue a require call from the initializer.

Not saying this solves the whole situation, but for starters it fixes something.

[fragkakis]

I have a workaround, but it doesn't seem the right. I added the following require statements (with the specific order, so that when application_record is loaded the two dependencies have already been loaded). I also removed the reference to the Member constant in my lib/shield_settings.rb.

# config/initializers/omniauth.rb
# ... other require statements
require "concerns/fine_grained_validations"
require "concerns/policy_scope_support"
require "application_record"

If you have any other insights, I will appreciate it if you chime in. Thank you.

[fxn]

OK, we need to polish a bit more.

No reloadable code should be loaded while the application is booting. The file lib/shield_settings.rb is still reloadable, because lib is in the autoload paths. Since the middleware uses the contants defined there, you need to tell the autoloader this file has to be ignored as well:

Rails.autoloaders.main.ignore("#{config.root}/lib/shield_settings.rb")

Let's iterate until we reach a more controlled setup to address the issue with the migrations.

Ideally, no warning should be issued. This section of the upgrdading guide has some tips, but I am here to help you anyway ❤️.

[fxn]

Since this is something related to how the Rails application is configured, I have moved the issue to the Rails repository (rails/rails#49310).

[eileencodes]

I am pretty sure that the connection closed issue is fixed by rails/rails#45450 but it can't be backported to 7-0-stable because it's too big of a change in internal behavior. Upgrading to 7.1.x should fix that particular issue.