fix: raise request error on invalid fragment variables (#4799)

Author: yaacovCR

src/execution/__tests__/variables-test.ts

@@ -1609,6 +1609,27 @@ describe('Execute: Handles inputs', () => {
       });
     });
 
+    it('does not allow invalid types to be used as fragment variables', () => {
+      const result = executeQueryWithFragmentArguments(`
+        query {
+          ...a
+        }
+        fragment a($value: TestType!) on TestType {
+          fieldWithNullableStringInput(input: $value)
+        }
+      `);
+
+      expectJSON(result).toDeepEqual({
+        errors: [
+          {
+            message:
+              'Variable "$value" expected value of type "TestType!" which cannot be used as an input type.',
+            locations: [{ line: 5, column: 28 }],
+          },
+        ],
+      });
+    });
+
     it('when a definition has a default, is not provided, and spreads another fragment', () => {
       const result = executeQueryWithFragmentArguments(`
         query {

src/execution/execute.ts

@@ -51,6 +51,7 @@ import type {
 import type { ExecutionResult } from './Executor.ts';
 import { Executor } from './Executor.ts';
 import { ExecutorThrowingOnIncremental } from './ExecutorThrowingOnIncremental.ts';
+import type { GraphQLVariableSignature } from './getVariableSignature.ts';
 import { getVariableSignature } from './getVariableSignature.ts';
 import type { ExperimentalIncrementalExecutionResults } from './incremental/IncrementalExecutor.ts';
 import { IncrementalExecutor } from './incremental/IncrementalExecutor.ts';
@@ -744,6 +745,7 @@ export function validateExecutionArgs(
   const fragmentDefinitions: ObjMap<FragmentDefinitionNode> =
     Object.create(null);
   const fragments: ObjMap<FragmentDetails> = Object.create(null);
+  const fragmentVariableSignatureErrors: Array<GraphQLError> = [];
   for (const definition of document.definitions) {
     switch (definition.kind) {
       case Kind.OPERATION_DEFINITION:
@@ -764,11 +766,17 @@ export function validateExecutionArgs(
         fragmentDefinitions[definition.name.value] = definition;
         let variableSignatures;
         if (definition.variableDefinitions) {
-          variableSignatures = Object.create(null);
+          const signatures: ObjMap<GraphQLVariableSignature> =
+            Object.create(null);
           for (const varDef of definition.variableDefinitions) {
             const signature = getVariableSignature(schema, varDef);
-            variableSignatures[signature.name] = signature;
+            if (signature instanceof GraphQLError) {
+              fragmentVariableSignatureErrors.push(signature);
+              continue;
+            }
+            signatures[signature.name] = signature;
           }
+          variableSignatures = signatures;
         }
         fragments[definition.name.value] = { definition, variableSignatures };
         break;
@@ -785,6 +793,10 @@ export function validateExecutionArgs(
     return [new GraphQLError('Must provide an operation.')];
   }
 
+  if (fragmentVariableSignatureErrors.length > 0) {
+    return fragmentVariableSignatureErrors;
+  }
+
   const variableDefinitions = operation.variableDefinitions ?? [];
   const hideSuggestions = args.hideSuggestions ?? false;
 

src/utilities/__tests__/replaceVariables-test.ts

@@ -5,6 +5,8 @@ import { expect } from 'chai';
 import { invariant } from '../../jsutils/invariant.ts';
 import type { ReadOnlyObjMap } from '../../jsutils/ObjMap.ts';
 
+import { GraphQLError } from '../../error/GraphQLError.ts';
+
 import type {
   FragmentArgumentNode,
   FragmentSpreadNode,
@@ -53,6 +55,7 @@ function testFragmentVariables(variableDefs: string, fragmentArgs: string) {
   const fragmentSignatures = Object.create(null);
   for (const varDef of parseVariableDefinitions(variableDefs)) {
     const signature = getVariableSignature(schema, varDef);
+    invariant(!(signature instanceof GraphQLError));
     fragmentSignatures[signature.name] = signature;
   }
   const spread: FragmentSpreadNode = {