Merge pull request #408 from yura3d/error-handling-improvements

Error handling improvements
This commit is contained in:
Vladimir Razuvaev 2018-12-01 23:02:57 +07:00 committed by GitHub
commit 99f93939db
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 109 additions and 96 deletions

View File

@ -8,6 +8,7 @@ use Exception;
use GraphQL\Error\Error; use GraphQL\Error\Error;
use GraphQL\Language\AST\BooleanValueNode; use GraphQL\Language\AST\BooleanValueNode;
use GraphQL\Language\AST\EnumValueNode; use GraphQL\Language\AST\EnumValueNode;
use GraphQL\Language\AST\FieldNode;
use GraphQL\Language\AST\FloatValueNode; use GraphQL\Language\AST\FloatValueNode;
use GraphQL\Language\AST\IntValueNode; use GraphQL\Language\AST\IntValueNode;
use GraphQL\Language\AST\ListValueNode; use GraphQL\Language\AST\ListValueNode;
@ -21,6 +22,7 @@ use GraphQL\Language\Printer;
use GraphQL\Language\Visitor; use GraphQL\Language\Visitor;
use GraphQL\Type\Definition\EnumType; use GraphQL\Type\Definition\EnumType;
use GraphQL\Type\Definition\EnumValueDefinition; use GraphQL\Type\Definition\EnumValueDefinition;
use GraphQL\Type\Definition\FieldArgument;
use GraphQL\Type\Definition\InputObjectType; use GraphQL\Type\Definition\InputObjectType;
use GraphQL\Type\Definition\ListOfType; use GraphQL\Type\Definition\ListOfType;
use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\NonNull;
@ -46,8 +48,14 @@ class ValuesOfCorrectType extends ValidationRule
{ {
public function getVisitor(ValidationContext $context) public function getVisitor(ValidationContext $context)
{ {
$fieldName = '';
return [ return [
NodeKind::NULL => static function (NullValueNode $node) use ($context) { NodeKind::FIELD => [
'enter' => static function (FieldNode $node) use (&$fieldName) {
$fieldName = $node->name->value;
},
],
NodeKind::NULL => static function (NullValueNode $node) use ($context, &$fieldName) {
$type = $context->getInputType(); $type = $context->getInputType();
if (! ($type instanceof NonNull)) { if (! ($type instanceof NonNull)) {
return; return;
@ -55,30 +63,31 @@ class ValuesOfCorrectType extends ValidationRule
$context->reportError( $context->reportError(
new Error( new Error(
self::badValueMessage((string) $type, Printer::doPrint($node)), self::getBadValueMessage((string) $type, Printer::doPrint($node), null, $context, $fieldName),
$node $node
) )
); );
}, },
NodeKind::LST => function (ListValueNode $node) use ($context) { NodeKind::LST => function (ListValueNode $node) use ($context, &$fieldName) {
// Note: TypeInfo will traverse into a list's item type, so look to the // Note: TypeInfo will traverse into a list's item type, so look to the
// parent input type to check if it is a list. // parent input type to check if it is a list.
$type = Type::getNullableType($context->getParentInputType()); $type = Type::getNullableType($context->getParentInputType());
if (! $type instanceof ListOfType) { if (! $type instanceof ListOfType) {
$this->isValidScalar($context, $node); $this->isValidScalar($context, $node, $fieldName);
return Visitor::skipNode(); return Visitor::skipNode();
} }
}, },
NodeKind::OBJECT => function (ObjectValueNode $node) use ($context) { NodeKind::OBJECT => function (ObjectValueNode $node) use ($context, &$fieldName) {
// Note: TypeInfo will traverse into a list's item type, so look to the // Note: TypeInfo will traverse into a list's item type, so look to the
// parent input type to check if it is a list. // parent input type to check if it is a list.
$type = Type::getNamedType($context->getInputType()); $type = Type::getNamedType($context->getInputType());
if (! $type instanceof InputObjectType) { if (! $type instanceof InputObjectType) {
$this->isValidScalar($context, $node); $this->isValidScalar($context, $node, $fieldName);
return Visitor::skipNode(); return Visitor::skipNode();
} }
unset($fieldName);
// Ensure every required field exists. // Ensure every required field exists.
$inputFields = $type->getFields(); $inputFields = $type->getFields();
$nodeFields = iterator_to_array($node->fields); $nodeFields = iterator_to_array($node->fields);
@ -127,34 +136,36 @@ class ValuesOfCorrectType extends ValidationRule
) )
); );
}, },
NodeKind::ENUM => function (EnumValueNode $node) use ($context) { NodeKind::ENUM => function (EnumValueNode $node) use ($context, &$fieldName) {
$type = Type::getNamedType($context->getInputType()); $type = Type::getNamedType($context->getInputType());
if (! $type instanceof EnumType) { if (! $type instanceof EnumType) {
$this->isValidScalar($context, $node); $this->isValidScalar($context, $node, $fieldName);
} elseif (! $type->getValue($node->value)) { } elseif (! $type->getValue($node->value)) {
$context->reportError( $context->reportError(
new Error( new Error(
self::badValueMessage( self::getBadValueMessage(
$type->name, $type->name,
Printer::doPrint($node), Printer::doPrint($node),
$this->enumTypeSuggestion($type, $node) $this->enumTypeSuggestion($type, $node),
$context,
$fieldName
), ),
$node $node
) )
); );
} }
}, },
NodeKind::INT => function (IntValueNode $node) use ($context) { NodeKind::INT => function (IntValueNode $node) use ($context, &$fieldName) {
$this->isValidScalar($context, $node); $this->isValidScalar($context, $node, $fieldName);
}, },
NodeKind::FLOAT => function (FloatValueNode $node) use ($context) { NodeKind::FLOAT => function (FloatValueNode $node) use ($context, &$fieldName) {
$this->isValidScalar($context, $node); $this->isValidScalar($context, $node, $fieldName);
}, },
NodeKind::STRING => function (StringValueNode $node) use ($context) { NodeKind::STRING => function (StringValueNode $node) use ($context, &$fieldName) {
$this->isValidScalar($context, $node); $this->isValidScalar($context, $node, $fieldName);
}, },
NodeKind::BOOLEAN => function (BooleanValueNode $node) use ($context) { NodeKind::BOOLEAN => function (BooleanValueNode $node) use ($context, &$fieldName) {
$this->isValidScalar($context, $node); $this->isValidScalar($context, $node, $fieldName);
}, },
]; ];
} }
@ -165,7 +176,7 @@ class ValuesOfCorrectType extends ValidationRule
($message ? "; ${message}" : '.'); ($message ? "; ${message}" : '.');
} }
private function isValidScalar(ValidationContext $context, ValueNode $node) private function isValidScalar(ValidationContext $context, ValueNode $node, $fieldName)
{ {
// Report any error at the full type expected by the location. // Report any error at the full type expected by the location.
$locationType = $context->getInputType(); $locationType = $context->getInputType();
@ -179,10 +190,12 @@ class ValuesOfCorrectType extends ValidationRule
if (! $type instanceof ScalarType) { if (! $type instanceof ScalarType) {
$context->reportError( $context->reportError(
new Error( new Error(
self::badValueMessage( self::getBadValueMessage(
(string) $locationType, (string) $locationType,
Printer::doPrint($node), Printer::doPrint($node),
$this->enumTypeSuggestion($type, $node) $this->enumTypeSuggestion($type, $node),
$context,
$fieldName
), ),
$node $node
) )
@ -199,32 +212,28 @@ class ValuesOfCorrectType extends ValidationRule
// Ensure a reference to the original error is maintained. // Ensure a reference to the original error is maintained.
$context->reportError( $context->reportError(
new Error( new Error(
self::badValueMessage( self::getBadValueMessage(
(string) $locationType, (string) $locationType,
Printer::doPrint($node), Printer::doPrint($node),
$error->getMessage() $error->getMessage(),
$context,
$fieldName
), ),
$node, $node
null,
null,
null,
$error
) )
); );
} catch (Throwable $error) { } catch (Throwable $error) {
// Ensure a reference to the original error is maintained. // Ensure a reference to the original error is maintained.
$context->reportError( $context->reportError(
new Error( new Error(
self::badValueMessage( self::getBadValueMessage(
(string) $locationType, (string) $locationType,
Printer::doPrint($node), Printer::doPrint($node),
$error->getMessage() $error->getMessage(),
$context,
$fieldName
), ),
$node, $node
null,
null,
null,
$error
) )
); );
} }
@ -247,6 +256,12 @@ class ValuesOfCorrectType extends ValidationRule
} }
} }
public static function badArgumentValueMessage($typeName, $valueName, $fieldName, $argName, $message = null)
{
return sprintf('Field "%s" argument "%s" requires type %s, found %s', $fieldName, $argName, $typeName, $valueName) .
($message ? sprintf('; %s', $message) : '.');
}
public static function requiredFieldMessage($typeName, $fieldName, $fieldTypeName) public static function requiredFieldMessage($typeName, $fieldName, $fieldTypeName)
{ {
return sprintf('Field %s.%s of required type %s was not provided.', $typeName, $fieldName, $fieldTypeName); return sprintf('Field %s.%s of required type %s was not provided.', $typeName, $fieldName, $fieldTypeName);
@ -257,4 +272,15 @@ class ValuesOfCorrectType extends ValidationRule
return sprintf('Field "%s" is not defined by type %s', $fieldName, $typeName) . return sprintf('Field "%s" is not defined by type %s', $fieldName, $typeName) .
($message ? sprintf('; %s', $message) : '.'); ($message ? sprintf('; %s', $message) : '.');
} }
private static function getBadValueMessage($typeName, $valueName, $message = null, $context = null, $fieldName = null)
{
if ($context) {
$arg = $context->getArgument();
if ($arg) {
return self::badArgumentValueMessage($typeName, $valueName, $fieldName, $arg->name, $message);
}
}
return self::badValueMessage($typeName, $valueName, $message);
}
} }

View File

@ -231,7 +231,7 @@ class EnumTypeTest extends TestCase
'{ colorEnum(fromEnum: "GREEN") }', '{ colorEnum(fromEnum: "GREEN") }',
null, null,
[ [
'message' => 'Expected type Color, found "GREEN"; Did you mean the enum value GREEN?', 'message' => 'Field "colorEnum" argument "fromEnum" requires type Color, found "GREEN"; Did you mean the enum value GREEN?',
'locations' => [new SourceLocation(1, 23)], 'locations' => [new SourceLocation(1, 23)],
] ]
); );
@ -268,7 +268,7 @@ class EnumTypeTest extends TestCase
'{ colorEnum(fromEnum: GREENISH) }', '{ colorEnum(fromEnum: GREENISH) }',
null, null,
[ [
'message' => 'Expected type Color, found GREENISH; Did you mean the enum value GREEN?', 'message' => 'Field "colorEnum" argument "fromEnum" requires type Color, found GREENISH; Did you mean the enum value GREEN?',
'locations' => [new SourceLocation(1, 23)], 'locations' => [new SourceLocation(1, 23)],
] ]
); );
@ -283,7 +283,7 @@ class EnumTypeTest extends TestCase
'{ colorEnum(fromEnum: green) }', '{ colorEnum(fromEnum: green) }',
null, null,
[ [
'message' => 'Expected type Color, found green; Did you mean the enum value GREEN?', 'message' => 'Field "colorEnum" argument "fromEnum" requires type Color, found green; Did you mean the enum value GREEN?',
'locations' => [new SourceLocation(1, 23)], 'locations' => [new SourceLocation(1, 23)],
] ]
); );
@ -313,7 +313,7 @@ class EnumTypeTest extends TestCase
$this->expectFailure( $this->expectFailure(
'{ colorEnum(fromEnum: 1) }', '{ colorEnum(fromEnum: 1) }',
null, null,
'Expected type Color, found 1.' 'Field "colorEnum" argument "fromEnum" requires type Color, found 1.'
); );
} }
@ -325,7 +325,7 @@ class EnumTypeTest extends TestCase
$this->expectFailure( $this->expectFailure(
'{ colorEnum(fromInt: GREEN) }', '{ colorEnum(fromInt: GREEN) }',
null, null,
'Expected type Int, found GREEN.' 'Field "colorEnum" argument "fromInt" requires type Int, found GREEN.'
); );
} }

View File

@ -38,7 +38,7 @@ class ValidationTest extends ValidatorTestCase
'; ';
$expectedError = [ $expectedError = [
'message' => 'Expected type Invalid, found "bad value"; Invalid scalar is always invalid: bad value', 'message' => 'Field "invalidArg" argument "arg" requires type Invalid, found "bad value"; Invalid scalar is always invalid: bad value',
'locations' => [['line' => 3, 'column' => 25]], 'locations' => [['line' => 3, 'column' => 25]],
]; ];

View File

@ -240,7 +240,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('String', '1', 4, 39), $this->badValueWithMessage('Field "stringArgField" argument "stringArg" requires type String, found 1.', 4, 39),
] ]
); );
} }
@ -257,6 +257,11 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
); );
} }
private function badValueWithMessage($message, $line, $column)
{
return FormattedError::create($message, [new SourceLocation($line, $column)]);
}
/** /**
* @see it('Float into String') * @see it('Float into String')
*/ */
@ -272,7 +277,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('String', '1.0', 4, 39), $this->badValueWithMessage('Field "stringArgField" argument "stringArg" requires type String, found 1.0.', 4, 39),
] ]
); );
} }
@ -294,7 +299,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('String', 'true', 4, 39), $this->badValueWithMessage('Field "stringArgField" argument "stringArg" requires type String, found true.', 4, 39),
] ]
); );
} }
@ -314,7 +319,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('String', 'BAR', 4, 39), $this->badValueWithMessage('Field "stringArgField" argument "stringArg" requires type String, found BAR.', 4, 39),
] ]
); );
} }
@ -334,7 +339,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Int', '"3"', 4, 33), $this->badValueWithMessage('Field "intArgField" argument "intArg" requires type Int, found "3".', 4, 33),
] ]
); );
} }
@ -354,7 +359,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Int', '829384293849283498239482938', 4, 33), $this->badValueWithMessage('Field "intArgField" argument "intArg" requires type Int, found 829384293849283498239482938.', 4, 33),
] ]
); );
} }
@ -376,7 +381,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Int', 'FOO', 4, 33), $this->badValueWithMessage('Field "intArgField" argument "intArg" requires type Int, found FOO.', 4, 33),
] ]
); );
} }
@ -396,7 +401,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Int', '3.0', 4, 33), $this->badValueWithMessage('Field "intArgField" argument "intArg" requires type Int, found 3.0.', 4, 33),
] ]
); );
} }
@ -416,7 +421,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Int', '3.333', 4, 33), $this->badValueWithMessage('Field "intArgField" argument "intArg" requires type Int, found 3.333.', 4, 33),
] ]
); );
} }
@ -436,7 +441,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Float', '"3.333"', 4, 37), $this->badValueWithMessage('Field "floatArgField" argument "floatArg" requires type Float, found "3.333".', 4, 37),
] ]
); );
} }
@ -456,7 +461,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Float', 'true', 4, 37), $this->badValueWithMessage('Field "floatArgField" argument "floatArg" requires type Float, found true.', 4, 37),
] ]
); );
} }
@ -478,7 +483,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Float', 'FOO', 4, 37), $this->badValueWithMessage('Field "floatArgField" argument "floatArg" requires type Float, found FOO.', 4, 37),
] ]
); );
} }
@ -498,7 +503,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Boolean', '2', 4, 41), $this->badValueWithMessage('Field "booleanArgField" argument "booleanArg" requires type Boolean, found 2.', 4, 41),
] ]
); );
} }
@ -518,7 +523,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Boolean', '1.0', 4, 41), $this->badValueWithMessage('Field "booleanArgField" argument "booleanArg" requires type Boolean, found 1.0.', 4, 41),
] ]
); );
} }
@ -540,7 +545,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Boolean', '"true"', 4, 41), $this->badValueWithMessage('Field "booleanArgField" argument "booleanArg" requires type Boolean, found "true".', 4, 41),
] ]
); );
} }
@ -560,7 +565,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Boolean', 'TRUE', 4, 41), $this->badValueWithMessage('Field "booleanArgField" argument "booleanArg" requires type Boolean, found TRUE.', 4, 41),
] ]
); );
} }
@ -580,7 +585,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('ID', '1.0', 4, 31), $this->badValueWithMessage('Field "idArgField" argument "idArg" requires type ID, found 1.0.', 4, 31),
] ]
); );
} }
@ -600,7 +605,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('ID', 'true', 4, 31), $this->badValueWithMessage('Field "idArgField" argument "idArg" requires type ID, found true.', 4, 31),
] ]
); );
} }
@ -622,7 +627,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('ID', 'SOMETHING', 4, 31), $this->badValueWithMessage('Field "idArgField" argument "idArg" requires type ID, found SOMETHING.', 4, 31),
] ]
); );
} }
@ -642,7 +647,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('DogCommand', '2', 4, 41), $this->badValueWithMessage('Field "doesKnowCommand" argument "dogCommand" requires type DogCommand, found 2.', 4, 41),
] ]
); );
} }
@ -662,7 +667,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('DogCommand', '1.0', 4, 41), $this->badValueWithMessage('Field "doesKnowCommand" argument "dogCommand" requires type DogCommand, found 1.0.', 4, 41),
] ]
); );
} }
@ -684,13 +689,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue( $this->badValueWithMessage('Field "doesKnowCommand" argument "dogCommand" requires type DogCommand, found "SIT"; Did you mean the enum value SIT?', 4, 41),
'DogCommand',
'"SIT"',
4,
41,
'Did you mean the enum value SIT?'
),
] ]
); );
} }
@ -710,7 +709,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('DogCommand', 'true', 4, 41), $this->badValueWithMessage('Field "doesKnowCommand" argument "dogCommand" requires type DogCommand, found true.', 4, 41),
] ]
); );
} }
@ -730,7 +729,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('DogCommand', 'JUGGLE', 4, 41), $this->badValueWithMessage('Field "doesKnowCommand" argument "dogCommand" requires type DogCommand, found JUGGLE.', 4, 41),
] ]
); );
} }
@ -750,13 +749,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue( $this->badValueWithMessage('Field "doesKnowCommand" argument "dogCommand" requires type DogCommand, found sit; Did you mean the enum value SIT?', 4, 41),
'DogCommand',
'sit',
4,
41,
'Did you mean the enum value SIT?'
),
] ]
); );
} }
@ -846,7 +839,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('String', '2', 4, 55), $this->badValueWithMessage('Field "stringListArgField" argument "stringListArg" requires type String, found 2.', 4, 55),
] ]
); );
} }
@ -866,7 +859,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('[String]', '1', 4, 47), $this->badValueWithMessage('Field "stringListArgField" argument "stringListArg" requires type [String], found 1.', 4, 47),
] ]
); );
} }
@ -1060,8 +1053,8 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Int!', '"two"', 4, 32), $this->badValueWithMessage('Field "multipleReqs" argument "req2" requires type Int!, found "two".', 4, 32),
$this->badValue('Int!', '"one"', 4, 45), $this->badValueWithMessage('Field "multipleReqs" argument "req1" requires type Int!, found "one".', 4, 45),
] ]
); );
} }
@ -1081,7 +1074,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Int!', '"one"', 4, 32), $this->badValueWithMessage('Field "multipleReqs" argument "req1" requires type Int!, found "one".', 4, 32),
] ]
); );
} }
@ -1103,7 +1096,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Int!', 'null', 4, 32), $this->badValueWithMessage('Field "multipleReqs" argument "req1" requires type Int!, found null.', 4, 32),
] ]
); );
} }
@ -1277,7 +1270,7 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('String', '2', 5, 40), $this->badValueWithMessage('Field "complexArgField" argument "complexArg" requires type String, found 2.', 5, 40),
] ]
); );
} }
@ -1340,19 +1333,13 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue( $this->badValueWithMessage('Field "invalidArg" argument "arg" requires type Invalid, found 123; Invalid scalar is always invalid: 123', 3, 27),
'Invalid',
'123',
3,
27,
'Invalid scalar is always invalid: 123'
),
] ]
); );
self::assertEquals( self::assertEquals(
'Invalid scalar is always invalid: 123', 'Field "invalidArg" argument "arg" requires type Invalid, found 123; Invalid scalar is always invalid: 123',
$errors[0]->getPrevious()->getMessage() $errors[0]->getMessage()
); );
} }
@ -1411,8 +1398,8 @@ class ValuesOfCorrectTypeTest extends ValidatorTestCase
} }
', ',
[ [
$this->badValue('Boolean!', '"yes"', 3, 28), $this->badValueWithMessage('Field "dog" argument "if" requires type Boolean!, found "yes".', 3, 28),
$this->badValue('Boolean!', 'ENUM', 4, 28), $this->badValueWithMessage('Field "name" argument "if" requires type Boolean!, found ENUM.', 4, 28),
] ]
); );
} }