From 4308b8c3cd1a367a7ffdda56f3fddfba222877a9 Mon Sep 17 00:00:00 2001 From: Adrian Philipp Date: Tue, 20 Sep 2016 20:39:36 +0200 Subject: [PATCH 1/2] Exceptions can be mapped to UserError and UserWarning --- DependencyInjection/Configuration.php | 12 ++++ .../OverblogGraphQLExtension.php | 7 +++ Error/ErrorHandler.php | 36 +++++++++++- README.md | 19 +++++++ Resources/config/services.yml | 1 + .../OverblogGraphQLTypesExtensionTest.php | 33 +++++++++++ Tests/Error/ErrorHandlerTest.php | 27 +++++++++ Tests/Functional/Exception/ExceptionTest.php | 56 +++++++++++++++++++ .../app/Exception/ExampleException.php | 20 +++++++ .../app/config/exception/config.yml | 20 +++++++ .../exception/mapping/exception.types.yml | 8 +++ .../app/config/exception/services.yml | 5 ++ 12 files changed, 242 insertions(+), 2 deletions(-) create mode 100644 Tests/Functional/Exception/ExceptionTest.php create mode 100644 Tests/Functional/app/Exception/ExampleException.php create mode 100644 Tests/Functional/app/config/exception/config.yml create mode 100644 Tests/Functional/app/config/exception/mapping/exception.types.yml create mode 100644 Tests/Functional/app/config/exception/services.yml diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 29cfe4b06..94edfcc8d 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -66,6 +66,18 @@ public function getConfigTreeBuilder() ->end() ->end() ->end() + ->arrayNode('exceptions') + ->children() + ->arrayNode('warnings') + ->treatNullLike(array()) + ->prototype('scalar')->end() + ->end() + ->arrayNode('errors') + ->treatNullLike(array()) + ->prototype('scalar')->end() + ->end() + ->end() + ->end() ->arrayNode('builders') ->children() ->arrayNode('field') diff --git a/DependencyInjection/OverblogGraphQLExtension.php b/DependencyInjection/OverblogGraphQLExtension.php index e624f8b98..6f37e056e 100644 --- a/DependencyInjection/OverblogGraphQLExtension.php +++ b/DependencyInjection/OverblogGraphQLExtension.php @@ -102,6 +102,13 @@ private function setErrorHandlerArguments(array $config, ContainerBuilder $conta ->replaceArgument(0, $config['definitions']['internal_error_message']) ; } + + if (isset($config['definitions']['exceptions'])) { + $container + ->getDefinition($this->getAlias().'.error_handler') + ->replaceArgument(2, $config['definitions']['exceptions']) + ; + } } private function setSchemaBuilderArguments(array $config, ContainerBuilder $container) diff --git a/Error/ErrorHandler.php b/Error/ErrorHandler.php index 8072b59ba..331becdbc 100644 --- a/Error/ErrorHandler.php +++ b/Error/ErrorHandler.php @@ -26,13 +26,17 @@ class ErrorHandler /** @var string */ private $internalErrorMessage; - public function __construct($internalErrorMessage = null, LoggerInterface $logger = null) + /** @var array */ + private $exceptionMap; + + public function __construct($internalErrorMessage = null, LoggerInterface $logger = null, array $exceptionMap = []) { $this->logger = (null === $logger) ? new NullLogger() : $logger; if (empty($internalErrorMessage)) { $internalErrorMessage = self::DEFAULT_ERROR_MESSAGE; } $this->internalErrorMessage = $internalErrorMessage; + $this->exceptionMap = $exceptionMap; } /** @@ -54,7 +58,7 @@ protected function treatExceptions(array $errors, $throwRawException) /** @var Error $error */ foreach ($errors as $error) { - $rawException = $error->getPrevious(); + $rawException = $this->convertException($error->getPrevious()); // Parse error or user error if (null === $rawException) { @@ -129,4 +133,32 @@ public function handleErrors(ExecutionResult $executionResult, $throwRawExceptio $executionResult->extensions['warnings'] = array_map(['GraphQL\Error', 'formatError'], $exceptions['extensions']['warnings']); } } + + /** + * Tries to convert a raw exception into a user warning or error + * that is displayed to the user. + * + * @param \Exception $rawException + * @return \Exception + */ + protected function convertException(\Exception $rawException = null) + { + if (empty($rawException)) { + return $rawException; + } + + $types = [ + 'warnings' => 'Overblog\\GraphQLBundle\\Error\\UserWarning', + 'errors' => 'Overblog\\GraphQLBundle\\Error\\UserError', + ]; + + foreach ($types as $type => $errorClass) { + if (!empty($this->exceptionMap[$type]) + && in_array(get_class($rawException), $this->exceptionMap[$type])) { + return new $errorClass($rawException->getMessage(), $rawException->getCode(), $rawException); + } + } + + return $rawException; + } } diff --git a/README.md b/README.md index 523223ad0..f7cb609a7 100644 --- a/README.md +++ b/README.md @@ -665,6 +665,25 @@ class CharacterResolver } ``` +If you want to map your own exceptions to warnings and errors you can +define a custom exception mapping: + +```yaml +#app/config/config.yml +overblog_graphql: + #... + definitions: + #... + exceptions: + warnings: + - "Symfony\\Component\\Routing\\Exception\\ResourceNotFoundException" + errors: + - "InvalidArgumentException" +``` + +The message of those exceptions are then shown to the user like other +`UserError`s or `UserWarning`s. + Security -------- diff --git a/Resources/config/services.yml b/Resources/config/services.yml index d723c3453..4c6152c15 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -9,6 +9,7 @@ services: arguments: - ~ - "@?logger" + - [] overblog_graphql.request_executor: class: Overblog\GraphQLBundle\Request\Executor diff --git a/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php b/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php index 60279b046..a1ab34cc9 100644 --- a/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php +++ b/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php @@ -82,6 +82,39 @@ public function testInternalConfigKeysShouldNotBeUsed($internalConfigKey) $this->extension->load($configs, $this->container); } + /** + * @runInSeparateProcess + */ + public function testCustomExceptions() + { + $customExceptions = [ + 'warnings' => [ + 'Symfony\Component\Routing\Exception\ResourceNotFoundException' + ], + 'errors' => [ + 'InvalidArgumentException' + ], + ]; + + $ext = new OverblogGraphQLExtension(); + $ext->load( + [ + [ + 'definitions' => [ + 'exceptions' => $customExceptions + ], + ], + ], + $this->container + ); + + $definition = $this->container->getDefinition('overblog_graphql.error_handler'); + $this->assertEquals($customExceptions, $definition->getArgument(2)); + } + + /** + * @runInSeparateProcess + */ public function testCustomBuilders() { $ext = new OverblogGraphQLExtension(); diff --git a/Tests/Error/ErrorHandlerTest.php b/Tests/Error/ErrorHandlerTest.php index 7dfce193a..9bf3baeee 100644 --- a/Tests/Error/ErrorHandlerTest.php +++ b/Tests/Error/ErrorHandlerTest.php @@ -138,4 +138,31 @@ public function testMaskErrorWithoutWrappedExceptionAndThrowExceptionSetToTrue() $this->assertEquals($expected, $executionResult->toArray()); } + + public function testConvertExceptionToUserWarning() + { + $errorHandler = new ErrorHandler(null, null, ['warnings' => ["InvalidArgumentException"]]); + + $executionResult = new ExecutionResult( + null, + [ + new Error('Error with invalid argument exception', null, new \InvalidArgumentException('Invalid argument exception')), + ] + ); + + $errorHandler->handleErrors($executionResult, true); + + $expected = [ + 'data' => null, + 'extensions' => [ + 'warnings' => [ + [ + 'message' => 'Error with invalid argument exception', + ], + ], + ], + ]; + + $this->assertEquals($expected, $executionResult->toArray()); + } } diff --git a/Tests/Functional/Exception/ExceptionTest.php b/Tests/Functional/Exception/ExceptionTest.php new file mode 100644 index 000000000..cec13f0b0 --- /dev/null +++ b/Tests/Functional/Exception/ExceptionTest.php @@ -0,0 +1,56 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Tests\Functional\Exception; + +use Overblog\GraphQLBundle\Tests\Functional\TestCase; + +/** + * Class ConnectionTest. + * + * @see https://github.com/graphql/graphql-relay-js/blob/master/src/connection/__tests__/connection.js + */ +class ExceptionTest extends TestCase +{ + protected function setUp() + { + parent::setUp(); + + static::createAndBootKernel(['test_case' => 'exception']); + } + + public function testExceptionIsMappedToAWarning() + { + $query = << null, + ]; + + $expectedErrors = [ + [ + 'message' => 'Invalid argument exception', + 'locations' => [ + [ + 'line' => 2, + 'column' => 5, + ] + ], + ], + ]; + + $this->assertGraphQL($query, $expectedData, $expectedErrors); + } +} diff --git a/Tests/Functional/app/Exception/ExampleException.php b/Tests/Functional/app/Exception/ExampleException.php new file mode 100644 index 000000000..8a3389d58 --- /dev/null +++ b/Tests/Functional/app/Exception/ExampleException.php @@ -0,0 +1,20 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Tests\Functional\app\Exception; + +class ExampleException +{ + public function throwException() + { + throw new \InvalidArgumentException('Invalid argument exception'); + } +} diff --git a/Tests/Functional/app/config/exception/config.yml b/Tests/Functional/app/config/exception/config.yml new file mode 100644 index 000000000..92d4335bf --- /dev/null +++ b/Tests/Functional/app/config/exception/config.yml @@ -0,0 +1,20 @@ +imports: + - { resource: ../config.yml } + - { resource: services.yml } + +parameters: + overblog_graphql.type_class_namespace: "Overblog\\GraphQLBundle\\Exception\\__DEFINITIONS__" + +overblog_graphql: + definitions: + exceptions: + errors: + - "InvalidArgumentException" + schema: + query: Query + mutation: ~ + mappings: + types: + - + type: yml + dir: "%kernel.root_dir%/config/exception/mapping" diff --git a/Tests/Functional/app/config/exception/mapping/exception.types.yml b/Tests/Functional/app/config/exception/mapping/exception.types.yml new file mode 100644 index 000000000..16063961f --- /dev/null +++ b/Tests/Functional/app/config/exception/mapping/exception.types.yml @@ -0,0 +1,8 @@ +Query: + type: object + config: + fields: + test: + type: "Boolean" + resolve: "@=resolver('example_exception')" + diff --git a/Tests/Functional/app/config/exception/services.yml b/Tests/Functional/app/config/exception/services.yml new file mode 100644 index 000000000..13b79aa37 --- /dev/null +++ b/Tests/Functional/app/config/exception/services.yml @@ -0,0 +1,5 @@ +services: + overblog_graphql.test.exception: + class: Overblog\GraphQLBundle\Tests\Functional\app\Exception\ExampleException + tags: + - { name: "overblog_graphql.resolver", alias: "example_exception", method: "throwException" } From 0c9dab1786e495b97ffb2a15c3e0757eb958c8c5 Mon Sep 17 00:00:00 2001 From: Adrian Philipp Date: Wed, 21 Sep 2016 12:46:10 +0200 Subject: [PATCH 2/2] Improve custom exceptions performance Using a better data structure to map exceptions direclty to error classes speeds the exception matching. Building the mapping is done on the container compile time to use cache power. --- DependencyInjection/Configuration.php | 12 +++++++++ .../OverblogGraphQLExtension.php | 26 ++++++++++++++++++- Error/ErrorHandler.php | 16 ++++-------- .../OverblogGraphQLTypesExtensionTest.php | 25 ++++++++++-------- Tests/Error/ErrorHandlerTest.php | 2 +- 5 files changed, 57 insertions(+), 24 deletions(-) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 94edfcc8d..3f61b0813 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -76,8 +76,20 @@ public function getConfigTreeBuilder() ->treatNullLike(array()) ->prototype('scalar')->end() ->end() + ->arrayNode('types') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('warnings') + ->defaultValue('Overblog\\GraphQLBundle\\Error\\UserWarning') + ->end() + ->scalarNode('errors') + ->defaultValue('Overblog\\GraphQLBundle\\Error\\UserError') + ->end() + ->end() + ->end() ->end() ->end() + ->arrayNode('builders') ->children() ->arrayNode('field') diff --git a/DependencyInjection/OverblogGraphQLExtension.php b/DependencyInjection/OverblogGraphQLExtension.php index 6f37e056e..90f04f637 100644 --- a/DependencyInjection/OverblogGraphQLExtension.php +++ b/DependencyInjection/OverblogGraphQLExtension.php @@ -106,7 +106,7 @@ private function setErrorHandlerArguments(array $config, ContainerBuilder $conta if (isset($config['definitions']['exceptions'])) { $container ->getDefinition($this->getAlias().'.error_handler') - ->replaceArgument(2, $config['definitions']['exceptions']) + ->replaceArgument(2, $this->buildExceptionMap($config['definitions']['exceptions'])) ; } } @@ -149,4 +149,28 @@ public function getConfiguration(array $config, ContainerBuilder $container) { return new Configuration($container->getParameter('kernel.debug')); } + + /** + * Returns a list of custom exceptions mapped to error/warning classes. + * + * @param array $config + * @return array Custom exception map, [exception => UserError/UserWarning]. + */ + private function buildExceptionMap(array $exceptionConfig) + { + $exceptionMap = []; + $typeMap = $exceptionConfig['types']; + + foreach ($exceptionConfig as $type => $exceptionList) { + if ('types' === $type) { + continue; + } + + foreach ($exceptionList as $exception) { + $exceptionMap[$exception] = $typeMap[$type]; + } + } + + return $exceptionMap; + } } diff --git a/Error/ErrorHandler.php b/Error/ErrorHandler.php index 331becdbc..a14e0076b 100644 --- a/Error/ErrorHandler.php +++ b/Error/ErrorHandler.php @@ -143,20 +143,14 @@ public function handleErrors(ExecutionResult $executionResult, $throwRawExceptio */ protected function convertException(\Exception $rawException = null) { - if (empty($rawException)) { - return $rawException; + if (null === $rawException) { + return null; } - $types = [ - 'warnings' => 'Overblog\\GraphQLBundle\\Error\\UserWarning', - 'errors' => 'Overblog\\GraphQLBundle\\Error\\UserError', - ]; + if (!empty($this->exceptionMap[get_class($rawException)])) { + $errorClass = $this->exceptionMap[get_class($rawException)]; - foreach ($types as $type => $errorClass) { - if (!empty($this->exceptionMap[$type]) - && in_array(get_class($rawException), $this->exceptionMap[$type])) { - return new $errorClass($rawException->getMessage(), $rawException->getCode(), $rawException); - } + return new $errorClass($rawException->getMessage(), $rawException->getCode(), $rawException); } return $rawException; diff --git a/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php b/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php index a1ab34cc9..2027802c8 100644 --- a/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php +++ b/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php @@ -87,29 +87,32 @@ public function testInternalConfigKeysShouldNotBeUsed($internalConfigKey) */ public function testCustomExceptions() { - $customExceptions = [ - 'warnings' => [ - 'Symfony\Component\Routing\Exception\ResourceNotFoundException' - ], - 'errors' => [ - 'InvalidArgumentException' - ], - ]; - $ext = new OverblogGraphQLExtension(); $ext->load( [ [ 'definitions' => [ - 'exceptions' => $customExceptions + 'exceptions' => [ + 'warnings' => [ + 'Symfony\Component\Routing\Exception\ResourceNotFoundException' + ], + 'errors' => [ + 'InvalidArgumentException' + ], + ] ], ], ], $this->container ); + $expectedExceptionMap = [ + 'Symfony\Component\Routing\Exception\ResourceNotFoundException' => 'Overblog\\GraphQLBundle\\Error\\UserWarning', + 'InvalidArgumentException' => 'Overblog\\GraphQLBundle\\Error\\UserError', + ]; + $definition = $this->container->getDefinition('overblog_graphql.error_handler'); - $this->assertEquals($customExceptions, $definition->getArgument(2)); + $this->assertEquals($expectedExceptionMap, $definition->getArgument(2)); } /** diff --git a/Tests/Error/ErrorHandlerTest.php b/Tests/Error/ErrorHandlerTest.php index 9bf3baeee..de1392670 100644 --- a/Tests/Error/ErrorHandlerTest.php +++ b/Tests/Error/ErrorHandlerTest.php @@ -141,7 +141,7 @@ public function testMaskErrorWithoutWrappedExceptionAndThrowExceptionSetToTrue() public function testConvertExceptionToUserWarning() { - $errorHandler = new ErrorHandler(null, null, ['warnings' => ["InvalidArgumentException"]]); + $errorHandler = new ErrorHandler(null, null, ["InvalidArgumentException" => 'Overblog\\GraphQLBundle\\Error\\UserWarning']); $executionResult = new ExecutionResult( null,