From 326e0b47192f848363274b007b426a2f393e3b27 Mon Sep 17 00:00:00 2001 From: spawnia Date: Mon, 3 Sep 2018 21:18:04 +0200 Subject: [PATCH] Add Debug::RETHROW_UNSAFE_EXCEPTIONS flag This allows for a more granular default for which Exceptions are thrown, taking the settings made through the ClientAware interface into account. - Add docs - Add a test case - Differentiate clearly from other test --- docs/error-handling.md | 7 +++-- src/Error/Debug.php | 1 + src/Error/FormattedError.php | 14 +++++++--- tests/Server/QueryExecutionTest.php | 30 ++++++++++++++-------- tests/Server/ServerTestCase.php | 40 ++++++++++++++++++++++++++--- 5 files changed, 73 insertions(+), 19 deletions(-) diff --git a/docs/error-handling.md b/docs/error-handling.md index 8582949..17b0a2a 100644 --- a/docs/error-handling.md +++ b/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 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. diff --git a/src/Error/Debug.php b/src/Error/Debug.php index 6e98087..a48b81f 100644 --- a/src/Error/Debug.php +++ b/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; } diff --git a/src/Error/FormattedError.php b/src/Error/FormattedError.php index 705d822..fcf0914 100644 --- a/src/Error/FormattedError.php +++ b/src/Error/FormattedError.php @@ -235,9 +235,7 @@ class FormattedError $debug = (int) $debug; - $isInternal = ! $e instanceof ClientAware || ! $e->isClientSafe(); - - if (($debug & Debug::RETHROW_INTERNAL_EXCEPTIONS) && $isInternal) { + if ($debug & Debug::RETHROW_INTERNAL_EXCEPTIONS) { if (! $e instanceof Error) { throw $e; } @@ -247,7 +245,15 @@ class FormattedError } } - if (($debug & Debug::INCLUDE_DEBUG_MESSAGE) && $isInternal) { + $isUnsafe = ! $e instanceof ClientAware || ! $e->isClientSafe(); + + 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; } diff --git a/tests/Server/QueryExecutionTest.php b/tests/Server/QueryExecutionTest.php index 0b7d473..f443d9e 100644 --- a/tests/Server/QueryExecutionTest.php +++ b/tests/Server/QueryExecutionTest.php @@ -1,11 +1,9 @@ [ - 'fieldWithException' => null, + 'fieldWithSafeException' => null, 'f1' => 'f1' ], 'errors' => [ [ 'message' => 'This is the exception we want', - 'path' => ['fieldWithException'], + 'path' => ['fieldWithSafeException'], 'trace' => [] ] ] @@ -86,6 +84,18 @@ class QueryExecutionTest extends ServerTestCase $result = $this->executeQuery($query)->toArray(); $this->assertArraySubset($expected, $result); } + + public function testRethrowUnsafeExceptions() : void + { + $this->config->setDebug(Debug::RETHROW_UNSAFE_EXCEPTIONS); + $this->expectException(UnsafeException::class); + + $this->executeQuery(' + { + fieldWithUnsafeException + } + ')->toArray(); + } public function testPassesRootValueAndContext() : void { @@ -240,7 +250,7 @@ class QueryExecutionTest extends ServerTestCase 'query' => '{invalid}' ], [ - 'query' => '{f1,fieldWithException}' + 'query' => '{f1,fieldWithSafeException}' ] ]; @@ -405,7 +415,7 @@ class QueryExecutionTest extends ServerTestCase 'query' => '{invalid}' ], [ - 'query' => '{f1,fieldWithException}' + 'query' => '{f1,fieldWithSafeException}' ], [ 'query' => ' @@ -427,7 +437,7 @@ class QueryExecutionTest extends ServerTestCase [ 'data' => [ 'f1' => 'f1', - 'fieldWithException' => null + 'fieldWithSafeException' => null ], 'errors' => [ ['message' => 'This is the exception we want'] @@ -581,7 +591,7 @@ class QueryExecutionTest extends ServerTestCase return ['test' => 'formatted']; }); - $result = $this->executeQuery('{fieldWithException}'); + $result = $this->executeQuery('{fieldWithSafeException}'); $this->assertFalse($called); $formatted = $result->toArray(); $expected = [ @@ -620,7 +630,7 @@ class QueryExecutionTest extends ServerTestCase ]; }); - $result = $this->executeQuery('{fieldWithException,test: fieldWithException}'); + $result = $this->executeQuery('{fieldWithSafeException,test: fieldWithSafeException}'); $this->assertFalse($called); $formatted = $result->toArray(); diff --git a/tests/Server/ServerTestCase.php b/tests/Server/ServerTestCase.php index e76d8f4..5620337 100644 --- a/tests/Server/ServerTestCase.php +++ b/tests/Server/ServerTestCase.php @@ -3,6 +3,7 @@ namespace GraphQL\Tests\Server; use GraphQL\Deferred; +use GraphQL\Error\ClientAware; use GraphQL\Error\UserError; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; @@ -34,10 +35,16 @@ abstract class ServerTestCase extends TestCase return $info->fieldName; } ], - 'fieldWithException' => [ + 'fieldWithSafeException' => [ 'type' => Type::string(), - 'resolve' => function($root, $args, $context, $info) { - throw new UserError("This is the exception we want"); + 'resolve' => function() { + throw new UserError('This is the exception we want'); + } + ], + 'fieldWithUnsafeException' => [ + 'type' => Type::string(), + 'resolve' => function() { + throw new UnsafeException('This exception should not be shown to the user'); } ], 'testContextAndRootValue' => [ @@ -92,3 +99,30 @@ abstract class ServerTestCase extends TestCase return $schema; } } + +class UnsafeException 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'; + } +}