rmosolgo/graphql-ruby

Adding a resolver to a new type causes `Can't determine the return type` error

yannickgloster opened this issue · 3 comments

yannickgloster commented

Describe the bug

We've got a resolver which loads a lot of items of type Foo. This resolver originally only existed on our query_type.rb and could be directly queried from any part of our app.

Foo has a property Baz and Bar also has the property Baz. I was adding the ability to query Foo through the same resolver additionally from Bar.

Foo has quite a lot of filters that we can apply to it and we've got an exporter where we are taking those list of filters from the user and applying them to an exported version. At this stage is where we get the error. The actual app works fine.

Versions

graphql-pro version: 1.26.5
rails (or other framework): ~> 7.0.8.4

GraphQL schema

Include relevant types and fields (in Ruby is best, in GraphQL IDL is ok). Any custom extensions, etc?

module Types
  class FooType < Types::BaseObject
    graphql_name "Foo"
    implements GraphQL::Types::Relay::Node
    
    global_id_field :id

    foreign_key :team_id, ::Team, null: false

    connection_type_class Connections::FooSearchConnection
    
    # Other fields
  end
end

class Connections::FooSearchConnection < GraphQL::Types::Relay::BaseConnection
  edges_nullable(true)
  edge_nullable(true)
  node_nullable(true)
  has_nodes_field(true)

  field :total_count, Integer, null: false

  def total_count
    count = object.nodes.size
    return count.size if count.instance_of?(Hash)
    count
  end
end

module Types
  class QueryType < Types::BaseObject
    include ActionView::Helpers::DateHelper
    include GraphQL::Types::Relay::HasNodeField

    field :foo, resolver: Resolvers::TeamCases
  end
end

module Types
  class Bar < Types::BaseObject
    graphql_name "Bar"
    implements GraphQL::Types::Relay::Node

    field :foo, resolver: Resolvers::TeamCases
  end
end

module Types
  class BarListType < Types::BaseObject
    graphql_name "BarList"
    implements GraphQL::Types::Relay::Node

    global_id_field :id

    foreign_key :team_id, ::Team, null: false

    field :team, Types::TeamType, null: false

    field :bars, [Types::BarType], null: false
  end
end

class Resolvers::Foo < Resolvers::Base
  type Types::FooType.connection_type, null: true

  class Filter < Types::BaseInputObject
    graphql_name "FooFilters"
 
    argument ... # etc
  end

  def resolve()
     baz ||= object if object.is_a?(Bar)
     FooSearch.new(
            relation,
            # Other params...
          )
          .call(filters)
  end
end

class FooCsvExportJob
  include Sidekiq::Worker
  extend T::Sig

  class Filters
    extend T::Sig

    # Define each field in the struct
    Resolvers::Foo::Filter # ERRORs on this line in spec before getting to perform
      .arguments
      .keys
      .map(&:underscore)
      .each do |field|
        # Using `attr_accessor` to create getter and setter methods for each field
        attr_accessor field.to_sym
      end

    sig { params(args: T::Hash[Symbol, T.untyped]).void }
    def initialize(args = {})
      args.each { |key, value| send(:"#{key}=", value) if respond_to?(:"#{key}=") }
    end
  end

  FIELDS = [
      # list of fields
   ]
  def perform()
   # code
  end
end

Steps to reproduce

Expected behavior

It should not error error when accessing a resolver from two places.

Actual behavior

It errored

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

Click to view exception backtrace 
An error occurred while loading ./spec/jobs/foo_csv_export_job.rb.
Failure/Error: super(*args, **kwargs, &block)

GraphQL::Schema::Field::MissingReturnTypeError:
  Can't determine the return type for Baz.foos (it has `resolver: Resolvers::Foos`, consider configuration a `type ...` for that class)

# ./app/graphql/types/base_field.rb:9:in `initialize'
# ./app/graphql/types/bar_type.rb:31:in `<class:BarType>'
# ./app/graphql/types/bar_type.rb:5:in `<module:Types>'
# ./app/graphql/types/bar_type.rb:4:in `<top (required)>'
# ./app/graphql/types/bar_list_type.rb:18:in `<class:BarListType>'
# ./app/graphql/types/bar_list_type.rb:5:in `<module:Types>'
# ./app/graphql/types/bar_list_type.rb:4:in `<top (required)>'
# ./app/graphql/types/team_type.rb:245:in `<class:TeamType>'
# ./app/graphql/types/team_type.rb:5:in `<module:Types>'
# ./app/graphql/types/team_type.rb:4:in `<top (required)>'
# ./app/graphql/types/baz_type.rb:13:in `<class:BazType>'
# ./app/graphql/types/baz_type.rb:5:in `<module:Types>'
# ./app/graphql/types/baz_type.rb:4:in `<top (required)>'
# ./app/graphql/types/foo_type.rb:23:in `<class:FooType>'
# ./app/graphql/types/foo_type.rb:5:in `<module:Types>'
# ./app/graphql/types/foo_type.rb:4:in `<top (required)>'
# ./app/graphql/resolvers/foos.rb:4:in `<class:Foos>'
# ./app/graphql/resolvers/foos.rb:3:in `<top (required)>'
# ./app/jobs/foo_csv_export_job.rb:13:in `<class:Filters>'
# ./app/jobs/foo_csv_export_job.rb:9:in `<class:FoosCsvExportJob>'
# ./app/jobs/foo_csv_export_job.rb:4:in `<top (required)>'
# ./spec/jobs/foo_csv_export_job.rb:5:in `<top (required)>'
Run options: include {:locations=>{"./spec/jobs/foo_csv_export_job.rb"=>[7]}}

Additional context

Similar to #3974. I've attempted the suggestions from that thread by wrapping the connection as a string, but it did not work.

yannickgloster commented

I've just been able to get around this error by forcibly specifying the type here

module Types
  class Bar < Types::BaseObject
    graphql_name "Bar"
    implements GraphQL::Types::Relay::Node

    field :foo, type: Types::FooType.connection_type, resolver: Resolvers::Foos
  end
end

I'm not certain this is recommended practice though.

rmosolgo commented

Hey, thanks for this detailed report. I agree that what you're describing should work, one way or another.

The workaround you found looks great to me. As you probably noticed, a field prioritizes the directly-given type: ... over the one from the resolver:

graphql-ruby/lib/graphql/schema/field.rb

Line 582 in 5a7e235

return_type = @return_type_expr || @resolver_class.type_expr

That behavior is also tested in the gem here:

graphql-ruby/spec/graphql/schema/resolver_spec.rb

Lines 1111 to 1125 in 5a7e235

describe "when the field has configurations that override the resolvers" do
it "uses the field's type and description, not the resolver's" do
resolver = Class.new(GraphQL::Schema::Resolver) do
description "Does things!"
type(String, null: true)
end
field = GraphQL::Schema::Field.new(name: "blah", resolver_class: resolver, description: nil, type: Integer, null: false)
assert_equal "Int!", field.type.to_type_signature
assert_equal "String", resolver.type.to_type_signature
assert_nil field.description
assert_equal "Does things!", resolver.description
end
end

So, I expect that to keep working just fine indefinitely.

However, the original code should work, and it's on my list to improve GraphQL-Ruby's lazy-loading of type definitions (as part of #5014), so I'll keep this issue open until this issue is addressed better.

  • 👍1
yannickgloster commented

Hey, thanks for this detailed report. I agree that what you're describing should work, one way or another.

The workaround you found looks great to me. As you probably noticed, a field prioritizes the directly-given type: ... over the one from the resolver:

graphql-ruby/lib/graphql/schema/field.rb

Line 582 in 5a7e235

return_type = @return_type_expr || @resolver_class.type_expr

That behavior is also tested in the gem here:

graphql-ruby/spec/graphql/schema/resolver_spec.rb

Lines 1111 to 1125 in 5a7e235

describe "when the field has configurations that override the resolvers" do
it "uses the field's type and description, not the resolver's" do
resolver = Class.new(GraphQL::Schema::Resolver) do
description "Does things!"
type(String, null: true)
end
field = GraphQL::Schema::Field.new(name: "blah", resolver_class: resolver, description: nil, type: Integer, null: false)
assert_equal "Int!", field.type.to_type_signature
assert_equal "String", resolver.type.to_type_signature
assert_nil field.description
assert_equal "Does things!", resolver.description
end
end

So, I expect that to keep working just fine indefinitely.

However, the original code should work, and it's on my list to improve GraphQL-Ruby's lazy-loading of type definitions (as part of #5014), so I'll keep this issue open until this issue is addressed better.

That's great, if you need help with more reproducibility, I'm happy to help provide more context to help get it work (or to break it haha). I couldn't just copy paste the code directly from work, so I may have made a typo while changing all the names.