rmosolgo / graphql-ruby

Ruby implementation of GraphQL
http://graphql-ruby.org
MIT License
5.38k stars 1.39k forks source link

GraphQL schema visibility raises `NoMethodError` in v2.4.2 #5156

Closed FraDim closed 2 weeks ago

FraDim commented 2 weeks ago

Describe the bug

I noticed NoMethodError raised with the new 2.4.2 version. It does not happen all the time.

Versions

graphql version: 2.4.2 rails: 7.1

GraphQL schema

I have simplified some functionality.

class PublicSchema < GraphQL::Schema
  max_depth 50
  max_complexity 5000
  default_max_page_size 500

  use GraphQL::Dataloader
  use GraphQL::Schema::Visibility

  use(GraphQL::Enterprise::ObjectCache, **Rails.configuration.x.graphql_cache)

  mutation { Types::MutationType }
  query { Types::QueryType }

  connections.add(ActiveRecord::Relation, GraphQL::Pro::PostgresStableRelationConnection)

  query_analyzer Schema::LoggingAnalyzer
end
module Types
  class MutationType < BaseObject
    def self.authorized?(_object, _context)
      true
    end

    field :create_acceptable_data, mutation: Mutations::CreateAcceptableData
  end
end
module Mutations
  class CreateAcceptableData < BaseMutation
    def self.visible?(_context)
      ApplicationFeatureSet.quality_assurance_tools_enabled?
    end

    field :date_of_birth, null: false do
      type GraphQL::Types::ISO8601Date
    end
    field :email, String, null: false
    field :phone_number, String, null: false

    def resolve = ::AcceptableData::CreateCommand.call
  end
end

GraphQL query

It seems that any query manages to reproduce it.

Expected behavior

Not raising an error.

Actual behavior

Raising an error

Place full backtrace here (if a Ruby exception is involved):

Click to view exception backtrace ``` NoMethodError: undefined method `kind' for an instance of Proc (NoMethodError) if type.kind.fields? && @preloaded_types.add?(type) ^^^^^ from graphql (2.4.2) lib/graphql/schema/visibility.rb:133:in `ensure_all_loaded' from graphql (2.4.2) lib/graphql/schema/visibility.rb:63:in `mutation_configured' from graphql (2.4.2) lib/graphql/schema.rb:495:in `mutation' from graphql (2.4.2) lib/graphql/schema.rb:545:in `root_type_for_operation' from graphql (2.4.2) lib/graphql/static_validation/base_visitor.rb:57:in `on_operation_definition' from graphql (2.4.2) lib/graphql/language/static_visitor.rb:61:in `public_send' from graphql (2.4.2) lib/graphql/language/static_visitor.rb:61:in `block in on_document_children' from graphql (2.4.2) lib/graphql/language/static_visitor.rb:59:in `each' from graphql (2.4.2) lib/graphql/language/static_visitor.rb:59:in `on_document_children' from graphql (2.4.2) lib/graphql/language/static_visitor.rb:41:in `on_document' from graphql (2.4.2) lib/graphql/static_validation/definition_dependencies.rb:38:in `on_document' from graphql (2.4.2) lib/graphql/static_validation/rules/variables_are_used_and_defined.rb:79:in `on_document' from graphql (2.4.2) lib/graphql/static_validation/rules/fragment_spreads_are_possible.rb:26:in `on_document' from graphql (2.4.2) lib/graphql/static_validation/rules/fragments_are_used.rb:6:in `on_document' from graphql (2.4.2) lib/graphql/static_validation/rules/fragments_are_finite.rb:6:in `on_document' from graphql (2.4.2) lib/graphql/static_validation/rules/fragment_names_are_unique.rb:17:in `on_document' from graphql (2.4.2) lib/graphql/static_validation/rules/operation_names_are_valid.rb:16:in `on_document' from graphql (2.4.2) lib/graphql/static_validation/rules/no_definitions_are_present.rb:34:in `on_document' from graphql (2.4.2) lib/graphql/language/static_visitor.rb:16:in `public_send' from graphql (2.4.2) lib/graphql/language/static_visitor.rb:16:in `visit' from graphql (2.4.2) lib/graphql/static_validation/validator.rb:45:in `block (3 levels) in validate' from graphql (2.4.2) lib/graphql/static_validation/validator.rb:44:in `catch' from graphql (2.4.2) lib/graphql/static_validation/validator.rb:44:in `block (2 levels) in validate' from timeout (0.4.2) lib/timeout.rb:167:in `timeout' from graphql (2.4.2) lib/graphql/static_validation/validator.rb:43:in `block in validate' from graphql (2.4.2) lib/graphql/tracing/trace.rb:28:in `validate' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:36:in `block in validate' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:84:in `block in instrument_sentry_execution' from sentry-ruby (5.21.0) lib/sentry/hub.rb:115:in `block in with_child_span' from sentry-ruby (5.21.0) lib/sentry/span.rb:232:in `with_child_span' from sentry-ruby (5.21.0) lib/sentry/hub.rb:113:in `with_child_span' from sentry-ruby (5.21.0) lib/sentry-ruby.rb:499:in `with_child_span' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:83:in `instrument_sentry_execution' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:36:in `validate' from graphql (2.4.2) lib/graphql/static_validation/validator.rb:30:in `validate' from graphql (2.4.2) lib/graphql/query/validation_pipeline.rb:72:in `ensure_has_validated' from graphql (2.4.2) lib/graphql/query/validation_pipeline.rb:33:in `valid?' from graphql (2.4.2) lib/graphql/query.rb:349:in `valid?' from graphql (2.4.2) lib/graphql/analysis.rb:27:in `block (2 levels) in analyze_multiplex' from graphql (2.4.2) lib/graphql/analysis.rb:26:in `map' from graphql (2.4.2) lib/graphql/analysis.rb:26:in `block in analyze_multiplex' from graphql (2.4.2) lib/graphql/tracing/trace.rb:32:in `analyze_multiplex' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:36:in `block in analyze_multiplex' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:84:in `block in instrument_sentry_execution' from sentry-ruby (5.21.0) lib/sentry/hub.rb:115:in `block in with_child_span' from sentry-ruby (5.21.0) lib/sentry/span.rb:232:in `with_child_span' from sentry-ruby (5.21.0) lib/sentry/hub.rb:113:in `with_child_span' from sentry-ruby (5.21.0) lib/sentry-ruby.rb:499:in `with_child_span' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:83:in `instrument_sentry_execution' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:36:in `analyze_multiplex' from graphql-enterprise (1.5.4) lib/graphql/enterprise/object_cache.rb:533:in `analyze_multiplex' from graphql (2.4.2) lib/graphql/analysis.rb:25:in `analyze_multiplex' from graphql (2.4.2) lib/graphql/execution/interpreter.rb:47:in `block in run_all' from graphql (2.4.2) lib/graphql/tracing/trace.rb:40:in `execute_multiplex' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:36:in `block in execute_multiplex' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:84:in `block in instrument_sentry_execution' from sentry-ruby (5.21.0) lib/sentry/hub.rb:115:in `block in with_child_span' from sentry-ruby (5.21.0) lib/sentry/span.rb:232:in `with_child_span' from sentry-ruby (5.21.0) lib/sentry/hub.rb:113:in `with_child_span' from sentry-ruby (5.21.0) lib/sentry-ruby.rb:499:in `with_child_span' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:83:in `instrument_sentry_execution' from graphql (2.4.2) lib/graphql/tracing/sentry_trace.rb:36:in `execute_multiplex' from graphql-enterprise (1.5.4) lib/graphql/enterprise/object_cache.rb:527:in `execute_multiplex' from graphql (2.4.2) lib/graphql/execution/interpreter.rb:38:in `run_all' from graphql (2.4.2) lib/graphql/schema.rb:1527:in `multiplex' from graphql (2.4.2) lib/graphql/schema.rb:1503:in `execute' from app/controllers/graph_ql_controller.rb:75:in `result' from memery (1.6.0) lib/memery.rb:88:in `block in define_memoized_method!' from app/controllers/graph_ql_controller.rb:17:in `create' from actionpack (7.1.4.2) lib/action_controller/metal/basic_implicit_render.rb:6:in `send_action' from actionpack (7.1.4.2) lib/abstract_controller/base.rb:224:in `process_action' from actionpack (7.1.4.2) lib/action_controller/metal/rendering.rb:165:in `process_action' from actionpack (7.1.4.2) lib/abstract_controller/callbacks.rb:259:in `block in process_action' from activesupport (7.1.4.2) lib/active_support/callbacks.rb:121:in `block in run_callbacks' #… ```
rmosolgo commented 2 weeks ago

Hey! Thanks for this report. It looks like a bug in the lazy-loading code. I'll take a look and follow up here in a bit 👀

rmosolgo commented 2 weeks ago

Derp... there was a typo in that code path. I will merge #5158 and release 2.4.3 shortly. Thanks again for reporting this!