Spec compliance: errors in buildExecutionContext() are caught and included in result rather than thrown

src/Executor/Executor.php

@@ -115,6 +115,9 @@ class Executor
             );
         }
 
+        $promiseAdapter = self::getPromiseAdapter();
+
+        try {
             $exeContext = self::buildExecutionContext(
                 $schema,
                 $ast,
@@ -124,7 +127,13 @@ class Executor
                 $operationName,
                 $fieldResolver
             );
-        $promiseAdapter = self::getPromiseAdapter();
+        } catch (Error $e) {
+            if ($promiseAdapter instanceof SyncPromiseAdapter) {
+                return new ExecutionResult(null, [$e]);
+            } else {
+                return $promiseAdapter->createFulfilled(new ExecutionResult(null, [$e]));
+            }
+        }
 
         $executor = new self($exeContext, $promiseAdapter);
         $result = $executor->executeQuery();
@@ -283,11 +292,30 @@ class Executor
         $fields = $this->collectFields($type, $operation->selectionSet, new \ArrayObject(), new \ArrayObject());
 
         $path = [];
-        if ($operation->operation === 'mutation') {
-            return $this->executeFieldsSerially($type, $rootValue, $path, $fields);
-        }
 
-        return $this->executeFields($type, $rootValue, $path, $fields);
+        // Errors from sub-fields of a NonNull type may propagate to the top level,
+        // at which point we still log the error and null the parent field, which
+        // in this case is the entire response.
+        //
+        // Similar to completeValueCatchingError.
+        try {
+            $result = $operation->operation === 'mutation' ?
+                $this->executeFieldsSerially($type, $rootValue, $path, $fields) :
+                $this->executeFields($type, $rootValue, $path, $fields);
+
+            $promise = $this->getPromise($result);
+            if ($promise) {
+                return $promise->then(null, function($error) {
+                    $this->exeContext->addError($error);
+                    return null;
+                });
+            }
+            return $result;
+
+        } catch (Error $error) {
+            $this->exeContext->addError($error);
+            return null;
+        }
     }
 
 

tests/Executor/ExecutorTest.php

@@ -589,9 +589,9 @@ class ExecutorTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @it throws if no operation is provided
+     * @it provides error if no operation is provided
      */
-    public function testThrowsIfNoOperationIsProvided()
+    public function testProvidesErrorIfNoOperationIsProvided()
     {
         $doc = 'fragment Example on Type { a }';
         $data = [ 'a' => 'b' ];
@@ -605,62 +605,84 @@ class ExecutorTest extends \PHPUnit_Framework_TestCase
             ])
         ]);
 
-        try {
+        $result = Executor::execute($schema, $ast, $data);
-            Executor::execute($schema, $ast, $data);
+        $expected = [
-            $this->fail('Expected exception was not thrown');
+            'errors' => [
-        } catch (Error $e) {
+                [
-            $this->assertEquals('Must provide an operation.', $e->getMessage());
+                    'message' => 'Must provide an operation.',
-        }
+                ]
+            ]
+        ];
+
+        $this->assertEquals($expected, $result->toArray());
     }
 
     /**
-     * @it throws if no operation name is provided with multiple operations
+     * @it errors if no op name is provided with multiple operations
      */
-    public function testThrowsIfNoOperationIsProvidedWithMultipleOperations()
+    public function testErrorsIfNoOperationIsProvidedWithMultipleOperations()
     {
         $doc = 'query Example { a } query OtherExample { a }';
-        $data = [ 'a' => 'b' ];
+        $data = ['a' => 'b'];
         $ast = Parser::parse($doc);
         $schema = new Schema([
             'query' => new ObjectType([
                 'name' => 'Type',
                 'fields' => [
-                    'a' => [ 'type' => Type::string() ],
+                    'a' => ['type' => Type::string()],
                 ]
             ])
         ]);
 
-        try {
+        $result = Executor::execute($schema, $ast, $data);
-            Executor::execute($schema, $ast, $data);
+
-            $this->fail('Expected exception is not thrown');
+        $expected = [
-        } catch (Error $err) {
+            'errors' => [
-            $this->assertEquals('Must provide operation name if query contains multiple operations.', $err->getMessage());
+                [
-        }
+                    'message' => 'Must provide operation name if query contains multiple operations.',
+                ]
+            ]
+        ];
+
+        $this->assertEquals($expected, $result->toArray());
     }
 
     /**
-     * @it throws if unknown operation name is provided
+     * @it errors if unknown operation name is provided
      */
-    public function testThrowsIfUnknownOperationNameIsProvided()
+    public function testErrorsIfUnknownOperationNameIsProvided()
     {
         $doc = 'query Example { a } query OtherExample { a }';
-        $data = [ 'a' => 'b' ];
         $ast = Parser::parse($doc);
         $schema = new Schema([
             'query' => new ObjectType([
                 'name' => 'Type',
                 'fields' => [
-                    'a' => [ 'type' => Type::string() ],
+                    'a' => ['type' => Type::string()],
                 ]
             ])
         ]);
 
-        try {
+
-            Executor::execute($schema, $ast, $data, null, null, 'UnknownExample');
+        $result = Executor::execute(
-            $this->fail('Expected exception was not thrown');
+            $schema,
-        } catch (Error $e) {
+            $ast,
-            $this->assertEquals('Unknown operation named "UnknownExample".', $e->getMessage());
+            null,
-        }
+            null,
+            null,
+            'UnknownExample'
+        );
+
+        $expected = [
+            'errors' => [
+                [
+                    'message' => 'Unknown operation named "UnknownExample".',
+                ]
+            ]
+
+        ];
+
+        $this->assertEquals($expected, $result->toArray());
     }
 
     /**
@@ -956,20 +978,24 @@ class ExecutorTest extends \PHPUnit_Framework_TestCase
             'query' => new ObjectType([
                 'name' => 'Query',
                 'fields' => [
-                    'foo' => ['type' => Type::string() ]
+                    'foo' => ['type' => Type::string()]
                 ]
             ])
         ]);
 
-        try {
+
-            Executor::execute($schema, $query);
+        $result = Executor::execute($schema, $query);
-            $this->fail('Expected exception was not thrown');
+
-        } catch (Error $e) {
+        $expected = [
-            $this->assertEquals([
+            'errors' => [
+                [
                     'message' => 'GraphQL cannot execute a request containing a ObjectTypeDefinition.',
-                'locations' => [['line' => 4, 'column' => 7]]
+                    'locations' => [['line' => 4, 'column' => 7]],
-            ], $e->toSerializableArray());
+                ]
-        }
+            ]
+        ];
+
+        $this->assertEquals($expected, $result->toArray());
     }
 
     /**

tests/Executor/VariablesTest.php

@@ -155,49 +155,49 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
 
         // errors on null for nested non-null:
         $params = ['input' => ['a' => 'foo', 'b' => 'bar', 'c' => null]];
-        $expected = FormattedError::create(
+        $result = Executor::execute($schema, $ast, null, null, $params);
+        $expected = [
+            'errors' => [
+                [
+                    'message' =>
                         'Variable "$input" got invalid value {"a":"foo","b":"bar","c":null}.'. "\n".
                         'In field "c": Expected "String!", found null.',
-            [new SourceLocation(2, 17)]
+                    'locations' => [['line' => 2, 'column' => 17]]
-        );
+                ]
-
+            ]
-        try {
+        ];
-            Executor::execute($schema, $ast, null, null, $params);
+        $this->assertEquals($expected, $result->toArray());
-            $this->fail('Expected exception not thrown');
-        } catch (Error $err) {
-            $this->assertEquals($expected, Error::formatError($err));
-        }
 
         // errors on incorrect type:
         $params = [ 'input' => 'foo bar' ];
-
+        $result = Executor::execute($schema, $ast, null, null, $params);
-        try {
+        $expected = [
-            Executor::execute($schema, $ast, null, null, $params);
+            'errors' => [
-            $this->fail('Expected exception not thrown');
+                [
-        } catch (Error $error) {
+                    'message' =>
-            $expected = FormattedError::create(
+                        'Variable "$input" got invalid value "foo bar".' . "\n" .
-                'Variable "$input" got invalid value "foo bar".'."\n".
                         'Expected "TestInputObject", found not an object.',
-                [new SourceLocation(2, 17)]
+                    'locations' => [ [ 'line' => 2, 'column' => 17 ] ],
-            );
+                ]
-            $this->assertEquals($expected, Error::formatError($error));
+            ]
-        }
+        ];
+        $this->assertEquals($expected, $result->toArray());
 
         // errors on omission of nested non-null:
         $params = ['input' => ['a' => 'foo', 'b' => 'bar']];
 
-        try {
+        $result = Executor::execute($schema, $ast, null, null, $params);
-            Executor::execute($schema, $ast, null, null, $params);
+        $expected = [
-            $this->fail('Expected exception not thrown');
+            'errors' => [
-        } catch (Error $e) {
+                [
-            $expected = FormattedError::create(
+                    'message' =>
                         'Variable "$input" got invalid value {"a":"foo","b":"bar"}.'. "\n".
                         'In field "c": Expected "String!", found null.',
-                [new SourceLocation(2, 17)]
+                    'locations' => [['line' => 2, 'column' => 17]]
-            );
+                ]
-
+            ]
-            $this->assertEquals($expected, Error::formatError($e));
+        ];
-        }
+        $this->assertEquals($expected, $result->toArray());
 
         // errors on deep nested errors and with many errors
         $nestedDoc = '
@@ -208,33 +208,35 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
         $nestedAst = Parser::parse($nestedDoc);
         $params = [ 'input' => [ 'na' => [ 'a' => 'foo' ] ] ];
 
-        try {
+        $result = Executor::execute($schema, $nestedAst, null, null, $params);
-            Executor::execute($schema, $nestedAst, null, null, $params);
+        $expected = [
-            $this->fail('Expected exception not thrown');
+            'errors' => [
-        } catch (Error $error) {
+                [
-            $expected = FormattedError::create(
+                    'message' =>
                         'Variable "$input" got invalid value {"na":{"a":"foo"}}.' . "\n" .
                         'In field "na": In field "c": Expected "String!", found null.' . "\n" .
                         'In field "nb": Expected "String!", found null.',
-                [new SourceLocation(2, 19)]
+                    'locations' => [['line' => 2, 'column' => 19]]
-            );
+                ]
-            $this->assertEquals($expected, Error::formatError($error));
+            ]
-        }
+        ];
+        $this->assertEquals($expected, $result->toArray());
+
 
         // errors on addition of unknown input field
         $params = ['input' => [ 'a' => 'foo', 'b' => 'bar', 'c' => 'baz', 'd' => 'dog' ]];
-
+        $result = Executor::execute($schema, $ast, null, null, $params);
-        try {
+        $expected = [
-            Executor::execute($schema, $ast, null, null, $params);
+            'errors' => [
-            $this->fail('Expected exception not thrown');
+                [
-        } catch (Error $e) {
+                    'message' =>
-            $expected = FormattedError::create(
                         'Variable "$input" got invalid value {"a":"foo","b":"bar","c":"baz","d":"dog"}.'."\n".
                         'In field "d": Expected type "ComplexScalar", found "dog".',
-                [new SourceLocation(2, 17)]
+                    'locations' => [['line' => 2, 'column' => 17]]
-            );
+                ]
-            $this->assertEquals($expected, Error::formatError($e));
+            ]
-        }
+        ];
+        $this->assertEquals($expected, $result->toArray());
     }
 
     // Describe: Handles nullable scalars
@@ -366,16 +368,17 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
         }
         ';
         $ast = Parser::parse($doc);
-        try {
+        $result = Executor::execute($this->schema(), $ast);
-            Executor::execute($this->schema(), $ast);
+
-            $this->fail('Expected exception not thrown');
+        $expected = [
-        } catch (Error $e) {
+            'errors' => [
-            $expected = FormattedError::create(
+                [
-                'Variable "$value" of required type "String!" was not provided.',
+                    'message' => 'Variable "$value" of required type "String!" was not provided.',
-                [new SourceLocation(2, 31)]
+                    'locations' => [['line' => 2, 'column' => 31]]
-            );
+                ]
-            $this->assertEquals($expected, Error::formatError($e));
+            ]
-        }
+        ];
+        $this->assertEquals($expected, $result->toArray());
     }
 
     /**
@@ -389,19 +392,18 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
         }
         ';
         $ast = Parser::parse($doc);
-
+        $result = Executor::execute($this->schema(), $ast, null, null, ['value' => null]);
-        try {
-            Executor::execute($this->schema(), $ast, null, null, ['value' => null]);
-            $this->fail('Expected exception not thrown');
-        } catch (Error $e) {
         $expected = [
+            'errors' => [
+                [
                     'message' =>
                         'Variable "$value" got invalid value null.' . "\n".
                         'Expected "String!", found null.',
                     'locations' => [['line' => 2, 'column' => 31]]
+                ]
+            ]
         ];
-            $this->assertEquals($expected, Error::formatError($e));
+        $this->assertEquals($expected, $result->toArray());
-        }
     }
 
     /**
@@ -546,18 +548,18 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
         }
         ';
         $ast = Parser::parse($doc);
-        $expected = FormattedError::create(
+        $result = Executor::execute($this->schema(), $ast, null, null, ['input' => null]);
+        $expected = [
+            'errors' => [
+                [
+                    'message' =>
                         'Variable "$input" got invalid value null.' . "\n" .
                         'Expected "[String]!", found null.',
-            [new SourceLocation(2, 17)]
+                    'locations' => [['line' => 2, 'column' => 17]]
-        );
+                ]
-
+            ]
-        try {
+        ];
-            $this->assertEquals($expected, Executor::execute($this->schema(), $ast, null, null, ['input' => null])->toArray());
+        $this->assertEquals($expected, $result->toArray());
-            $this->fail('Expected exception not thrown');
-        } catch (Error $e) {
-            $this->assertEquals($expected, Error::formatError($e));
-        }
     }
 
     /**
@@ -633,18 +635,18 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
         }
         ';
         $ast = Parser::parse($doc);
-        $expected = FormattedError::create(
+        $result = Executor::execute($this->schema(), $ast, null, null, ['input' => ['A', null, 'B']]);
+        $expected = [
+            'errors' => [
+                [
+                    'message' =>
                         'Variable "$input" got invalid value ["A",null,"B"].' . "\n" .
                         'In element #1: Expected "String!", found null.',
-            [new SourceLocation(2, 17)]
+                    'locations' => [ ['line' => 2, 'column' => 17] ]
-        );
+                ]
-
+            ]
-        try {
+        ];
-            Executor::execute($this->schema(), $ast, null, null, ['input' => ['A', null, 'B']]);
+        $this->assertEquals($expected, $result->toArray());
-            $this->fail('Expected exception not thrown');
-        } catch (Error $e) {
-            $this->assertEquals($expected, Error::formatError($e));
-        }
     }
 
     /**
@@ -658,17 +660,18 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
         }
         ';
         $ast = Parser::parse($doc);
-        $expected = FormattedError::create(
+        $result = Executor::execute($this->schema(), $ast, null, null, ['input' => null]);
+        $expected = [
+            'errors' => [
+                [
+                    'message' =>
                         'Variable "$input" got invalid value null.' . "\n" .
                         'Expected "[String!]!", found null.',
-            [new SourceLocation(2, 17)]
+                    'locations' => [ ['line' => 2, 'column' => 17] ]
-        );
+                ]
-        try {
+            ]
-            Executor::execute($this->schema(), $ast, null, null, ['input' => null]);
+        ];
-            $this->fail('Expected exception not thrown');
+        $this->assertEquals($expected, $result->toArray());
-        } catch (Error $e) {
-            $this->assertEquals($expected, Error::formatError($e));
-        }
     }
 
     /**
@@ -697,17 +700,18 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
         }
         ';
         $ast = Parser::parse($doc);
-        $expected = FormattedError::create(
+        $result = Executor::execute($this->schema(), $ast, null, null, ['input' => ['A', null, 'B']]);
+        $expected = [
+            'errors' => [
+                [
+                    'message' =>
                         'Variable "$input" got invalid value ["A",null,"B"].'."\n".
                         'In element #1: Expected "String!", found null.',
-            [new SourceLocation(2, 17)]
+                    'locations' => [ ['line' => 2, 'column' => 17] ]
-        );
+                ]
-        try {
+            ]
-            Executor::execute($this->schema(), $ast, null, null, ['input' => ['A', null, 'B']]);
+        ];
-            $this->fail('Expected exception not thrown');
+        $this->assertEquals($expected, $result->toArray());
-        } catch (Error $e) {
-            $this->assertEquals($expected, Error::formatError($e));
-        }
     }
 
     /**
@@ -722,19 +726,18 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
         ';
         $ast = Parser::parse($doc);
         $vars = [ 'input' => [ 'list' => [ 'A', 'B' ] ] ];
-
+        $result = Executor::execute($this->schema(), $ast, null, null, $vars);
-        try {
-            Executor::execute($this->schema(), $ast, null, null, $vars);
-            $this->fail('Expected exception not thrown');
-        } catch (Error $error) {
         $expected = [
+            'errors' => [
+                [
                     'message' =>
                         'Variable "$input" expected value of type "TestType!" which cannot ' .
                         'be used as an input type.',
                     'locations' => [['line' => 2, 'column' => 25]]
+                ]
+            ]
         ];
-            $this->assertEquals($expected, Error::formatError($error));
+        $this->assertEquals($expected, $result->toArray());
-        }
     }
 
     /**
@@ -750,17 +753,18 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
         $ast = Parser::parse($doc);
         $vars = ['input' => 'whoknows'];
 
-        try {
+        $result = Executor::execute($this->schema(), $ast, null, null, $vars);
-            Executor::execute($this->schema(), $ast, null, null, $vars);
+        $expected = [
-            $this->fail('Expected exception not thrown');
+            'errors' => [
-        } catch (Error $error) {
+                [
-            $expected = FormattedError::create(
+                    'message' =>
                         'Variable "$input" expected value of type "UnknownType!" which ' .
                         'cannot be used as an input type.',
-                [new SourceLocation(2, 25)]
+                    'locations' => [['line' => 2, 'column' => 25]]
-            );
+                ]
-            $this->assertEquals($expected, Error::formatError($error));
+            ]
-        }
+        ];
+        $this->assertEquals($expected, $result->toArray());
     }
 
     // Describe: Execute: Uses argument default values

tests/Language/ParserTest.php

@@ -1,12 +1,12 @@
 <?php
 namespace GraphQL\Tests\Language;
 
+use GraphQL\Error\InvariantViolation;
 use GraphQL\Language\AST\ArgumentNode;
 use GraphQL\Language\AST\FieldNode;
 use GraphQL\Language\AST\NameNode;
 use GraphQL\Language\AST\Node;
 use GraphQL\Language\AST\NodeKind;
-use GraphQL\Language\AST\NullValueNode;
 use GraphQL\Language\AST\SelectionSetNode;
 use GraphQL\Language\AST\StringValueNode;
 use GraphQL\Language\Parser;
@@ -17,6 +17,33 @@ use GraphQL\Utils;
 
 class ParserTest extends \PHPUnit_Framework_TestCase
 {
+    /**
+     * @it asserts that a source to parse was provided
+     */
+    public function testAssertsThatASourceToParseWasProvided()
+    {
+        try {
+            Parser::parse(null);
+            $this->fail('Expected exception was not thrown');
+        } catch (InvariantViolation $e) {
+            $this->assertEquals('GraphQL query body is expected to be string, but got NULL', $e->getMessage());
+        }
+
+        try {
+            Parser::parse(['a' => 'b']);
+            $this->fail('Expected exception was not thrown');
+        } catch (InvariantViolation $e) {
+            $this->assertEquals('GraphQL query body is expected to be string, but got array', $e->getMessage());
+        }
+
+        try {
+            Parser::parse(new \stdClass());
+            $this->fail('Expected exception was not thrown');
+        } catch (InvariantViolation $e) {
+            $this->assertEquals('GraphQL query body is expected to be string, but got stdClass', $e->getMessage());
+        }
+    }
+
     /**
      * @it parse provides useful errors
      */