Merge pull request #15 from moufmouf/return_code_if_error

Error HTTP code when an error occurs

Author: moufmouf

.travis.yml

@@ -14,7 +14,6 @@ matrix:
   fast_finish: true
   include:
       # Test the latest stable release
-    - php: 7.1
     - php: 7.2
     - php: 7.3
       env: COVERAGE=true PHPUNIT_FLAGS="-v --coverage-text"
@@ -31,7 +30,7 @@ matrix:
     # Minimum supported dependencies with the latest and oldest PHP version
     - php: 7.3
       env: COMPOSER_FLAGS="--prefer-stable --prefer-lowest" SYMFONY_DEPRECATIONS_HELPER="weak_vendors"
-    - php: 7.1
+    - php: 7.2
       env: COMPOSER_FLAGS="--prefer-stable --prefer-lowest" SYMFONY_DEPRECATIONS_HELPER="weak_vendors"
     # Dev-master is allowed to fail.
     - env: STABILITY="dev"

Controller/GraphqliteController.php

@@ -4,11 +4,15 @@
 namespace TheCodingMachine\Graphqlite\Bundle\Controller;
 
 
+use function array_map;
+use GraphQL\Error\ClientAware;
 use GraphQL\Error\Debug;
+use GraphQL\Error\Error;
 use GraphQL\Executor\ExecutionResult;
 use GraphQL\Executor\Promise\Promise;
 use GraphQL\Server\StandardServer;
 use GraphQL\Upload\UploadMiddleware;
+use function in_array;
 use function json_decode;
 use Psr\Http\Message\ServerRequestInterface;
 use RuntimeException;
@@ -80,26 +84,67 @@ public function handleRequest(Request $request): Response
         $uploadMiddleware = new UploadMiddleware();
         $psr7Request = $uploadMiddleware->processRequest($psr7Request);
 
-        $result = $this->handlePsr7Request($psr7Request);
-
-        return new JsonResponse($result);
+        return $this->handlePsr7Request($psr7Request);
     }
 
-    private function handlePsr7Request(ServerRequestInterface $request): array
+    private function handlePsr7Request(ServerRequestInterface $request): JsonResponse
     {
         $result = $this->standardServer->executePsrRequest($request);
 
         if ($result instanceof ExecutionResult) {
-            return $result->toArray($this->debug);
+            return new JsonResponse($result->toArray($this->debug), $this->decideHttpStatusCode($result));
         }
         if (is_array($result)) {
-            return array_map(function (ExecutionResult $executionResult) {
+            $finalResult = array_map(function (ExecutionResult $executionResult) {
                 return $executionResult->toArray($this->debug);
             }, $result);
+            // Let's return the highest result.
+            $statuses = array_map([$this, 'decideHttpStatusCode'], $result);
+            $status = max($statuses);
+            return new JsonResponse($finalResult, $status);
         }
         if ($result instanceof Promise) {
             throw new RuntimeException('Only SyncPromiseAdapter is supported');
         }
         throw new RuntimeException('Unexpected response from StandardServer::executePsrRequest'); // @codeCoverageIgnore
     }
+
+    /**
+     * Decides the HTTP status code based on the answer.
+     *
+     * @see https://github.com/APIs-guru/graphql-over-http#status-codes
+     */
+    private function decideHttpStatusCode(ExecutionResult $result): int
+    {
+        // If the data entry in the response has any value other than null (when the operation has successfully executed without error) then the response should use the 200 (OK) status code.
+        if ($result->data !== null) {
+            return 200;
+        }
+
+        if (empty($result->errors)) {
+            return 200;
+        }
+
+        $status = 0;
+        // There might be many errors. Let's return the highest code we encounter.
+        foreach ($result->errors as $error) {
+            if ($error->getCategory() === Error::CATEGORY_GRAPHQL) {
+                $code = 400;
+            } else {
+                $code = $error->getCode();
+                if (!isset(Response::$statusTexts[$code])) {
+                    // The exception code is not a valid HTTP code. Let's ignore it
+                    continue;
+                }
+            }
+            $status = max($status, $code);
+        }
+
+        // If exceptions have been thrown and they have not a "HTTP like code", let's throw a 500.
+        if ($status < 200) {
+            $status = 500;
+        }
+
+        return $status;
+    }
 }

Tests/Fixtures/Controller/MyException.php

@@ -0,0 +1,37 @@
+<?php
+
+
+namespace TheCodingMachine\Graphqlite\Bundle\Tests\Fixtures\Controller;
+
+
+use GraphQL\Error\ClientAware;
+
+class MyException extends \Exception implements ClientAware
+{
+
+    /**
+     * Returns true when exception message is safe to be displayed to a client.
+     *
+     * @return bool
+     *
+     * @api
+     */
+    public function isClientSafe()
+    {
+        return true;
+    }
+
+    /**
+     * Returns string describing a category of the error.
+     *
+     * Value "graphql" is reserved for errors produced by query parsing or validation, do not use it.
+     *
+     * @return string
+     *
+     * @api
+     */
+    public function getCategory()
+    {
+        return 'foobar';
+    }
+}

Tests/Fixtures/Controller/TestGraphqlController.php

@@ -56,4 +56,13 @@ public function contacts(): ArrayResult
     {
         return new ArrayResult([new Contact('Mouf')]);
     }
+
+    /**
+     * @Query()
+     * @return string
+     */
+    public function triggerError(): string
+    {
+        throw new MyException('Boom');
+    }
 }

Tests/FunctionalTest.php

@@ -58,4 +58,29 @@ public function testServiceWiring()
             ]
         ], $result);
     }
+
+    public function testErrors()
+    {
+        $kernel = new GraphqliteTestingKernel('test', true);
+        $kernel->boot();
+
+        $request = Request::create('/graphql', 'GET', ['query' => '
+        { 
+          notExists
+        }']);
+
+        $response = $kernel->handle($request);
+
+        $this->assertSame(400, $response->getStatusCode());
+
+        $request = Request::create('/graphql', 'GET', ['query' => '
+        { 
+          triggerError
+        }']);
+
+        $response = $kernel->handle($request);
+
+        $this->assertSame(500, $response->getStatusCode());
+
+    }
 }

composer.json

@@ -16,7 +16,7 @@
   }
   ],
   "require" : {
-    "php" : ">=7.1",
+    "php" : ">=7.2",
     "thecodingmachine/graphqlite" : "~4.0.0",
     "symfony/framework-bundle": "^4.1.9",
     "doctrine/annotations": "^1.6",