Enabled GraphQL\Error to include path to failed value during execution step (not location in source query, but exact path to value, including index in array, etc) + tests for errors

src/Error.php

@@ -2,36 +2,66 @@
 namespace GraphQL;
 
 use GraphQL\Language\Source;
+use GraphQL\Language\SourceLocation;
 
-// /graphql-js/src/error/GraphQLError.js
 
-class Error extends \Exception
+/**
+ * Class Error
+ * A GraphQLError describes an Error found during the parse, validate, or
+ * execute phases of performing a GraphQL operation. In addition to a message
+ * and stack trace, it also includes information about the locations in a
+ * GraphQL document and/or execution result that correspond to the Error.
+ *
+ * @package GraphQL
+ */
+class Error extends \Exception implements \JsonSerializable
 {
     /**
+     * A message describing the Error for debugging purposes.
+     *
      * @var string
      */
     public $message;
 
     /**
+     * An array of [ line => x, column => y] locations within the source GraphQL document
+     * which correspond to this error.
+     *
+     * Errors during validation often contain multiple locations, for example to
+     * point out two things with the same name. Errors during execution include a
+     * single location, the field which produced the error.
+     *
+     * @var SourceLocation[]
+     */
+    private $locations;
+
+    /**
+     * An array describing the JSON-path into the execution response which
+     * corresponds to this error. Only included for errors during execution.
+     *
+     * @var array
+     */
+    public $path;
+
+    /**
+     * An array of GraphQL AST Nodes corresponding to this error.
+     *
      * @var array
      */
     public $nodes;
 
     /**
-     * @var array
-     */
-    private $positions;
-
-    /**
-     * @var array<SourceLocation>
-     */
-    private $locations;
-
-    /**
+     * The source GraphQL document corresponding to this error.
+     *
      * @var Source|null
      */
     private $source;
 
+    /**
+     * @var array
+     */
+    private $positions;
+
     /**
      * Given an arbitrary Error, presumably thrown while attempting to execute a
      * GraphQL operation, produce a new GraphQLError aware of the location in the
@@ -39,10 +69,15 @@ class Error extends \Exception
      *
      * @param $error
      * @param array|null $nodes
+     * @param array|null $path
      * @return Error
      */
-    public static function createLocatedError($error, $nodes = null)
+    public static function createLocatedError($error, $nodes = null, $path = null)
     {
+        if ($error instanceof self) {
+            return $error;
+        }
+
         if ($error instanceof \Exception) {
             $message = $error->getMessage();
             $previous = $error;
@@ -51,7 +86,7 @@ class Error extends \Exception
             $previous = null;
         }
 
-        return new Error($message, $nodes, $previous);
+        return new Error($message, $nodes, null, null, $path, $previous);
     }
 
     /**
@@ -64,12 +99,14 @@ class Error extends \Exception
     }
 
     /**
-     * @param string|\Exception $message
+     * @param string $message
      * @param array|null $nodes
      * @param Source $source
-     * @param null $positions
+     * @param array|null $positions
+     * @param array|null $path
+     * @param \Exception $previous
      */
-    public function __construct($message, $nodes = null, \Exception $previous = null, Source $source = null, $positions = null)
+    public function __construct($message, $nodes = null, Source $source = null, $positions = null, $path = null, \Exception $previous = null)
     {
         parent::__construct($message, 0, $previous);
 
@@ -80,6 +117,7 @@ class Error extends \Exception
         $this->nodes = $nodes;
         $this->source = $source;
         $this->positions = $positions;
+        $this->path = $path;
     }
 
     /**
@@ -103,14 +141,14 @@ class Error extends \Exception
         if (null === $this->positions) {
             if (!empty($this->nodes)) {
                 $positions = array_map(function($node) { return isset($node->loc) ? $node->loc->start : null; }, $this->nodes);
-                $this->positions = array_filter($positions);
+                $this->positions = array_filter($positions, function($p) {return $p !== null;});
             }
         }
         return $this->positions;
     }
 
     /**
-     * @return array<SourceLocation>
+     * @return SourceLocation[]
      */
     public function getLocations()
     {
@@ -129,4 +167,38 @@ class Error extends \Exception
 
         return $this->locations;
     }
+
+    /**
+     * Returns array representation of error suitable for serialization
+     *
+     * @return array
+     */
+    public function toSerializableArray()
+    {
+        $arr = [
+            'message' => $this->getMessage(),
+        ];
+
+        $locations = Utils::map($this->getLocations(), function(SourceLocation $loc) {return $loc->toArray();});
+        if (!empty($locations)) {
+            $arr['locations'] = $locations;
+        }
+        if (!empty($this->path)) {
+            $arr['path'] = $this->path;
+        }
+
+        return $arr;
+    }
+
+    /**
+     * Specify data which should be serialized to JSON
+     * @link http://php.net/manual/en/jsonserializable.jsonserialize.php
+     * @return mixed data which can be serialized by <b>json_encode</b>,
+     * which is a value of any type other than a resource.
+     * @since 5.4.0
+     */
+    function jsonSerialize()
+    {
+        return $this->toSerializableArray();
+    }
 }

src/Executor/Executor.php

@@ -166,11 +166,12 @@ class Executor
         $type = self::getOperationRootType($exeContext->schema, $operation);
         $fields = self::collectFields($exeContext, $type, $operation->selectionSet, new \ArrayObject(), new \ArrayObject());
 
+        $path = [];
         if ($operation->operation === 'mutation') {
-            return self::executeFieldsSerially($exeContext, $type, $rootValue, $fields);
+            return self::executeFieldsSerially($exeContext, $type, $rootValue, $path, $fields);
         }
 
-        return self::executeFields($exeContext, $type, $rootValue, $fields);
+        return self::executeFields($exeContext, $type, $rootValue, $path, $fields);
     }
 
 
@@ -217,11 +218,12 @@ class Executor
      * Implements the "Evaluating selection sets" section of the spec
      * for "write" mode.
      */
-    private static function executeFieldsSerially(ExecutionContext $exeContext, ObjectType $parentType, $sourceValue, $fields)
+    private static function executeFieldsSerially(ExecutionContext $exeContext, ObjectType $parentType, $sourceValue, $path, $fields)
     {
         $results = [];
         foreach ($fields as $responseName => $fieldASTs) {
-            $result = self::resolveField($exeContext, $parentType, $sourceValue, $fieldASTs);
+            $path[] = $responseName;
+            $result = self::resolveField($exeContext, $parentType, $sourceValue, $fieldASTs, $path);
 
             if ($result !== self::$UNDEFINED) {
                 // Undefined means that field is not defined in schema
@@ -235,11 +237,11 @@ class Executor
      * Implements the "Evaluating selection sets" section of the spec
      * for "read" mode.
      */
-    private static function executeFields(ExecutionContext $exeContext, ObjectType $parentType, $source, $fields)
+    private static function executeFields(ExecutionContext $exeContext, ObjectType $parentType, $source, $path, $fields)
     {
         // Native PHP doesn't support promises.
         // Custom executor should be built for platforms like ReactPHP
-        return self::executeFieldsSerially($exeContext, $parentType, $source, $fields);
+        return self::executeFieldsSerially($exeContext, $parentType, $source, $path, $fields);
     }
 
 
@@ -387,7 +389,7 @@ class Executor
      * then calls completeValue to complete promises, serialize scalars, or execute
      * the sub-selection-set for objects.
      */
-    private static function resolveField(ExecutionContext $exeContext, ObjectType $parentType, $source, $fieldASTs)
+    private static function resolveField(ExecutionContext $exeContext, ObjectType $parentType, $source, $fieldASTs, $path)
     {
         $fieldAST = $fieldASTs[0];
 
@@ -416,6 +418,7 @@ class Executor
             'fieldASTs' => $fieldASTs,
             'returnType' => $returnType,
             'parentType' => $parentType,
+            'path' => $path,
             'schema' => $exeContext->schema,
             'fragments' => $exeContext->fragments,
             'rootValue' => $exeContext->rootValue,
@@ -446,6 +449,7 @@ class Executor
             $returnType,
             $fieldASTs,
             $info,
+            $path,
             $result
         );
 
@@ -463,34 +467,96 @@ class Executor
         }
     }
 
-    // This is a small wrapper around completeValue which detects and logs errors
-    // in the execution context.
-    public static function completeValueCatchingError(
+    /**
+     * This is a small wrapper around completeValue which detects and logs errors
+     * in the execution context.
+     *
+     * @param ExecutionContext $exeContext
+     * @param Type $returnType
+     * @param $fieldASTs
+     * @param ResolveInfo $info
+     * @param $path
+     * @param $result
+     * @return array|null
+     */
+    private static function completeValueCatchingError(
         ExecutionContext $exeContext,
         Type $returnType,
         $fieldASTs,
         ResolveInfo $info,
+        $path,
         $result
     )
     {
         // If the field type is non-nullable, then it is resolved without any
         // protection from errors.
         if ($returnType instanceof NonNull) {
-            return self::completeValue($exeContext, $returnType, $fieldASTs, $info, $result);
+            return self::completeValueWithLocatedError(
+                $exeContext,
+                $returnType,
+                $fieldASTs,
+                $info,
+                $path,
+                $result
+            );
         }
 
         // Otherwise, error protection is applied, logging the error and resolving
         // a null value for this field if one is encountered.
         try {
-            return self::completeValue($exeContext, $returnType, $fieldASTs, $info, $result);
+            return self::completeValueWithLocatedError(
+                $exeContext,
+                $returnType,
+                $fieldASTs,
+                $info,
+                $path,
+                $result
+            );
         } catch (Error $err) {
-            // If `completeValue` returned abruptly (threw an error), log the error
+            // If `completeValueWithLocatedError` returned abruptly (threw an error), log the error
             // and return null.
             $exeContext->addError($err);
             return null;
         }
     }
 
+
+    /**
+     * This is a small wrapper around completeValue which annotates errors with
+     * location information.
+     *
+     * @param ExecutionContext $exeContext
+     * @param Type $returnType
+     * @param $fieldASTs
+     * @param ResolveInfo $info
+     * @param $path
+     * @param $result
+     * @return array|null
+     * @throws Error
+     */
+    static function completeValueWithLocatedError(
+        ExecutionContext $exeContext,
+        Type $returnType,
+        $fieldASTs,
+        ResolveInfo $info,
+        $path,
+        $result
+    )
+    {
+        try {
+            return self::completeValue(
+                $exeContext,
+                $returnType,
+                $fieldASTs,
+                $info,
+                $path,
+                $result
+            );
+        } catch (\Exception $error) {
+            throw Error::createLocatedError($error, $fieldASTs, $path);
+        }
+    }
+
     /**
      * Implements the instructions for completeValue as defined in the
      * "Field entries" section of the spec.
@@ -516,15 +582,23 @@ class Executor
      * @param Type $returnType
      * @param Field[] $fieldASTs
      * @param ResolveInfo $info
+     * @param array $path
      * @param $result
      * @return array|null
      * @throws Error
      * @throws \Exception
      */
-    private static function completeValue(ExecutionContext $exeContext, Type $returnType, $fieldASTs, ResolveInfo $info, &$result)
+    private static function completeValue(
+        ExecutionContext $exeContext,
+        Type $returnType,
+        $fieldASTs,
+        ResolveInfo $info,
+        $path,
+        &$result
+    )
     {
         if ($result instanceof \Exception) {
-            throw Error::createLocatedError($result, $fieldASTs);
+            throw $result;
         }
 
         // If field type is NonNull, complete for inner type, and throw field error
@@ -535,12 +609,12 @@ class Executor
                 $returnType->getWrappedType(),
                 $fieldASTs,
                 $info,
+                $path,
                 $result
             );
             if ($completed === null) {
-                throw new Error(
-                    'Cannot return null for non-nullable field ' . $info->parentType . '.' . $info->fieldName . '.',
-                    $fieldASTs instanceof \ArrayObject ? $fieldASTs->getArrayCopy() : $fieldASTs
+                throw new \UnexpectedValueException(
+                    'Cannot return null for non-nullable field ' . $info->parentType . '.' . $info->fieldName . '.'
                 );
             }
             return $completed;
@@ -553,7 +627,7 @@ class Executor
 
         // If field type is List, complete each item in the list with the inner type
         if ($returnType instanceof ListOfType) {
-            return self::completeListValue($exeContext, $returnType, $fieldASTs, $info, $result);
+            return self::completeListValue($exeContext, $returnType, $fieldASTs, $info, $path, $result);
         }
 
         // If field type is Scalar or Enum, serialize to a valid value, returning
@@ -564,15 +638,15 @@ class Executor
         }
 
         if ($returnType instanceof AbstractType) {
-            return self::completeAbstractValue($exeContext, $returnType, $fieldASTs, $info, $result);
+            return self::completeAbstractValue($exeContext, $returnType, $fieldASTs, $info, $path, $result);
         }
 
         // Field type must be Object, Interface or Union and expect sub-selections.
         if ($returnType instanceof ObjectType) {
-            return self::completeObjectValue($exeContext, $returnType, $fieldASTs, $info, $result);
+            return self::completeObjectValue($exeContext, $returnType, $fieldASTs, $info, $path, $result);
         }
 
-        throw new Error("Cannot complete value of unexpected type \"{$returnType}\".");
+        throw new \RuntimeException("Cannot complete value of unexpected type \"{$returnType}\".");
     }
 
 
@@ -648,11 +722,12 @@ class Executor
      * @param AbstractType $returnType
      * @param $fieldASTs
      * @param ResolveInfo $info
+     * @param array $path
      * @param $result
      * @return mixed
      * @throws Error
      */
-    private static function completeAbstractValue(ExecutionContext $exeContext, AbstractType $returnType, $fieldASTs, ResolveInfo $info, &$result)
+    private static function completeAbstractValue(ExecutionContext $exeContext, AbstractType $returnType, $fieldASTs, ResolveInfo $info, $path, &$result)
     {
         $resolveType = $returnType->getResolveTypeFn();
 
@@ -675,7 +750,7 @@ class Executor
                 $fieldASTs
             );
         }
-        return self::completeObjectValue($exeContext, $runtimeType, $fieldASTs, $info, $result);
+        return self::completeObjectValue($exeContext, $runtimeType, $fieldASTs, $info, $path, $result);
     }
 
     /**
@@ -686,11 +761,12 @@ class Executor
      * @param ListOfType $returnType
      * @param $fieldASTs
      * @param ResolveInfo $info
+     * @param array $path
      * @param $result
      * @return array
      * @throws \Exception
      */
-    private static function completeListValue(ExecutionContext $exeContext, ListOfType $returnType, $fieldASTs, ResolveInfo $info, &$result)
+    private static function completeListValue(ExecutionContext $exeContext, ListOfType $returnType, $fieldASTs, ResolveInfo $info, $path, &$result)
     {
         $itemType = $returnType->getWrappedType();
         Utils::invariant(
@@ -698,9 +774,11 @@ class Executor
             'User Error: expected iterable, but did not find one for field ' . $info->parentType . '.' . $info->fieldName . '.'
         );
 
+        $i = 0;
         $tmp = [];
         foreach ($result as $item) {
-            $tmp[] = self::completeValueCatchingError($exeContext, $itemType, $fieldASTs, $info, $item);
+            $path[] = $i++;
+            $tmp[] = self::completeValueCatchingError($exeContext, $itemType, $fieldASTs, $info, $path, $item);
         }
         return $tmp;
     }
@@ -727,11 +805,12 @@ class Executor
      * @param ObjectType $returnType
      * @param $fieldASTs
      * @param ResolveInfo $info
+     * @param array $path
      * @param $result
      * @return array
      * @throws Error
      */
-    private static function completeObjectValue(ExecutionContext $exeContext, ObjectType $returnType, $fieldASTs, ResolveInfo $info, &$result)
+    private static function completeObjectValue(ExecutionContext $exeContext, ObjectType $returnType, $fieldASTs, ResolveInfo $info, $path, &$result)
     {
         // If there is an isTypeOf predicate function, call it with the
         // current result. If isTypeOf returns false, then raise an error rather
@@ -768,6 +847,6 @@ class Executor
             }
         }
 
-        return self::executeFields($exeContext, $returnType, $result, $subFieldASTs);
+        return self::executeFields($exeContext, $returnType, $result, $path, $subFieldASTs);
     }
 }

src/SyntaxError.php

@@ -18,7 +18,7 @@ class SyntaxError extends Error
             "Syntax Error {$source->name} ({$location->line}:{$location->column}) $description\n\n" .
             self::highlightSourceAtLocation($source, $location);
 
-        parent::__construct($syntaxError, null, null, $source, [$position]);
+        parent::__construct($syntaxError, null, $source, [$position]);
     }
 
     public static function highlightSourceAtLocation(Source $source, SourceLocation $location)

tests/ErrorTest.php

@@ -0,0 +1,113 @@
+<?php
+namespace GraphQL\Tests;
+
+use GraphQL\Error;
+use GraphQL\Language\Parser;
+use GraphQL\Language\Source;
+use GraphQL\Language\SourceLocation;
+
+class ErrorTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @it uses the stack of an original error
+     */
+    public function testUsesTheStackOfAnOriginalError()
+    {
+        $prev = new \Exception("Original");
+        $err = new Error('msg', null, null, null, null, $prev);
+
+        $this->assertSame($err->getPrevious(), $prev);
+    }
+
+    /**
+     * @it converts nodes to positions and locations
+     */
+    public function testConvertsNodesToPositionsAndLocations()
+    {
+        $source = new Source('{
+      field
+    }');
+        $ast = Parser::parse($source);
+        $fieldAST = $ast->definitions[0]->selectionSet->selections[0];
+        $e = new Error('msg', [ $fieldAST ]);
+
+        $this->assertEquals([$fieldAST], $e->nodes);
+        $this->assertEquals($source, $e->getSource());
+        $this->assertEquals([8], $e->getPositions());
+        $this->assertEquals([new SourceLocation(2, 7)], $e->getLocations());
+    }
+
+    /**
+     * @it converts node with loc.start === 0 to positions and locations
+     */
+    public function testConvertsNodeWithStart0ToPositionsAndLocations()
+    {
+        $source = new Source('{
+      field
+    }');
+        $ast = Parser::parse($source);
+        $operationAST = $ast->definitions[0];
+        $e = new Error('msg', [ $operationAST ]);
+
+        $this->assertEquals([$operationAST], $e->nodes);
+        $this->assertEquals($source, $e->getSource());
+        $this->assertEquals([0], $e->getPositions());
+        $this->assertEquals([new SourceLocation(1, 1)], $e->getLocations());
+    }
+
+    /**
+     * @it converts source and positions to locations
+     */
+    public function testConvertsSourceAndPositionsToLocations()
+    {
+        $source = new Source('{
+      field
+    }');
+        $e = new Error('msg', null, $source, [ 10 ]);
+
+        $this->assertEquals(null, $e->nodes);
+        $this->assertEquals($source, $e->getSource());
+        $this->assertEquals([10], $e->getPositions());
+        $this->assertEquals([new SourceLocation(2, 9)], $e->getLocations());
+    }
+
+    /**
+     * @it serializes to include message
+     */
+    public function testSerializesToIncludeMessage()
+    {
+        $e = new Error('msg');
+        $this->assertEquals(['message' => 'msg'], $e->toSerializableArray());
+    }
+
+    /**
+     * @it serializes to include message and locations
+     */
+    public function testSerializesToIncludeMessageAndLocations()
+    {
+        $node = Parser::parse('{ field }')->definitions[0]->selectionSet->selections[0];
+        $e = new Error('msg', [ $node ]);
+
+        $this->assertEquals(
+            ['message' => 'msg', 'locations' => [['line' => 1, 'column' => 3]]],
+            $e->toSerializableArray()
+        );
+    }
+
+    /**
+     * @it serializes to include path
+     */
+    public function testSerializesToIncludePath()
+    {
+        $e = new Error(
+            'msg',
+            null,
+            null,
+            null,
+            [ 'path', 3, 'to', 'field' ]
+        );
+
+        $this->assertEquals([ 'path', 3, 'to', 'field' ], $e->path);
+        $this->assertEquals(['message' => 'msg', 'path' => [ 'path', 3, 'to', 'field' ]], $e->toSerializableArray());
+    }
+}

tests/Executor/ExecutorTest.php

@@ -352,7 +352,7 @@ class ExecutorTest extends \PHPUnit_Framework_TestCase
                 return 'sync';
             },
             'syncError' => function () {
-                throw new Error('Error getting syncError');
+                throw new \Exception('Error getting syncError');
             },
             'syncRawError' => function() {
                 throw new \Exception('Error getting syncRawError');

tests/Executor/NonNullTest.php

@@ -12,8 +12,10 @@ use GraphQL\Type\Definition\Type;
 
 class NonNullTest extends \PHPUnit_Framework_TestCase
 {
-    /** @var Error */
+    /** @var \Exception */
     public $syncError;
+
+    /** @var \Exception */
     public $nonNullSyncError;
     public $throwingData;
     public $nullingData;
@@ -21,8 +23,8 @@ class NonNullTest extends \PHPUnit_Framework_TestCase
 
     public function setUp()
     {
-        $this->syncError = new Error('sync');
-        $this->nonNullSyncError = new Error('nonNullSync');
+        $this->syncError = new \Exception('sync');
+        $this->nonNullSyncError = new \Exception('nonNullSync');
 
         $this->throwingData = [
             'sync' => function () {
@@ -92,7 +94,7 @@ class NonNullTest extends \PHPUnit_Framework_TestCase
             ],
             'errors' => [
                 FormattedError::create(
-                    $this->syncError->message,
+                    $this->syncError->getMessage(),
                     [new SourceLocation(3, 9)]
                 )
             ]
@@ -118,7 +120,7 @@ class NonNullTest extends \PHPUnit_Framework_TestCase
                 'nest' => null
             ],
             'errors' => [
-                FormattedError::create($this->nonNullSyncError->message, [new SourceLocation(4, 11)])
+                FormattedError::create($this->nonNullSyncError->getMessage(), [new SourceLocation(4, 11)])
             ]
         ];
         $this->assertEquals($expected, Executor::execute($this->schema, $ast, $this->throwingData, null, [], 'Q')->toArray());
@@ -149,8 +151,8 @@ class NonNullTest extends \PHPUnit_Framework_TestCase
                 ]
             ],
             'errors' => [
-                FormattedError::create($this->syncError->message, [new SourceLocation(4, 11)]),
-                FormattedError::create($this->syncError->message, [new SourceLocation(6, 13)]),
+                FormattedError::create($this->syncError->getMessage(), [new SourceLocation(4, 11)]),
+                FormattedError::create($this->syncError->getMessage(), [new SourceLocation(6, 13)]),
             ]
         ];
         $this->assertEquals($expected, Executor::execute($this->schema, $ast, $this->throwingData, null, [], 'Q')->toArray());
@@ -243,7 +245,7 @@ class NonNullTest extends \PHPUnit_Framework_TestCase
         $expected = [
             'data' => null,
             'errors' => [
-                FormattedError::create($this->nonNullSyncError->message, [new SourceLocation(2, 17)])
+                FormattedError::create($this->nonNullSyncError->getMessage(), [new SourceLocation(2, 17)])
             ]
         ];
         $this->assertEquals($expected, Executor::execute($this->schema, Parser::parse($doc), $this->throwingData)->toArray());