Server: throw only when there is a configuration or logic error (invariant violation)

Author: vladar

src/Server/Helper.php

@@ -4,7 +4,6 @@
 use GraphQL\Error\Error;
 use GraphQL\Error\FormattedError;
 use GraphQL\Error\InvariantViolation;
-use GraphQL\Error\UserError;
 use GraphQL\Executor\ExecutionResult;
 use GraphQL\Executor\Executor;
 use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter;
@@ -36,7 +35,14 @@ class Helper
     public function executeOperation(ServerConfig $config, OperationParams $op)
     {
         $promiseAdapter = $config->getPromiseAdapter() ?: Executor::getPromiseAdapter();
-        $result = $this->promiseToExecuteOperation($promiseAdapter, $config, $op);
+
+        $errors = $this->validateOperationParams($op);
+
+        if (!empty($errors)) {
+            $result = $promiseAdapter->createFulfilled(new ExecutionResult(null, $errors));
+        } else {
+            $result = $this->promiseToExecuteOperation($promiseAdapter, $config, $op);
+        }
 
         if ($promiseAdapter instanceof SyncPromiseAdapter) {
             $result = $promiseAdapter->wait($result);
@@ -59,7 +65,12 @@ public function executeBatch(ServerConfig $config, array $operations)
         $result = [];
 
         foreach ($operations as $operation) {
-            $result[] = $this->promiseToExecuteOperation($promiseAdapter, $config, $operation);
+            $errors = $this->validateOperationParams($operation);
+            if (!empty($errors)) {
+                $result[] = $promiseAdapter->createFulfilled(new ExecutionResult(null, $errors));
+            } else {
+                $result[] = $this->promiseToExecuteOperation($promiseAdapter, $config, $operation);
+            }
         }
 
         $result = $promiseAdapter->all($result);
@@ -79,63 +90,52 @@ public function executeBatch(ServerConfig $config, array $operations)
      */
     private function promiseToExecuteOperation(PromiseAdapter $promiseAdapter, ServerConfig $config, OperationParams $op)
     {
-        $phpErrors = [];
+        try {
+            $doc = $op->queryId ? static::loadPersistedQuery($config, $op) : $op->query;
 
-        $execute = function() use ($config, $op, $promiseAdapter) {
-            try {
-                $doc = $op->queryId ? static::loadPersistedQuery($config, $op) : $op->query;
+            if (!$doc instanceof DocumentNode) {
+                $doc = Parser::parse($doc);
+            }
+            if ($op->isReadOnly() && AST::getOperation($doc, $op->operation) !== 'query') {
+                throw new Error("GET supports only query operation");
+            }
 
-                if (!$doc instanceof DocumentNode) {
-                    $doc = Parser::parse($doc);
-                }
-                if ($op->isReadOnly() && AST::isMutation($op->operation, $doc)) {
-                    throw new UserError("Cannot execute mutation in read-only context");
-                }
+            $validationErrors = DocumentValidator::validate(
+                $config->getSchema(),
+                $doc,
+                $this->resolveValidationRules($config, $op)
+            );
 
-                $validationErrors = DocumentValidator::validate(
+            if (!empty($validationErrors)) {
+                $result = $promiseAdapter->createFulfilled(
+                    new ExecutionResult(null, $validationErrors)
+                );
+            } else {
+                $result = Executor::promiseToExecute(
+                    $promiseAdapter,
                     $config->getSchema(),
                     $doc,
-                    $this->resolveValidationRules($config, $op)
-                );
-
-                if (!empty($validationErrors)) {
-                    return $promiseAdapter->createFulfilled(
-                        new ExecutionResult(null, $validationErrors)
-                    );
-                } else {
-                    return Executor::promiseToExecute(
-                        $promiseAdapter,
-                        $config->getSchema(),
-                        $doc,
-                        $config->getRootValue(),
-                        $config->getContext(),
-                        $op->variables,
-                        $op->operation,
-                        $config->getDefaultFieldResolver()
-                    );
-                }
-            } catch (Error $e) {
-                return $promiseAdapter->createFulfilled(
-                    new ExecutionResult(null, [$e])
+                    $config->getRootValue(),
+                    $config->getContext(),
+                    $op->variables,
+                    $op->operation,
+                    $config->getDefaultFieldResolver()
                 );
             }
-        };
-        if ($config->getDebug()) {
-            $execute = Utils::withErrorHandling($execute, $phpErrors);
+        } catch (Error $e) {
+            $result = $promiseAdapter->createFulfilled(
+                new ExecutionResult(null, [$e])
+            );
         }
-        $result = $execute();
 
-        $applyErrorFormatting = function (ExecutionResult $result) use ($config, $phpErrors) {
+        $applyErrorFormatting = function (ExecutionResult $result) use ($config) {
             if ($config->getDebug()) {
                 $errorFormatter = function($e) {
                     return FormattedError::createFromException($e, true);
                 };
             } else {
                 $errorFormatter = $config->getErrorFormatter();
             }
-            if (!empty($phpErrors)) {
-                $result->extensions['phpErrors'] = array_map($errorFormatter, $phpErrors);
-            }
             $result->setErrorFormatter($errorFormatter);
             return $result;
         };
@@ -160,7 +160,7 @@ public function loadPersistedQuery(ServerConfig $config, OperationParams $op)
         $loader = $config->getPersistentQueryLoader();
 
         if (!$loader) {
-            throw new UserError("Persisted queries are not supported by this server");
+            throw new Error("Persisted queries are not supported by this server");
         }
 
         $source = $loader($op->queryId, $op);
@@ -190,10 +190,10 @@ public function resolveValidationRules(ServerConfig $config, OperationParams $pa
             $validationRules = $validationRules($params);
 
             if (!is_array($validationRules)) {
-                throw new InvariantViolation(
-                    "Validation rules callable must return array of rules, but got: %s" .
+                throw new InvariantViolation(sprintf(
+                    "Expecting validation rules to be array or callable returning array, but got: %s",
                     Utils::printSafe($validationRules)
-                );
+                ));
             }
         }
 
@@ -210,19 +210,14 @@ public function resolveValidationRules(ServerConfig $config, OperationParams $pa
      *
      * @param callable|null $readRawBodyFn
      * @return OperationParams|OperationParams[]
+     * @throws Error
      */
     public function parseHttpRequest(callable $readRawBodyFn = null)
     {
         $method = isset($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : null;
 
         if ($method === 'GET') {
-            $request = array_change_key_case($_GET);
-
-            if (isset($request['query']) || isset($request['queryid']) || isset($request['documentid'])) {
-                $result = OperationParams::create($_GET, true);
-            } else {
-                throw new UserError('Cannot execute GET request without "query" or "queryId" parameter');
-            }
+            $result = OperationParams::create($_GET, true);
         } else if ($method === 'POST') {
             $contentType = isset($_SERVER['CONTENT_TYPE']) ? $_SERVER['CONTENT_TYPE'] : null;
 
@@ -234,18 +229,18 @@ public function parseHttpRequest(callable $readRawBodyFn = null)
                 $body = json_decode($rawBody ?: '', true);
 
                 if (json_last_error()) {
-                    throw new UserError("Could not parse JSON: " . json_last_error_msg());
+                    throw new Error("Could not parse JSON: " . json_last_error_msg());
                 }
                 if (!is_array($body)) {
-                    throw new UserError(
+                    throw new Error(
                         "GraphQL Server expects JSON object or array, but got " .
                         Utils::printSafeJson($body)
                     );
                 }
                 if (isset($body[0])) {
                     $result = [];
                     foreach ($body as $index => $entry) {
-                        $op = OperationParams::create($entry, true);
+                        $op = OperationParams::create($entry);
                         $result[] = $op;
                     }
                 } else {
@@ -254,12 +249,12 @@ public function parseHttpRequest(callable $readRawBodyFn = null)
             } else if (stripos($contentType, 'application/x-www-form-urlencoded') !== false) {
                 $result = OperationParams::create($_POST);
             } else if (null === $contentType) {
-                throw new UserError('Missing "Content-Type" header');
+                throw new Error('Missing "Content-Type" header');
             } else {
-                throw new UserError("Unexpected content type: " . Utils::printSafeJson($contentType));
+                throw new Error("Unexpected content type: " . Utils::printSafeJson($contentType));
             }
         } else {
-            throw new UserError('HTTP Method "' . $method . '" is not supported', 405);
+            throw new Error('HTTP Method "' . $method . '" is not supported');
         }
         return $result;
     }
@@ -276,77 +271,43 @@ public function readRawBody()
      * Checks validity of operation params and returns array of errors (empty array when params are valid)
      *
      * @param OperationParams $params
-     * @return array
+     * @return Error[]
      */
     public function validateOperationParams(OperationParams $params)
     {
         $errors = [];
         if (!$params->query && !$params->queryId) {
-            $errors[] = 'GraphQL Request must include at least one of those two parameters: "query" or "queryId"';
+            $errors[] = new Error('GraphQL Request must include at least one of those two parameters: "query" or "queryId"');
         }
         if ($params->query && $params->queryId) {
-            $errors[] = 'GraphQL Request parameters "query" and "queryId" are mutually exclusive';
+            $errors[] = new Error('GraphQL Request parameters "query" and "queryId" are mutually exclusive');
         }
 
         if ($params->query !== null && (!is_string($params->query) || empty($params->query))) {
-            $errors[] = 'GraphQL Request parameter "query" must be string, but got ' .
-                Utils::printSafeJson($params->query);
+            $errors[] = new Error(
+                'GraphQL Request parameter "query" must be string, but got ' .
+                Utils::printSafeJson($params->query)
+            );
         }
         if ($params->queryId !== null && (!is_string($params->queryId) || empty($params->queryId))) {
-            $errors[] = 'GraphQL Request parameter "queryId" must be string, but got ' .
-                Utils::printSafeJson($params->queryId);
+            $errors[] = new Error(
+                'GraphQL Request parameter "queryId" must be string, but got ' .
+                Utils::printSafeJson($params->queryId)
+            );
         }
 
         if ($params->operation !== null && (!is_string($params->operation) || empty($params->operation))) {
-            $errors[] = 'GraphQL Request parameter "operation" must be string, but got ' .
-                Utils::printSafeJson($params->operation);
+            $errors[] = new Error(
+                'GraphQL Request parameter "operation" must be string, but got ' .
+                Utils::printSafeJson($params->operation)
+            );
         }
         if ($params->variables !== null && (!is_array($params->variables) || isset($params->variables[0]))) {
-            $errors[] = 'GraphQL Request parameter "variables" must be object, but got ' .
-                Utils::printSafeJson($params->variables);
+            $errors[] = new Error(
+                'GraphQL Request parameter "variables" must be object or JSON string parsed to object, but got ' .
+                Utils::printSafeJson($params->getOriginalInput('variables'))
+            );
         }
         return $errors;
     }
-
-    /**
-     * Assertion to check that parsed body is valid instance of OperationParams (or array of instances)
-     *
-     * @param OperationParams|OperationParams[] $parsedBody
-     * @throws InvariantViolation
-     * @throws UserError
-     */
-    public function assertValidRequest($parsedBody)
-    {
-        if (is_array($parsedBody)) {
-            foreach ($parsedBody as $index => $entry) {
-                if (!$entry instanceof OperationParams) {
-                    throw new InvariantViolation(sprintf(
-                        'GraphQL Server: Parsed http request must be an instance of %s or array of such instances, '.
-                        'but got invalid array where entry at position %d is %s',
-                        OperationParams::class,
-                        $index,
-                        Utils::printSafe($entry)
-                    ));
-                }
-
-                $errors = $this->validateOperationParams($entry);
-
-                if (!empty($errors[0])) {
-                    $err = $index ? "Error in query #$index: {$errors[0]}" : $errors[0];
-                    throw new UserError($err);
-                }
-            }
-        } else if ($parsedBody instanceof OperationParams) {
-            $errors = $this->validateOperationParams($parsedBody);
-            if (!empty($errors[0])) {
-                throw new UserError($errors[0]);
-            }
-        } else {
-            throw new InvariantViolation(sprintf(
-                'GraphQL Server: Parsed http request must be an instance of %s or array of such instances, but got %s',
-                OperationParams::class,
-                Utils::printSafe($parsedBody)
-            ));
-        }
-    }
 }

src/Server/OperationParams.php

@@ -1,8 +1,6 @@
 <?php
 namespace GraphQL\Server;
 
-use GraphQL\Utils\Utils;
-
 /**
  * Class QueryParams
  * Represents all available parsed query parameters
@@ -52,9 +50,9 @@ class OperationParams
     public static function create(array $params, $readonly = false)
     {
         $instance = new static();
-        $instance->originalInput = $params;
 
         $params = array_change_key_case($params, CASE_LOWER);
+        $instance->originalInput = $params;
 
         $params += [
             'query' => null,
@@ -64,6 +62,13 @@ public static function create(array $params, $readonly = false)
             'variables' => null
         ];
 
+        if (is_string($params['variables'])) {
+            $tmp = json_decode($params['variables'], true);
+            if (!json_last_error()) {
+                $params['variables'] = $tmp;
+            }
+        }
+
         $instance->query = $params['query'];
         $instance->queryId = $params['queryid'] ?: $params['documentid'];
         $instance->operation = $params['operation'];
@@ -74,11 +79,12 @@ public static function create(array $params, $readonly = false)
     }
 
     /**
-     * @return array
+     * @param string $key
+     * @return mixed
      */
-    public function getOriginalInput()
+    public function getOriginalInput($key)
     {
-        return $this->originalInput;
+        return isset($this->originalInput[$key]) ? $this->originalInput[$key] : null;
     }
 
     /**

src/Server/StandardServer.php

@@ -1,6 +1,7 @@
 <?php
 namespace GraphQL\Server;
 
+use GraphQL\Error\InvariantViolation;
 use GraphQL\Executor\ExecutionResult;
 use GraphQL\Executor\Promise\Promise;
 
@@ -48,13 +49,13 @@ protected function __construct(ServerConfig $config)
     /**
      * @param OperationParams|OperationParams[] $parsedBody
      * @return ExecutionResult|ExecutionResult[]|Promise
+     * @throws InvariantViolation
      */
     public function executeRequest($parsedBody = null)
     {
         if (null !== $parsedBody) {
             $parsedBody = $this->helper->parseHttpRequest();
         }
-        $this->helper->assertValidRequest($parsedBody);
 
         if (is_array($parsedBody)) {
             return $this->helper->executeBatch($this->config, $parsedBody);