[GraphQL::Schema::Visibility] Inherited schemas don't preload resulting in crash

[cocoahero]

TL;DR;

When a parent schema enables the GraphQL::Schema::Visibility plugin using preload: true, child schemas will crash when declaring the root query object.

/ruby/3.4.4/gems/graphql-2.5.9/lib/graphql/schema/visibility.rb:191:in 'GraphQL::Schema::Visibility#ensure_all_loaded': undefined method 'add?' for nil (NoMethodError)

    if type.kind.fields? && @preloaded_types.add?(type)
                                            ^^^^^
from /ruby/3.4.4/gems/graphql-2.5.9/lib/graphql/schema/visibility.rb:96:in 'GraphQL::Schema::Visibility#query_configured'
from /ruby/3.4.4/gems/graphql-2.5.9/lib/graphql/schema.rb:451:in 'GraphQL::Schema.query'
from ./example.rb:20:in '<class:ConcreteSchema>'
from ./example.rb:19:in '<main>'

I believe the issue is here, where the visibility instance is duplicated with preload: true, but the preloaded state is not initialized / copied.

Version

/ruby/3.4.4/gems/graphql-2.5.9

Example

class QueryRoot < GraphQL::Schema::Object
  field :hello, String, null: false

  def hello
    "Hello, world!"
  end
end

class AbstractSchema < GraphQL::Schema
  # simulate Rails.env.production? == true
  use(GraphQL::Schema::Visibility, preload: true)
end

class ConcreteSchema < AbstractSchema
  query(QueryRoot)
end

result = ConcreteSchema.execute("{ hello }")

[rmosolgo]

👋 Thanks for the detailed report and sorry about the trouble. I replicated the bug and I've got a fix in the works at #5386

[spacebat]

We just got bitten by this during a production deployment as we have subclassed schemas and were relying on the default for preload, will update to the latest.

I suggest if a default behaviour is going to depend on the Rails environment, then instead of "it's different in production", it is better to have "it's different in development". That way bugs are more likely to be found by QA processes in staging environments.

Thanks for having a fix out there already!

[rmosolgo]

Great point, @spacebat. I opened #5409 to switch that around.