Validation/coercion of input variables violates the GraphQL spec

[dkbarn]

When callling graphql-core's execute() function with variable_values which do not pass validation -- for example, including an unexpected key in the variable_values dictionary -- the current behavior is that an ExecutionResult object is returned from the function, with the associated GraphQLError present inside it. Instead, this should be treated as a Request error, according to the spec, meaning that a GraphQLError should be raised from execute().

The GraphQL spec states:

Request errors
Request errors are raised before execution begins. This may occur due to a parse grammar or validation error in the requested document, an inability to determine which operation to execute, or invalid input values for variables.

This means that it is incorrect for the coerce_variable_values function to be returning a GraphQLError inside an ExecutionResult:

graphql-core/src/graphql/execution/values.py

Lines 93 to 99 in 9dcf25e

	on_error(
	GraphQLError(
	f"Variable '${var_name}' expected value of type '{var_type_str}'"
	" which cannot be used as an input type.",
	var_def_node.type,
	)
	)

doing so means that a response payload is returned containing both an "errors" key and a null "data" key. Again, this a violation of the spec:

If a request error is raised, execution does not begin and the data entry in the response must not be present. The errors entry must include the error.

[Cito]

Hi @dkbarn, thanks for bringing this up.

Actually I believe this is ok in view of the comment directly in front of the lines you linked to. The error in the variable values should be caught during validation already and never happen when execute() is called via the high-level graphql() function that includes validation. Conversion of the ExecutionResult to a JSON response payload and removal of the data property with a null value for a request error should happen at the server level.

If you still believe the behavior of the values module should be changed, please report upstream at the GraphQL.js project since the goal of GraphQL-core is to implement the exact same behavior as this reference implementation. See the corresponding code here.