webonyx · ruudk · Aug 25, 2026
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co
 
 ## Unreleased
 
+### Fixed
+
+- Report variable coercion errors from `QueryComplexity` through the validation context instead of throwing them out of `DocumentValidator::validate()` https://github.com/webonyx/graphql-php/issues/1967
+
 ## v15.37.2
 
 ### Changed

diff --git a/src/Validator/Rules/QueryComplexity.php b/src/Validator/Rules/QueryComplexity.php
@@ -45,6 +45,13 @@ class QueryComplexity extends QuerySecurityRule
     /** @var array<string, mixed>|null Lazily coerced variable values; reset per document. */
     private ?array $coercedVariableValues = null;
 
+    /**
+     * Did variable coercion fail for this document?
+     *
+     * The errors have been reported to the context, so complexity analysis is abandoned.
+     */
+    private bool $variableCoercionFailed = false;
+
     /** @throws \InvalidArgumentException */
     public function __construct(int $maxQueryComplexity)
     {
@@ -58,6 +65,7 @@ public function getVisitor(QueryValidationContext $context): array
         $this->variableDefs = new NodeList([]);
         $this->fieldNodeAndDefs = new \ArrayObject();
         $this->coercedVariableValues = null;
+        $this->variableCoercionFailed = false;
 
         return $this->invokeIfNeeded(
             $context,
@@ -95,6 +103,16 @@ public function getVisitor(QueryValidationContext $context): array
 
                             $this->queryComplexity = $this->fieldComplexity($definition->selectionSet);
 
+                            // Without usable variable values the computed complexity is
+                            // meaningless: it is a partial sum of whatever was visited
+                            // before coercion failed. Reset it so `getQueryComplexity()`
+                            // does not expose it. The coercion errors were reported.
+                            if ($this->variableCoercionFailed) {
+                                $this->queryComplexity = 0;
+
+                                return;
+                            }
+
                             if ($this->queryComplexity > $this->maxQueryComplexity) {
                                 $context->reportError(
                                     new Error(static::maxQueryComplexityErrorMessage(
@@ -118,6 +136,10 @@ protected function fieldComplexity(SelectionSetNode $selectionSet): int
         $complexity = 0;
 
         foreach ($selectionSet->selections as $selection) {
+            if ($this->variableCoercionFailed) {
+                return 0;
+            }
+
             $complexity += $this->nodeComplexity($selection);
         }
 
@@ -146,6 +168,10 @@ protected function nodeComplexity(SelectionNode $node): int
                 if ($fieldDef instanceof FieldDefinition && $fieldDef->complexityFn !== null) {
                     $fieldArguments = $this->buildFieldArguments($node);
 
+                    if ($this->variableCoercionFailed) {
+                        return 0;
+                    }
+
                     return ($fieldDef->complexityFn)($childrenComplexity, $fieldArguments);
                 }
 
@@ -187,10 +213,15 @@ protected function directiveExcludesField(FieldNode $node): bool
     {
         foreach ($node->directives as $directiveNode) {
             if ($directiveNode->name->value === Directive::INCLUDE_NAME) {
+                $variableValues = $this->getCoercedVariableValues();
+                if ($variableValues === null) {
+                    return false;
+                }
+
                 $includeArguments = Values::getArgumentValues(
                     Directive::includeDirective(),
                     $directiveNode,
-                    $this->getCoercedVariableValues()
+                    $variableValues
                 );
                 assert(is_bool($includeArguments['if']), 'ensured by query validation');
 
@@ -200,10 +231,15 @@ protected function directiveExcludesField(FieldNode $node): bool
             }
 
             if ($directiveNode->name->value === Directive::SKIP_NAME) {
+                $variableValues = $this->getCoercedVariableValues();
+                if ($variableValues === null) {
+                    return false;
+                }
+
                 $skipArguments = Values::getArgumentValues(
                     Directive::skipDirective(),
                     $directiveNode,
-                    $this->getCoercedVariableValues()
+                    $variableValues
                 );
                 assert(is_bool($skipArguments['if']), 'ensured by query validation');
 
@@ -219,13 +255,20 @@ protected function directiveExcludesField(FieldNode $node): bool
     /**
      * Coerce variable values once per document and cache them.
      *
+     * Returns `null` when coercion failed, in which case the coercion errors have been
+     * reported to the validation context.
+     *
      * @throws \Exception
      * @throws InvariantViolation
      *
-     * @return array<string, mixed>
+     * @return array<string, mixed>|null
      */
-    private function getCoercedVariableValues(): array
+    private function getCoercedVariableValues(): ?array
     {
+        if ($this->variableCoercionFailed) {
+            return null;
+        }
+
         if ($this->coercedVariableValues !== null) {
             return $this->coercedVariableValues;
         }
@@ -236,7 +279,13 @@ private function getCoercedVariableValues(): array
             $this->getRawVariableValues()
         );
         if ($errors !== null && $errors !== []) {
-            throw new Error(implode("\n\n", array_map(static fn (Error $error): string => $error->getMessage(), $errors)));
+            $this->variableCoercionFailed = true;
+
+            foreach ($errors as $error) {
+                $this->context->reportError($error);
+            }
+
+            return null;
         }
 
         return $this->coercedVariableValues = $variableValues ?? [];
@@ -256,27 +305,21 @@ public function setRawVariableValues(?array $rawVariableValues = null): void
 
     /**
      * @throws \Exception
-     * @throws Error
+     * @throws InvariantViolation
      *
      * @return array<string, mixed>
      */
     protected function buildFieldArguments(FieldNode $node): array
     {
-        $rawVariableValues = $this->getRawVariableValues();
         $fieldDef = $this->fieldDefinition($node);
 
         /** @var array<string, mixed> $args */
         $args = [];
 
         if ($fieldDef instanceof FieldDefinition) {
-            [$errors, $variableValues] = Values::getVariableValues(
-                $this->context->getSchema(),
-                $this->variableDefs,
-                $rawVariableValues
-            );
-
-            if (is_array($errors) && $errors !== []) {
-                throw new Error(implode("\n\n", array_map(static fn ($error) => $error->getMessage(), $errors)));
+            $variableValues = $this->getCoercedVariableValues();
+            if ($variableValues === null) {
+                return $args;
             }
 
             $args = Values::getArgumentValues($fieldDef, $node, $variableValues);

diff --git a/tests/Validator/QueryComplexityTest.php b/tests/Validator/QueryComplexityTest.php
@@ -5,6 +5,7 @@
 use GraphQL\Error\Error;
 use GraphQL\Language\AST\NodeKind;
 use GraphQL\Language\Parser;
+use GraphQL\Language\SourceLocation;
 use GraphQL\Type\Introspection;
 use GraphQL\Validator\DocumentValidator;
 use GraphQL\Validator\Rules\CustomValidationRule;
@@ -331,4 +332,169 @@ public function testVariableCoercionIsCachedAcrossMultipleDirectives(): void
         // skipIt=false means nothing is skipped: complexity = human(1) + firstName(1) + dogs(10) + name(1) = 13
         $this->assertDocumentValidators($query, 13, 14);
     }
+
+    /**
+     * Variable coercion errors must be reported through the validation context rather
+     * than thrown, so that `DocumentValidator::validate()` returns them like any other
+     * validation error.
+     *
+     * @see https://github.com/webonyx/graphql-php/issues/1967
+     *
+     * @dataProvider variableCoercionFailureProvider
+     *
+     * @param array<string, mixed> $rawVariableValues
+     */
+    public function testReportsVariableCoercionErrorsInsteadOfThrowing(
+        string $query,
+        array $rawVariableValues,
+        string $expectedMessage
+    ): void {
+        $rule = $this->getRule(1);
+        $rule->setRawVariableValues($rawVariableValues);
+
+        $errors = DocumentValidator::validate(
+            QuerySecuritySchema::buildSchema(),
+            Parser::parse($query),
+            [$rule]
+        );
+
+        self::assertCount(1, $errors);
+        self::assertSame($expectedMessage, $errors[0]->getMessage());
+
+        // The complexity limit of 1 is not reported: without usable variable values the
+        // computed complexity is meaningless.
+        self::assertStringNotContainsString('Max query complexity', $errors[0]->getMessage());
+        self::assertSame(0, $rule->getQueryComplexity());
+    }
+
+    /** @return iterable<string, array{string, array<string, mixed>, string}> */
+    public static function variableCoercionFailureProvider(): iterable
+    {
+        yield 'missing required variable used by @include' => [
+            'query MyQuery($withDogs: Boolean!) { human { dogs(name: "Root") @include(if: $withDogs) { name } } }',
+            [],
+            'Variable "$withDogs" of required type "Boolean!" was not provided.',
+        ];
+
+        yield 'missing required variable used by @skip' => [
+            'query MyQuery($withoutDogs: Boolean!) { human { dogs(name: "Root") @skip(if: $withoutDogs) { name } } }',
+            [],
+            'Variable "$withoutDogs" of required type "Boolean!" was not provided.',
+        ];
+
+        yield 'missing required variable used as a field argument' => [
+            'query MyQuery($dog: String!) { human { dogs(name: $dog) { name } } }',
+            [],
+            'Variable "$dog" of required type "String!" was not provided.',
+        ];
+
+        yield 'invalid variable value used as a field argument' => [
+            'query MyQuery($dog: String!) { human { dogs(name: $dog) { name } } }',
+            ['dog' => 42],
+            'Variable "$dog" got invalid value 42; String cannot represent a non string value: 42',
+        ];
+    }
+
+    /** Every failing variable is reported, not just the first. */
+    public function testReportsAllVariableCoercionErrors(): void
+    {
+        $rule = $this->getRule(1);
+        $rule->setRawVariableValues([]);
+
+        $errors = DocumentValidator::validate(
+            QuerySecuritySchema::buildSchema(),
+            Parser::parse('query MyQuery($withDogs: Boolean!, $dog: String!) { human { dogs(name: $dog) @include(if: $withDogs) { name } } }'),
+            [$rule]
+        );
+
+        self::assertSame(
+            [
+                'Variable "$withDogs" of required type "Boolean!" was not provided.',
+                'Variable "$dog" of required type "String!" was not provided.',
+            ],
+            array_map(static fn (Error $error): string => $error->getMessage(), $errors)
+        );
+    }
+
+    /**
+     * Variable coercion errors carry the location of the offending variable definition,
+     * which the previously thrown error lost by concatenating messages.
+     */
+    public function testVariableCoercionErrorsHaveLocations(): void
+    {
+        $rule = $this->getRule(1);
+        $rule->setRawVariableValues([]);
+
+        $errors = DocumentValidator::validate(
+            QuerySecuritySchema::buildSchema(),
+            Parser::parse('query MyQuery($withDogs: Boolean!) { human { dogs(name: "Root") @include(if: $withDogs) { name } } }'),
+            [$rule]
+        );
+
+        self::assertCount(1, $errors);
+        self::assertEquals([new SourceLocation(1, 15)], $errors[0]->getLocations());
+    }
+
+    /**
+     * Coercion is only attempted once per document, even though the rule short-circuits
+     * the rest of the traversal after it fails.
+     */
+    public function testVariableCoercionFailureIsReportedOncePerDocument(): void
+    {
+        $query = <<<'GRAPHQL'
+        query MyQuery($withoutDogs: Boolean!) {
+          human {
+            firstName @skip(if: $withoutDogs)
+            dogs(name: "Root") @skip(if: $withoutDogs) { name }
+          }
+        }
+        GRAPHQL;
+
+        $rule = $this->getRule(1);
+        $rule->setRawVariableValues([]);
+
+        $errors = DocumentValidator::validate(
+            QuerySecuritySchema::buildSchema(),
+            Parser::parse($query),
+            [$rule]
+        );
+
+        self::assertCount(1, $errors);
+    }
+
+    /**
+     * A coercion failure leaves `fieldComplexity()` holding a partial sum of whatever
+     * was visited before it failed — here `human` is counted but `dogs` short-circuits
+     * to 0. That number must not reach `getQueryComplexity()`.
+     */
+    public function testDoesNotExposePartialComplexityAfterVariableCoercionFailure(): void
+    {
+        $rule = $this->getRule(100);
+        $rule->setRawVariableValues([]);
+
+        $errors = DocumentValidator::validate(
+            QuerySecuritySchema::buildSchema(),
+            Parser::parse('query MyQuery($dog: String!) { human { dogs(name: $dog) { name } } }'),
+            [$rule]
+        );
+
+        self::assertCount(1, $errors);
+        self::assertSame(0, $rule->getQueryComplexity());
+    }
+
+    /** A failed document must not leak its failure into the next one. */
+    public function testVariableCoercionFailureIsResetBetweenDocuments(): void
+    {
+        $query = 'query MyQuery($withoutDogs: Boolean!) { human { dogs(name: "Root") @skip(if: $withoutDogs) { name } } }';
+        $schema = QuerySecuritySchema::buildSchema();
+        $ast = Parser::parse($query);
+
+        $rule = $this->getRule(100);
+        $rule->setRawVariableValues([]);
+        self::assertCount(1, DocumentValidator::validate($schema, $ast, [$rule]));
+
+        $rule->setRawVariableValues(['withoutDogs' => false]);
+        self::assertSame([], DocumentValidator::validate($schema, $ast, [$rule]));
+        self::assertSame(3, $rule->getQueryComplexity());
+    }
 }