Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraphQL::Client::ValidationError raised with global arguments method #1775

Closed
dblock opened this issue Aug 15, 2018 · 8 comments
Closed

GraphQL::Client::ValidationError raised with global arguments method #1775

dblock opened this issue Aug 15, 2018 · 8 comments

Comments

@dblock
Copy link
Contributor

dblock commented Aug 15, 2018

Coming from ashkan18/graphlient#47 I ran into a really weird problem trying to build a command line tool that queries the Github GraphQL API and uses https://github.com/davetron5000/gli. As soon as I tried to do anything with arguments in the tool I got strange GraphQL::Client::ValidationError errors. The code looked like this:

include GLI::App

# do some graphql stuff, get GraphQL::Client::ValidationError

Here's a minimal repro without GLI.

require 'graphql-client'

module X
  def arguments(args)
  end
end

include X

HTTP = GraphQL::Client::HTTP.new("https://api.github.com/graphql") do
  def headers(context)
    {
      "Authorization" => "Bearer #{ENV['GITHUB_ACCESS_TOKEN']}"
    }
  end
end

Schema = GraphQL::Client.load_schema(HTTP)

Client = GraphQL::Client.new(schema: Schema, execute: HTTP)

Query = Client.parse <<-GRAPHQL
  query($login: String!) {
    user(login: $login) {
      name
    }
  }
GRAPHQL

p Client.query(
  Query,
  variables: { login: 'dblock' }
) # => GraphQL::Client::ValidationError, Field 'user' doesn't accept argument 'login' 

This is obviously incorrect, the field accepts a login argument. Looks like graphql-ruby is calling arguments on Object? It probably shouldn't.

2: from /Users/dblock/.rvm/gems/ruby-2.5.1/gems/graphql-client-0.13.0/lib/graphql/client.rb:176:in `parse'
	1: from /Users/dblock/.rvm/gems/ruby-2.5.1/gems/graphql-client-0.13.0/lib/graphql/client.rb:176:in `each'
@rmosolgo
Copy link
Owner

Weird! Are those two lines the whole backtrace?

@dblock
Copy link
Contributor Author

dblock commented Aug 15, 2018

Adding a raise inside that method shows where it's called:

t.rb:7:in `raise': exception class/object expected (TypeError)
	from t.rb:7:in `arguments'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-1.8.7/lib/graphql/define/instance_definable.rb:157:in `public_send'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-1.8.7/lib/graphql/define/instance_definable.rb:157:in `block in ensure_defined'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-1.8.7/lib/graphql/define/instance_definable.rb:156:in `each'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-1.8.7/lib/graphql/define/instance_definable.rb:156:in `ensure_defined'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-1.8.7/lib/graphql/define/instance_definable.rb:206:in `block (3 levels) in stash_dependent_methods'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-1.8.7/lib/graphql/schema/loader.rb:24:in `block in load'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-1.8.7/lib/graphql/schema/loader.rb:21:in `each'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-1.8.7/lib/graphql/schema/loader.rb:21:in `load'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-client-0.13.0/lib/graphql/client.rb:52:in `load_schema'
	from /Users/dblock/.rvm/gems/ruby-2.3.0/gems/graphql-client-0.13.0/lib/graphql/client.rb:61:in `load_schema'
	from t.rb:21:in `<main>'

@rmosolgo
Copy link
Owner

Woah, it's somehow called from the load-schema-from-JSON code. I have no idea 😆

@dblock
Copy link
Contributor Author

dblock commented Aug 18, 2018

The problem comes from the excessive use of superclass, eg.here, which ends up calling methods on Object. Each instance can be fixed like so:

        # @return [Hash<String => GraphQL::Schema::Argument] Arguments defined on this thing, keyed by name. Includes inherited definitions
        def arguments
          inherited_arguments = ((
            self.is_a?(Class) && 
            !(superclass === Object) &&
            superclass.respond_to?(:arguments)
          ) ? superclass.arguments : {})
          # Local definitions override inherited ones
          inherited_arguments.merge(own_arguments)
        end

Now that can't be right. I would expect none of this code needing to call methods on superclass or needing relying on respond_to?, but I don't even know where to begin understanding what's going on :)

@rmosolgo
Copy link
Owner

It's implementing argument "inheritance", so if you have:

class BaseInputObject < GraphQL::Schema::InputObject 
end 

class EntityInput < BaseInputObject
  argument :id, ID, required: true, 
    description: "The ID of the entity to update"
end 

class TitledEntityInput < EntityInput 
  argument :title, String, required: false 
end 

class PostInput < TitledEntityInput 
  argument :body, String, required: false 
end 

Then, PostInput should "inherit" argument configurations from its parents, for example:

PostInput.arguments.keys 
# [:id, :title, :body]

That's why it traverses superclasses. Not sure how to fix it tho 😬

@dblock
Copy link
Contributor Author

dblock commented Aug 20, 2018

Why wouldn't it just store these in an inheritable class attribute?

Example: mongoid/mongoid-history#227

@rmosolgo
Copy link
Owner

Hmm, I took a look at the diff there, it looks like it's using Rails class_attribute, right? How would that work with merging hashes down the inheritance chain? I can't quite see it in my mind's eye 🤔

@rmosolgo
Copy link
Owner

Sorry, I don't have a clear understanding of how to fix this, and I haven't heard any other reports of it, so I'm going to close this issue. If someone else encounters it, please let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants