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

Parsing an SDL file with extend schema leads to NoMethodError (name on GraphQL::Language::Nodes::SchemaExtension) #4199

Closed
myronmarston opened this issue Sep 19, 2022 · 4 comments · Fixed by #4203

Comments

@myronmarston
Copy link

myronmarston commented Sep 19, 2022

Describe the bug

Apollo requires the following in a subgraph schema for Apollo Federation V2:

extend schema
  @link(url: "https://specs.apollo.dev/federation/v2.0",
        import: ["@key", "@shareable"])

However, when I try to parse an SDL file that contains this using graphql-ruby, I get a NoMethodError

Versions

graphql version: 2.0.14

GraphQL schema

See the script below for a full runnable example of this bug.

GraphQL query

N/A

Steps to reproduce

Put this file in tmp/graphql_ruby_bug.rb:

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "graphql", "2.0.14"
end

schema_sdl = <<~EOS
  extend schema
    @link(url: "https://specs.apollo.dev/federation/v2.0",
          import: ["@key", "@shareable"])

  type Query {
    something: Int
  }
EOS

require "graphql"

GraphQL::Schema.from_definition(schema_sdl)

Then run with:

$ ruby tmp/graphql_ruby_bug.rb

Expected behavior

This SDL string should parse without throwing any exceptions.

Actual behavior

$ ruby tmp/graphql_ruby_bug.rb                                                                                                                                                                                                                                                                        Traceback (most recent call last):
        6: from tmp/graphql_ruby_bug.rb:20:in `<main>'
        5: from /Users/myron/.rvm/gems/ruby-2.7.5/gems/graphql-2.0.14/lib/graphql/schema.rb:122:in `from_definition'
        4: from /Users/myron/.rvm/gems/ruby-2.7.5/gems/graphql-2.0.14/lib/graphql/schema/build_from_definition.rb:10:in `from_definition'
        3: from /Users/myron/.rvm/gems/ruby-2.7.5/gems/graphql-2.0.14/lib/graphql/schema/build_from_definition.rb:18:in `from_document'
        2: from /Users/myron/.rvm/gems/ruby-2.7.5/gems/graphql-2.0.14/lib/graphql/schema/build_from_definition.rb:68:in `build'
        1: from /Users/myron/.rvm/gems/ruby-2.7.5/gems/graphql-2.0.14/lib/graphql/schema/build_from_definition.rb:68:in `each'
/Users/myron/.rvm/gems/ruby-2.7.5/gems/graphql-2.0.14/lib/graphql/schema/build_from_definition.rb:74:in `block in build': 
undefined method `name' for #<GraphQL::Language::Nodes::SchemaExtension:0x0000000130a2b580> (NoMethodError)

Additional context

It looks like the case statement clause here is missing GraphQL::Language::Nodes::SchemaExtension.

https://github.com/rmosolgo/graphql-ruby/blob/v2.0.14/lib/graphql/schema/build_from_definition.rb#L70

If I add GraphQL::Language::Nodes::SchemaExtension to it, I no longer get the exception.

@rmosolgo
Copy link
Owner

rmosolgo commented Sep 20, 2022

Thanks for the detailed report! I think we updated the parser to handle extensions but never actually implemented loading schemas with extend. But I can take a look at getting that started 👍

@rmosolgo
Copy link
Owner

I'm taking a look over at #4203.

One issue is, GraphQL-Ruby can't validate extend schema @link(...) usage without a directive @link(...) definition in the document. Would it be reasonable to include a directive @link ... definition, as described in https://www.apollographql.com/docs/federation/federated-types/federated-directives/#link ?

Alternatively, you could lay out the directive definition in the schema class, then call .from_definition on that custom class, as demonstrated in this example:

class LinkSchema < GraphQL::Schema
class Import < GraphQL::Schema::Scalar
end
class Purpose < GraphQL::Schema::Scalar
end
class Link < GraphQL::Schema::Directive
argument :url, String
argument :as, String, required: false
argument :import, Import, required: false
argument :for, Purpose, required: false
repeatable(true)
locations SCHEMA
end
directive(Link)
end
assert_equal LinkSchema::Link, LinkSchema.directives["link"]
schema = LinkSchema.from_definition(schema_sdl)

Would one of those approaches work for your use case?

@myronmarston
Copy link
Author

Great catch! You're right that my SDL is missing a definition for the @link directive. I don't expect the graphql gem to magically understand it, and in fact in my real SDL file I have this definition for it:

scalar link__Import
directive @link(import: [link__Import], url: String!) on SCHEMA

On a side note: I didn't realize you could define some parts of a schema in ruby, and load a definition for the rest into it--that's cool and I may use that in the future--so thanks!

@rmosolgo
Copy link
Owner

define some parts of a schema in ruby

To clarify, it's actually leveraging inheritance: the directive is added to the superclass, then .from_definition(...) creates a subclass. But that won't work properly until #4203 is merged 😅

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

Successfully merging a pull request may close this issue.

2 participants