`Query#lookahead` with no selected operation

[ravangen]

Describe the bug

graphql-ruby/lib/graphql/query.rb

Lines 211 to 212 in c663621

	ast_node = selected_operation
	root_type = case ast_node.operation_type

When there is no selected_operation, we get an exception related to nil:

Error:
NoMethodError: undefined method `operation_type' for nil
    /Users/robvangennip/.gem/ruby/3.3.1/gems/graphql-2.3.18/lib/graphql/query.rb:206:in `lookahead'

GraphQL schema

class QueryRoot < GraphQL::Schema::Object
  field :echo, String, null: true do |field|
    field.argument(:value, String, required: true)
  end

  def echo(value:)
    value
  end
end

class Inspire < GraphQL::Schema::Mutation
  field :value, String

  def resolve
    "Do. Or do not. There is no try."
  end
end

class MutationRoot < GraphQL::Schema::Object
  field :inspire, mutation: Inspire
end

class Schema < GraphQL::Schema
  query QueryRoot
  mutation MutationRoot
end

class Query < GraphQL::Query
  def root_fields
    return @root_fields if defined?(@root_fields)

    @root_fields = lookahead.selections.filter_map { |selection| selection.field&.graphql_name }.uniq
  end
end

Steps to reproduce

query_string = <<~DESC
  query Greeting {
    echo(value: "Hello there")
  }
  mutation Motivation {
    inspire { value }
  }
DESC
query = Query.new(Schema, query_string, operation_name: "Invalid")

assert_equal [], query.root_fields

Expected behavior

I'm not sure what is best:

• Raise no operation selected (interrupts flow)
• Default to query_root (unexpected)
• Return nil (changes interface but matches selected_operation_name)
• Leave as is

Actual behavior

NoMethodError: undefined method `operation_type' for nil

Additional context

I've added a check for the query being valid to work around this, but should lookahead be more defensive?

query.valid?
=> false

[rmosolgo]

Thanks for the detailed report of this bug! I agree some other behavior would be an improvement. Another option is NULL_LOOKAHEAD which is what Lookahead returns when there are no selections:

graphql-ruby/lib/graphql/execution/lookahead.rb

Lines 213 to 241 in c663621

	# This is returned for {Lookahead#selection} when a non-existent field is passed
	class NullLookahead < Lookahead
	# No inputs required here.
	def initialize
	end
	def selected?
	false
	end
	def selects?(*)
	false
	end
	def selection(*)
	NULL_LOOKAHEAD
	end
	def selections(*)
	[]
	end
	def inspect
	"#<GraphQL::Execution::Lookahead::NullLookahead>"
	end
	end
	# A singleton, so that misses don't come with overhead.
	NULL_LOOKAHEAD = NullLookahead.new

What do you think of returning that in this case? It has the upside of keeping the API the same (no nil) and not raising any errors -- and probably, it would do the "right thing" in case of invalid queries like the one in your example.

[ravangen]

TIL, sounds reasonable