Merge pull request #337 from spawnia/patch-1

Add Debug::RETHROW_UNSAFE_EXCEPTIONS flag

docs/error-handling.md

@@ -84,7 +84,7 @@ To change default **"Internal server error"** message to something else, use:
 GraphQL\Error\FormattedError::setInternalErrorMessage("Unexpected error");
 ```
 
-#Debugging tools
+# Debugging tools
 
 During development or debugging use `$result->toArray(true)` to add **debugMessage** key to 
 each formatted error entry. If you also want to add exception trace - pass flags instead:
@@ -116,7 +116,7 @@ This will make each error entry to look like this:
 ];
 ```
 
-If you prefer first resolver exception to be re-thrown, use following flags:
+If you prefer the first resolver exception to be re-thrown, use following flags:
 ```php
 <?php
 use GraphQL\GraphQL;
@@ -127,6 +127,9 @@ $debug = Debug::INCLUDE_DEBUG_MESSAGE | Debug::RETHROW_INTERNAL_EXCEPTIONS;
 $result = GraphQL::executeQuery(/*args*/)->toArray($debug); 
 ```
 
+If you only want to re-throw Exceptions that are not marked as safe through the `ClientAware` interface, use
+the flag `Debug::RETHROW_UNSAFE_EXCEPTIONS`.
+
 # Custom Error Handling and Formatting
 It is possible to define custom **formatter** and **handler** for result errors.
 

src/Error/Debug.php

@@ -12,4 +12,5 @@ class Debug
     const INCLUDE_DEBUG_MESSAGE       = 1;
     const INCLUDE_TRACE               = 2;
     const RETHROW_INTERNAL_EXCEPTIONS = 4;
+    const RETHROW_UNSAFE_EXCEPTIONS   = 8;
 }

src/Error/FormattedError.php

@@ -245,9 +245,15 @@ class FormattedError
             }
         }
 
-        $isInternal = ! $e instanceof ClientAware || ! $e->isClientSafe();
+        $isUnsafe = ! $e instanceof ClientAware || ! $e->isClientSafe();
 
-        if (($debug & Debug::INCLUDE_DEBUG_MESSAGE) && $isInternal) {
+        if (($debug & Debug::RETHROW_UNSAFE_EXCEPTIONS) && $isUnsafe) {
+            if ($e->getPrevious()) {
+                throw $e->getPrevious();
+            }
+        }
+
+        if (($debug & Debug::INCLUDE_DEBUG_MESSAGE) && $isUnsafe) {
             // Displaying debugMessage as a first entry:
             $formattedError = ['debugMessage' => $e->getMessage()] + $formattedError;
         }

tests/Server/QueryExecutionTest.php

@@ -81,21 +81,21 @@ class QueryExecutionTest extends ServerTestCase
 
         $query = '
         {
-            fieldWithException
+            fieldWithSafeException
             f1
         }
         ';
 
         $expected = [
-            'data'   => [
+            'data' => [
-                'fieldWithException' => null,
+                'fieldWithSafeException' => null,
                 'f1'                 => 'f1',
             ],
             'errors' => [
                 [
                     'message' => 'This is the exception we want',
-                    'path'    => ['fieldWithException'],
+                    'path' => ['fieldWithSafeException'],
-                    'trace'   => [],
+                    'trace' => [],
                 ],
             ],
         ];
@@ -104,6 +104,18 @@ class QueryExecutionTest extends ServerTestCase
         $this->assertArraySubset($expected, $result);
     }
 
+    public function testRethrowUnsafeExceptions() : void
+    {
+        $this->config->setDebug(Debug::RETHROW_UNSAFE_EXCEPTIONS);
+        $this->expectException(Unsafe::class);
+
+        $this->executeQuery('
+        {
+            fieldWithUnsafeException
+        }
+        ')->toArray();
+    }
+
     public function testPassesRootValueAndContext() : void
     {
         $rootValue = 'myRootValue';
@@ -268,7 +280,7 @@ class QueryExecutionTest extends ServerTestCase
     {
         $batch = [
             ['query' => '{invalid}'],
-            ['query' => '{f1,fieldWithException}'],
+            ['query' => '{f1,fieldWithSafeException}'],
         ];
 
         $result = $this->executeBatchedQuery($batch);
@@ -453,7 +465,7 @@ class QueryExecutionTest extends ServerTestCase
 
         $batch = [
             ['query' => '{invalid}'],
-            ['query' => '{f1,fieldWithException}'],
+            ['query' => '{f1,fieldWithSafeException}'],
             [
                 'query'     => '
                     query ($a: String!, $b: String!) {
@@ -472,9 +484,9 @@ class QueryExecutionTest extends ServerTestCase
                 'errors' => [['message' => 'Cannot query field "invalid" on type "Query".']],
             ],
             [
-                'data'   => [
+                'data' => [
-                    'f1'                 => 'f1',
+                    'f1' => 'f1',
-                    'fieldWithException' => null,
+                    'fieldWithSafeException' => null,
                 ],
                 'errors' => [
                     ['message' => 'This is the exception we want'],
@@ -618,7 +630,7 @@ class QueryExecutionTest extends ServerTestCase
             return ['test' => 'formatted'];
         });
 
-        $result = $this->executeQuery('{fieldWithException}');
+        $result = $this->executeQuery('{fieldWithSafeException}');
         $this->assertFalse($called);
         $formatted = $result->toArray();
         $expected  = [
@@ -658,7 +670,7 @@ class QueryExecutionTest extends ServerTestCase
             ];
         });
 
-        $result = $this->executeQuery('{fieldWithException,test: fieldWithException}');
+        $result = $this->executeQuery('{fieldWithSafeException,test: fieldWithSafeException}');
 
         $this->assertFalse($called);
         $formatted = $result->toArray();

tests/Server/ServerTestCase.php

@@ -41,12 +41,18 @@ abstract class ServerTestCase extends TestCase
                             return $info->fieldName;
                         },
                     ],
-                    'fieldWithException'      => [
+                    'fieldWithSafeException' => [
-                        'type'    => Type::string(),
+                        'type' => Type::string(),
-                        'resolve' => function ($root, $args, $context, $info) {
+                        'resolve' => function () {
                             throw new UserError('This is the exception we want');
                         },
                     ],
+                    'fieldWithUnsafeException' => [
+                        'type' => Type::string(),
+                        'resolve' => function () {
+                            throw new Unsafe('This exception should not be shown to the user');
+                        },
+                    ],
                     'testContextAndRootValue' => [
                         'type'    => Type::string(),
                         'resolve' => function ($root, $args, $context, $info) {

tests/Server/Unsafe.php

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