diff --git a/Command/GraphQLDumpSchemaCommand.php b/Command/GraphQLDumpSchemaCommand.php index 9bb40d7fe..a40c37f49 100644 --- a/Command/GraphQLDumpSchemaCommand.php +++ b/Command/GraphQLDumpSchemaCommand.php @@ -94,7 +94,7 @@ private function createFile(InputInterface $input) $modern = $this->useModernJsonFormat($input); $result = $this->getRequestExecutor() - ->execute($request, [], $schemaName) + ->execute($schemaName, $request) ->toArray(); $content = json_encode($modern ? $result : $result['data'], \JSON_PRETTY_PRINT); diff --git a/Controller/GraphController.php b/Controller/GraphController.php index 5022e2bdd..6dacdd5fa 100644 --- a/Controller/GraphController.php +++ b/Controller/GraphController.php @@ -95,12 +95,7 @@ private function processBatchQuery(Request $request, $schemaName = null) foreach ($queries as $query) { $payloadResult = $this->requestExecutor->execute( - [ - 'query' => $query['query'], - 'variables' => $query['variables'], - ], - [], - $schemaName + $schemaName, ['query' => $query['query'], 'variables' => $query['variables']] ); $payloads[] = $apolloBatching ? $payloadResult->toArray() : ['id' => $query['id'], 'payload' => $payloadResult->toArray()]; } @@ -112,6 +107,6 @@ private function processNormalQuery(Request $request, $schemaName = null) { $params = $this->requestParser->parse($request); - return $this->requestExecutor->execute($params, [], $schemaName)->toArray(); + return $this->requestExecutor->execute($schemaName, $params)->toArray(); } } diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 4cb70f409..25bee13e7 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -5,12 +5,18 @@ use GraphQL\Validator\Rules\QueryComplexity; use GraphQL\Validator\Rules\QueryDepth; use Overblog\GraphQLBundle\Error\ErrorHandler; +use Overblog\GraphQLBundle\EventListener\ErrorLoggerListener; use Overblog\GraphQLBundle\Resolver\Resolver; +use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; +use Symfony\Component\Config\Definition\Builder\EnumNodeDefinition; +use Symfony\Component\Config\Definition\Builder\ScalarNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; class Configuration implements ConfigurationInterface { + const NAME = 'overblog_graphql'; + /** bool */ private $debug; @@ -32,162 +38,246 @@ public function __construct($debug, $cacheDir = null) public function getConfigTreeBuilder() { $treeBuilder = new TreeBuilder(); - $rootNode = $treeBuilder->root('overblog_graphql'); + $rootNode = $treeBuilder->root(self::NAME); $rootNode ->children() - ->enumNode('batching_method') - ->values(['relay', 'apollo']) - ->defaultValue('relay') - ->end() - ->arrayNode('definitions') + ->append($this->batchingMethodSection()) + ->append($this->definitionsSection()) + ->append($this->errorsHandlerSection()) + ->append($this->servicesSection()) + ->append($this->securitySection()) + ->end(); + + return $treeBuilder; + } + + private function batchingMethodSection() + { + $builder = new TreeBuilder(); + /** @var EnumNodeDefinition $node */ + $node = $builder->root('batching_method', 'enum'); + + $node + ->values(['relay', 'apollo']) + ->defaultValue('relay') + ->end(); + + return $node; + } + + private function errorsHandlerSection() + { + $builder = new TreeBuilder(); + /** @var ArrayNodeDefinition $node */ + $node = $builder->root('errors_handler'); + $node + ->treatFalseLike(['enabled' => false]) + ->treatTrueLike(['enabled' => true]) + ->treatNullLike(['enabled' => true]) + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('enabled')->defaultTrue()->end() + ->scalarNode('internal_error_message')->defaultValue(ErrorHandler::DEFAULT_ERROR_MESSAGE)->end() + ->booleanNode('rethrow_internal_exceptions')->defaultFalse()->end() + ->booleanNode('debug')->defaultValue($this->debug)->end() + ->booleanNode('log')->defaultTrue()->end() + ->scalarNode('logger_service')->defaultValue(ErrorLoggerListener::DEFAULT_LOGGER_SERVICE)->end() + ->booleanNode('map_exceptions_to_parent')->defaultFalse()->end() + ->arrayNode('exceptions') ->addDefaultsIfNotSet() ->children() - ->scalarNode('internal_error_message')->defaultNull()->end() - ->variableNode('default_resolver')->defaultValue([Resolver::class, 'defaultResolveFn'])->end() - ->scalarNode('class_namespace')->defaultValue('Overblog\\GraphQLBundle\\__DEFINITIONS__')->end() - ->scalarNode('cache_dir')->defaultValue($this->cacheDir.'/overblog/graphql-bundle/__definitions__')->end() - ->booleanNode('use_classloader_listener')->defaultTrue()->end() - ->booleanNode('auto_compile')->defaultTrue()->end() - ->booleanNode('show_debug_info')->defaultFalse()->end() - ->booleanNode('config_validation')->defaultValue($this->debug)->end() - ->arrayNode('schema') - ->beforeNormalization() - ->ifTrue(function ($v) { - return isset($v['query']) && is_string($v['query']) || isset($v['mutation']) && is_string($v['mutation']); - }) - ->then(function ($v) { - return ['default' => $v]; - }) - ->end() - ->useAttributeAsKey('name') - ->prototype('array') - ->addDefaultsIfNotSet() - ->children() - ->scalarNode('query')->defaultNull()->end() - ->scalarNode('mutation')->defaultNull()->end() - ->scalarNode('subscription')->defaultNull()->end() - ->end() - ->end() - ->end() - ->arrayNode('auto_mapping') - ->treatFalseLike(['enabled' => false]) - ->treatTrueLike(['enabled' => true]) - ->treatNullLike(['enabled' => true]) - ->addDefaultsIfNotSet() - ->children() - ->booleanNode('enabled')->defaultTrue()->end() - ->arrayNode('directories') - ->info('List of directories containing GraphQL classes.') - ->prototype('scalar')->end() - ->end() - ->end() + ->arrayNode('warnings') + ->treatNullLike([]) + ->prototype('scalar')->end() ->end() - ->arrayNode('mappings') - ->children() - ->arrayNode('auto_discover') - ->treatFalseLike(['bundles' => false, 'root_dir' => false]) - ->treatTrueLike(['bundles' => true, 'root_dir' => true]) - ->treatNullLike(['bundles' => true, 'root_dir' => true]) - ->addDefaultsIfNotSet() - ->children() - ->booleanNode('bundles')->defaultTrue()->end() - ->booleanNode('root_dir')->defaultTrue()->end() - ->end() - ->end() - ->arrayNode('types') - ->prototype('array') - ->addDefaultsIfNotSet() - ->beforeNormalization() - ->ifTrue(function ($v) { - return isset($v['type']) && 'yml' === $v['type']; - }) - ->then(function ($v) { - $v['type'] = 'yaml'; - - return $v; - }) - ->end() - ->children() - ->enumNode('type')->values(['yaml', 'xml'])->defaultNull()->end() - ->scalarNode('dir')->defaultNull()->end() - ->scalarNode('suffix')->defaultValue(OverblogGraphQLTypesExtension::DEFAULT_TYPES_SUFFIX)->end() - ->end() - ->end() - ->end() - ->end() - ->end() - ->booleanNode('map_exceptions_to_parent')->defaultFalse()->end() - ->arrayNode('exceptions') - ->addDefaultsIfNotSet() - ->children() - ->arrayNode('warnings') - ->treatNullLike([]) - ->prototype('scalar')->end() - ->end() - ->arrayNode('errors') - ->treatNullLike([]) - ->prototype('scalar')->end() - ->end() - ->arrayNode('types') - ->addDefaultsIfNotSet() - ->children() - ->scalarNode('warnings') - ->defaultValue(ErrorHandler::DEFAULT_USER_WARNING_CLASS) - ->end() - ->scalarNode('errors') - ->defaultValue(ErrorHandler::DEFAULT_USER_ERROR_CLASS) - ->end() - ->end() - ->end() - ->end() + ->arrayNode('errors') + ->treatNullLike([]) + ->prototype('scalar')->end() ->end() + ->end() + ->end() + ->end(); - ->arrayNode('builders') - ->children() - ->append($this->addBuilderSection('field')) - ->append($this->addBuilderSection('args')) - ->end() - ->end() + return $node; + } + private function definitionsSection() + { + $builder = new TreeBuilder(); + /** @var ArrayNodeDefinition $node */ + $node = $builder->root('definitions'); + $node + ->addDefaultsIfNotSet() + ->children() + ->variableNode('default_resolver')->defaultValue([Resolver::class, 'defaultResolveFn'])->end() + ->scalarNode('class_namespace')->defaultValue('Overblog\\GraphQLBundle\\__DEFINITIONS__')->end() + ->scalarNode('cache_dir')->defaultValue($this->cacheDir.'/overblog/graphql-bundle/__definitions__')->end() + ->booleanNode('use_classloader_listener')->defaultTrue()->end() + ->booleanNode('auto_compile')->defaultTrue()->end() + ->booleanNode('show_debug_info')->info('Show some performance stats in extensions')->defaultFalse()->end() + ->booleanNode('config_validation')->defaultValue($this->debug)->end() + ->append($this->definitionsSchemaSection()) + ->append($this->definitionsAutoMappingSection()) + ->append($this->definitionsMappingsSection()) + ->arrayNode('builders') + ->children() + ->append($this->builderSection('field')) + ->append($this->builderSection('args')) ->end() ->end() - ->arrayNode('services') + + ->end() + ->end(); + + return $node; + } + + private function servicesSection() + { + $builder = new TreeBuilder(); + /** @var ArrayNodeDefinition $node */ + $node = $builder->root('services'); + $node + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('executor') + ->defaultValue(self::NAME.'.executor.default') + ->end() + ->scalarNode('promise_adapter') + ->defaultValue(self::NAME.'.promise_adapter.default') + ->end() + ->scalarNode('expression_language') + ->defaultValue(self::NAME.'.expression_language.default') + ->end() + ->scalarNode('cache_expression_language_parser') + ->defaultValue(self::NAME.'.cache_expression_language_parser.default') + ->end() + ->end() + ->end(); + + return $node; + } + + private function securitySection() + { + $builder = new TreeBuilder(); + /** @var ArrayNodeDefinition $node */ + $node = $builder->root('security'); + $node + ->addDefaultsIfNotSet() + ->children() + ->append($this->securityQuerySection('query_max_depth', QueryDepth::DISABLED)) + ->append($this->securityQuerySection('query_max_complexity', QueryComplexity::DISABLED)) + ->booleanNode('handle_cors')->defaultFalse()->end() + ->end() + ->end(); + + return $node; + } + + private function definitionsSchemaSection() + { + $builder = new TreeBuilder(); + /** @var ArrayNodeDefinition $node */ + $node = $builder->root('schema'); + $node + ->beforeNormalization() + ->ifTrue(function ($v) { + return isset($v['query']) && is_string($v['query']) || isset($v['mutation']) && is_string($v['mutation']); + }) + ->then(function ($v) { + return ['default' => $v]; + }) + ->end() + ->useAttributeAsKey('name') + ->prototype('array') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('query')->defaultNull()->end() + ->scalarNode('mutation')->defaultNull()->end() + ->scalarNode('subscription')->defaultNull()->end() + ->end() + ->end() + ->end(); + + return $node; + } + + private function definitionsAutoMappingSection() + { + $builder = new TreeBuilder(); + /** @var ArrayNodeDefinition $node */ + $node = $builder->root('auto_mapping'); + $node + ->treatFalseLike(['enabled' => false]) + ->treatTrueLike(['enabled' => true]) + ->treatNullLike(['enabled' => true]) + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('enabled')->defaultTrue()->end() + ->arrayNode('directories') + ->info('List of directories containing GraphQL classes.') + ->prototype('scalar')->end() + ->end() + ->end() + ->end(); + + return $node; + } + + private function definitionsMappingsSection() + { + $builder = new TreeBuilder(); + /** @var ArrayNodeDefinition $node */ + $node = $builder->root('mappings'); + $node + ->children() + ->arrayNode('auto_discover') + ->treatFalseLike(['bundles' => false, 'root_dir' => false]) + ->treatTrueLike(['bundles' => true, 'root_dir' => true]) + ->treatNullLike(['bundles' => true, 'root_dir' => true]) ->addDefaultsIfNotSet() ->children() - ->scalarNode('executor') - ->defaultValue('overblog_graphql.executor.default') - ->end() - ->scalarNode('promise_adapter') - ->defaultValue('overblog_graphql.promise_adapter.default') - ->end() - ->scalarNode('expression_language') - ->defaultValue('overblog_graphql.expression_language.default') - ->end() - ->scalarNode('cache_expression_language_parser') - ->defaultValue('overblog_graphql.cache_expression_language_parser.default') - ->end() + ->booleanNode('bundles')->defaultTrue()->end() + ->booleanNode('root_dir')->defaultTrue()->end() ->end() ->end() - ->arrayNode('security') - ->addDefaultsIfNotSet() - ->children() - ->append($this->addSecurityQuerySection('query_max_depth', QueryDepth::DISABLED)) - ->append($this->addSecurityQuerySection('query_max_complexity', QueryComplexity::DISABLED)) - ->booleanNode('handle_cors')->defaultFalse()->end() + ->arrayNode('types') + ->prototype('array') + ->addDefaultsIfNotSet() + ->beforeNormalization() + ->ifTrue(function ($v) { + return isset($v['type']) && 'yml' === $v['type']; + }) + ->then(function ($v) { + $v['type'] = 'yaml'; + + return $v; + }) + ->end() + ->children() + ->enumNode('type')->values(['yaml', 'xml'])->defaultNull()->end() + ->scalarNode('dir')->defaultNull()->end() + ->scalarNode('suffix')->defaultValue(OverblogGraphQLTypesExtension::DEFAULT_TYPES_SUFFIX)->end() + ->end() ->end() ->end() - ->end(); + ->end() + ->end(); - return $treeBuilder; + return $node; } /** * @param string $name + * + * @return ArrayNodeDefinition */ - private function addBuilderSection($name) + private function builderSection($name) { $builder = new TreeBuilder(); + /** @var ArrayNodeDefinition $node */ $node = $builder->root($name); $node->beforeNormalization() ->ifTrue(function ($v) { @@ -220,10 +310,14 @@ private function addBuilderSection($name) /** * @param string $name + * @param bool $disabledValue + * + * @return ScalarNodeDefinition */ - private function addSecurityQuerySection($name, $disabledValue) + private function securityQuerySection($name, $disabledValue) { $builder = new TreeBuilder(); + /** @var ScalarNodeDefinition $node */ $node = $builder->root($name, 'scalar'); $node->beforeNormalization() ->ifTrue(function ($v) { @@ -249,7 +343,7 @@ private function addSecurityQuerySection($name, $disabledValue) ->ifTrue(function ($v) { return is_int($v) && $v < 0; }) - ->thenInvalid('"overblog_graphql.security.'.$name.'" must be greater or equal to 0.') + ->thenInvalid(sprintf('"%s.security.%s" must be greater or equal to 0.', self::NAME, $name)) ->end() ; diff --git a/DependencyInjection/OverblogGraphQLExtension.php b/DependencyInjection/OverblogGraphQLExtension.php index 5704573ba..364be8854 100644 --- a/DependencyInjection/OverblogGraphQLExtension.php +++ b/DependencyInjection/OverblogGraphQLExtension.php @@ -2,13 +2,21 @@ namespace Overblog\GraphQLBundle\DependencyInjection; +use GraphQL\Error\UserError; use GraphQL\Type\Schema; use Overblog\GraphQLBundle\CacheWarmer\CompileCacheWarmer; use Overblog\GraphQLBundle\Config\Processor\BuilderProcessor; +use Overblog\GraphQLBundle\Error\ErrorHandler; +use Overblog\GraphQLBundle\Error\UserWarning; +use Overblog\GraphQLBundle\Event\Events; use Overblog\GraphQLBundle\EventListener\ClassLoaderListener; +use Overblog\GraphQLBundle\EventListener\DebugListener; +use Overblog\GraphQLBundle\EventListener\ErrorHandlerListener; +use Overblog\GraphQLBundle\EventListener\ErrorLoggerListener; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface; use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; @@ -32,10 +40,10 @@ public function load(array $configs, ContainerBuilder $container) $this->setServicesAliases($config, $container); $this->setSchemaBuilderArguments($config, $container); $this->setSchemaArguments($config, $container); - $this->setErrorHandlerArguments($config, $container); + $this->setErrorHandler($config, $container); $this->setSecurity($config, $container); $this->setConfigBuilders($config, $container); - $this->setShowDebug($config, $container); + $this->setDebugListener($config, $container); $this->setDefinitionParameters($config, $container); $this->setClassLoaderListener($config, $container); $this->setCompilerCacheWarmer($config, $container); @@ -56,7 +64,7 @@ public function prepend(ContainerBuilder $container) public function getAlias() { - return 'overblog_graphql'; + return Configuration::NAME; } public function getConfiguration(array $config, ContainerBuilder $container) @@ -87,6 +95,7 @@ private function setClassLoaderListener(array $config, ContainerBuilder $contain $this->getAlias().'.event_listener.classloader_listener', new Definition(ClassLoaderListener::class) ); + $definition->setPublic(true); $definition->setArguments([new Reference($this->getAlias().'.cache_compiler')]); $definition->addTag('kernel.event_listener', ['event' => 'kernel.request', 'method' => 'load', 'priority' => 255]); $definition->addTag('kernel.event_listener', ['event' => 'console.command', 'method' => 'load', 'priority' => 255]); @@ -117,9 +126,16 @@ private function setExpressionLanguageDefaultParser(ContainerBuilder $container) $container->setDefinition($this->getAlias().'.cache_expression_language_parser.default', $definition); } - private function setShowDebug(array $config, ContainerBuilder $container) + private function setDebugListener(array $config, ContainerBuilder $container) { - $container->getDefinition($this->getAlias().'.request_executor')->replaceArgument(4, $config['definitions']['show_debug_info']); + if ($config['definitions']['show_debug_info']) { + $definition = $container->setDefinition( + DebugListener::class, + new Definition(DebugListener::class) + ); + $definition->addTag('kernel.event_listener', ['event' => Events::PRE_EXECUTOR, 'method' => 'onPreExecutor']); + $definition->addTag('kernel.event_listener', ['event' => Events::POST_EXECUTOR, 'method' => 'onPostExecutor']); + } } private function setConfigBuilders(array $config, ContainerBuilder $container) @@ -157,27 +173,37 @@ private function setSecurity(array $config, ContainerBuilder $container) } } - private function setErrorHandlerArguments(array $config, ContainerBuilder $container) + private function setErrorHandler(array $config, ContainerBuilder $container) { - $errorHandlerDefinition = $container->getDefinition($this->getAlias().'.error_handler'); - - if (isset($config['definitions']['internal_error_message'])) { - $errorHandlerDefinition->replaceArgument(0, $config['definitions']['internal_error_message']); - } - - if (isset($config['definitions']['map_exceptions_to_parent'])) { - $errorHandlerDefinition->replaceArgument( - 3, - $config['definitions']['map_exceptions_to_parent'] - ); - } + if ($config['errors_handler']['enabled']) { + $id = $this->getAlias().'.error_handler'; + $errorHandlerDefinition = $container->setDefinition($id, new Definition(ErrorHandler::class)); + $errorHandlerDefinition->setPublic(false) + ->setArguments( + [ + new Reference('event_dispatcher'), + $config['errors_handler']['internal_error_message'], + $this->buildExceptionMap($config['errors_handler']['exceptions']), + $config['errors_handler']['map_exceptions_to_parent'], + ] + ) + ; - if (isset($config['definitions']['exceptions'])) { - $errorHandlerDefinition - ->replaceArgument(2, $this->buildExceptionMap($config['definitions']['exceptions'])) - ->addMethodCall('setUserWarningClass', [$config['definitions']['exceptions']['types']['warnings']]) - ->addMethodCall('setUserErrorClass', [$config['definitions']['exceptions']['types']['errors']]) + $errorHandlerListenerDefinition = $container->setDefinition(ErrorHandlerListener::class, new Definition(ErrorHandlerListener::class)); + $errorHandlerListenerDefinition->setPublic(true) + ->setArguments([new Reference($id), $config['errors_handler']['rethrow_internal_exceptions'], $config['errors_handler']['debug']]) + ->addTag('kernel.event_listener', ['event' => Events::POST_EXECUTOR, 'method' => 'onPostExecutor']) ; + + if ($config['errors_handler']['log']) { + $loggerServiceId = $config['errors_handler']['logger_service']; + $invalidBehavior = ErrorLoggerListener::DEFAULT_LOGGER_SERVICE === $loggerServiceId ? ContainerInterface::NULL_ON_INVALID_REFERENCE : ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE; + $errorHandlerListenerDefinition = $container->setDefinition(ErrorLoggerListener::class, new Definition(ErrorLoggerListener::class)); + $errorHandlerListenerDefinition->setPublic(true) + ->setArguments([new Reference($loggerServiceId, $invalidBehavior)]) + ->addTag('kernel.event_listener', ['event' => Events::ERROR_FORMATTING, 'method' => 'onErrorFormatting']) + ; + } } } @@ -225,15 +251,14 @@ private function setServicesAliases(array $config, ContainerBuilder $container) private function buildExceptionMap(array $exceptionConfig) { $exceptionMap = []; - $typeMap = $exceptionConfig['types']; + $errorsMapping = [ + 'errors' => UserError::class, + 'warnings' => UserWarning::class, + ]; foreach ($exceptionConfig as $type => $exceptionList) { - if ('types' === $type) { - continue; - } - foreach ($exceptionList as $exception) { - $exceptionMap[$exception] = $typeMap[$type]; + $exceptionMap[$exception] = $errorsMapping[$type]; } } diff --git a/Error/ErrorHandler.php b/Error/ErrorHandler.php index 6cc21ac30..a8e40f88d 100644 --- a/Error/ErrorHandler.php +++ b/Error/ErrorHandler.php @@ -2,21 +2,22 @@ namespace Overblog\GraphQLBundle\Error; +use GraphQL\Error\ClientAware; +use GraphQL\Error\Debug; use GraphQL\Error\Error as GraphQLError; +use GraphQL\Error\FormattedError; use GraphQL\Error\UserError as GraphQLUserError; use GraphQL\Executor\ExecutionResult; -use Psr\Log\LoggerInterface; -use Psr\Log\LogLevel; -use Psr\Log\NullLogger; +use Overblog\GraphQLBundle\Event\ErrorFormattingEvent; +use Overblog\GraphQLBundle\Event\Events; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; class ErrorHandler { const DEFAULT_ERROR_MESSAGE = 'Internal server Error'; - const DEFAULT_USER_WARNING_CLASS = UserWarning::class; - const DEFAULT_USER_ERROR_CLASS = UserError::class; - /** @var LoggerInterface */ - private $logger; + /** @var EventDispatcherInterface */ + private $dispatcher; /** @var string */ private $internalErrorMessage; @@ -24,22 +25,16 @@ class ErrorHandler /** @var array */ private $exceptionMap; - /** @var string */ - private $userWarningClass = self::DEFAULT_USER_WARNING_CLASS; - - /** @var string */ - private $userErrorClass = self::DEFAULT_USER_ERROR_CLASS; - /** @var bool */ private $mapExceptionsToParent; public function __construct( + EventDispatcherInterface $dispatcher, $internalErrorMessage = null, - LoggerInterface $logger = null, array $exceptionMap = [], $mapExceptionsToParent = false ) { - $this->logger = (null === $logger) ? new NullLogger() : $logger; + $this->dispatcher = $dispatcher; if (empty($internalErrorMessage)) { $internalErrorMessage = self::DEFAULT_ERROR_MESSAGE; } @@ -48,18 +43,30 @@ public function __construct( $this->mapExceptionsToParent = $mapExceptionsToParent; } - public function setUserWarningClass($userWarningClass) + public function handleErrors(ExecutionResult $executionResult, $throwRawException = false, $debug = false) { - $this->userWarningClass = $userWarningClass; - - return $this; + $errorFormatter = $this->createErrorFormatter($debug); + $executionResult->setErrorFormatter($errorFormatter); + $exceptions = $this->treatExceptions($executionResult->errors, $throwRawException); + $executionResult->errors = $exceptions['errors']; + if (!empty($exceptions['extensions']['warnings'])) { + $executionResult->extensions['warnings'] = array_map($errorFormatter, $exceptions['extensions']['warnings']); + } } - public function setUserErrorClass($userErrorClass) + private function createErrorFormatter($debug = false) { - $this->userErrorClass = $userErrorClass; + $debugMode = false; + if ($debug) { + $debugMode = Debug::INCLUDE_TRACE | Debug::INCLUDE_DEBUG_MESSAGE; + } - return $this; + return function (GraphQLError $error) use ($debugMode) { + $event = new ErrorFormattingEvent($error, FormattedError::createFromException($error, $debugMode, $this->internalErrorMessage)); + $this->dispatcher->dispatch(Events::ERROR_FORMATTING, $event); + + return $event->getFormattedError()->getArrayCopy(); + }; } /** @@ -68,9 +75,9 @@ public function setUserErrorClass($userErrorClass) * * @return array * - * @throws \Exception + * @throws \Error|\Exception */ - protected function treatExceptions(array $errors, $throwRawException) + private function treatExceptions(array $errors, $throwRawException) { $treatedExceptions = [ 'errors' => [], @@ -80,88 +87,71 @@ protected function treatExceptions(array $errors, $throwRawException) ]; /** @var GraphQLError $error */ - foreach ($errors as $error) { + foreach ($this->flattenErrors($errors) as $error) { $rawException = $this->convertException($error->getPrevious()); // raw GraphQL Error or InvariantViolation exception - if (null === $rawException || $rawException instanceof GraphQLUserError) { + if (null === $rawException) { $treatedExceptions['errors'][] = $error; continue; } + // recreate a error with converted exception + $errorWithConvertedException = new GraphQLError( + $error->getMessage(), + $error->nodes, + $error->getSource(), + $error->getPositions(), + $error->path, + $rawException + ); + // user error - if ($rawException instanceof $this->userErrorClass) { - $treatedExceptions['errors'][] = $error; - if ($rawException->getPrevious()) { - $this->logException($rawException->getPrevious()); - } + if ($rawException instanceof GraphQLUserError) { + $treatedExceptions['errors'][] = $errorWithConvertedException; continue; } // user warning - if ($rawException instanceof $this->userWarningClass) { - $treatedExceptions['extensions']['warnings'][] = $error; - if ($rawException->getPrevious()) { - $this->logException($rawException->getPrevious(), LogLevel::WARNING); - } - continue; - } - - // multiple errors - if ($rawException instanceof UserErrors) { - $rawExceptions = $rawException; - foreach ($rawExceptions->getErrors() as $rawException) { - $treatedExceptions['errors'][] = GraphQLError::createLocatedError($rawException, $error->nodes); - } + if ($rawException instanceof UserWarning) { + $treatedExceptions['extensions']['warnings'][] = $errorWithConvertedException; continue; } - // if is a try catch exception wrapped in Error + // if is a catch exception wrapped in Error if ($throwRawException) { throw $rawException; } - $this->logException($rawException, LogLevel::CRITICAL); - - $treatedExceptions['errors'][] = new GraphQLError( - $this->internalErrorMessage, - $error->nodes, - $error->getSource(), - $error->getPositions(), - $error->path, - $rawException - ); + $treatedExceptions['errors'][] = $errorWithConvertedException; } return $treatedExceptions; } /** - * @param \Exception|\Error $exception - * @param string $errorLevel + * @param GraphQLError[] $errors + * + * @return GraphQLError[] */ - public function logException($exception, $errorLevel = LogLevel::ERROR) + private function flattenErrors(array $errors) { - $message = sprintf( - '%s: %s[%d] (caught exception) at %s line %s.', - get_class($exception), - $exception->getMessage(), - $exception->getCode(), - $exception->getFile(), - $exception->getLine() - ); - - $this->logger->$errorLevel($message, ['exception' => $exception]); - } + $flattenErrors = []; - public function handleErrors(ExecutionResult $executionResult, $throwRawException = false) - { - $executionResult->setErrorFormatter(sprintf('\%s::formatError', GraphQLError::class)); - $exceptions = $this->treatExceptions($executionResult->errors, $throwRawException); - $executionResult->errors = $exceptions['errors']; - if (!empty($exceptions['extensions']['warnings'])) { - $executionResult->extensions['warnings'] = array_map([GraphQLError::class, 'formatError'], $exceptions['extensions']['warnings']); + foreach ($errors as $error) { + $rawException = $error->getPrevious(); + // multiple errors + if ($rawException instanceof UserErrors) { + $rawExceptions = $rawException; + foreach ($rawExceptions->getErrors() as $rawException) { + $flattenErrors[] = GraphQLError::createLocatedError($rawException, $error->nodes, $error->path); + } + } else { + $flattenErrors[] = $error; + } } + + return $flattenErrors; } /** @@ -172,10 +162,10 @@ public function handleErrors(ExecutionResult $executionResult, $throwRawExceptio * * @return \Exception|\Error */ - protected function convertException($rawException = null) + private function convertException($rawException = null) { - if (null === $rawException) { - return; + if (null === $rawException || $rawException instanceof ClientAware) { + return $rawException; } $errorClass = $this->findErrorClass($rawException); diff --git a/Error/UserError.php b/Error/UserError.php index 7d5af861a..521d682d8 100644 --- a/Error/UserError.php +++ b/Error/UserError.php @@ -2,9 +2,11 @@ namespace Overblog\GraphQLBundle\Error; +use GraphQL\Error\UserError as BaseUserError; + /** * Class UserError. */ -class UserError extends UserFacingError +class UserError extends BaseUserError { } diff --git a/Error/UserErrors.php b/Error/UserErrors.php index 2268db060..1e87c5670 100644 --- a/Error/UserErrors.php +++ b/Error/UserErrors.php @@ -5,7 +5,7 @@ /** * Class UserErrors. */ -class UserErrors extends UserFacingError +class UserErrors extends \RuntimeException { /** @var UserError[] */ private $errors = []; @@ -27,7 +27,7 @@ public function setErrors(array $errors) } /** - * @param string|UserError $error + * @param string|\GraphQL\Error\UserError $error * * @return $this */ @@ -35,8 +35,8 @@ public function addError($error) { if (is_string($error)) { $error = new UserError($error); - } elseif (!is_object($error) || !$error instanceof UserError) { - throw new \InvalidArgumentException(sprintf('Error must be string or instance of %s.', UserError::class)); + } elseif (!is_object($error) || !$error instanceof \GraphQL\Error\UserError) { + throw new \InvalidArgumentException(sprintf('Error must be string or instance of %s.', \GraphQL\Error\UserError::class)); } $this->errors[] = $error; diff --git a/Error/UserFacingError.php b/Error/UserFacingError.php deleted file mode 100644 index 6237ce6cc..000000000 --- a/Error/UserFacingError.php +++ /dev/null @@ -1,10 +0,0 @@ -error = $error; + $this->formattedError = new \ArrayObject($formattedError); + } + + public function getError() + { + return $this->error; + } + + /** + * @return \ArrayObject + */ + public function getFormattedError() + { + return $this->formattedError; + } +} diff --git a/Event/Events.php b/Event/Events.php index 02f137226..095a73f65 100644 --- a/Event/Events.php +++ b/Event/Events.php @@ -5,4 +5,7 @@ final class Events { const EXECUTOR_CONTEXT = 'graphql.executor.context'; + const PRE_EXECUTOR = 'graphql.pre_executor'; + const POST_EXECUTOR = 'graphql.post_executor'; + const ERROR_FORMATTING = 'graphql.error_formatting'; } diff --git a/Event/ExecutorArgumentsEvent.php b/Event/ExecutorArgumentsEvent.php new file mode 100644 index 000000000..dcc9ee4db --- /dev/null +++ b/Event/ExecutorArgumentsEvent.php @@ -0,0 +1,133 @@ +setSchema($schema); + $instance->setRequestString($requestString); + $instance->setContextValue($contextValue); + $instance->setRootValue($rootValue); + $instance->setVariableValue($variableValue); + $instance->setOperationName($operationName); + + return $instance; + } + + /** + * @param null|string $operationName + */ + public function setOperationName($operationName = null) + { + $this->operationName = $operationName; + } + + public function setContextValue(\ArrayObject $contextValue = null) + { + $this->contextValue = $contextValue; + } + + /** + * @param mixed $rootValue + */ + public function setRootValue($rootValue = null) + { + $this->rootValue = $rootValue; + } + + /** + * @param string $requestString + */ + public function setRequestString($requestString) + { + $this->requestString = $requestString; + } + + public function setVariableValue(array $variableValue = null) + { + $this->variableValue = $variableValue; + } + + public function setSchema(Schema $schema) + { + $this->schema = $schema; + } + + /** + * @return Schema + */ + public function getSchema() + { + return $this->schema; + } + + /** + * @return string + */ + public function getRequestString() + { + return $this->requestString; + } + + /** + * @return array|null + */ + public function getRootValue() + { + return $this->rootValue; + } + + /** + * @return \ArrayObject + */ + public function getContextValue() + { + return $this->contextValue; + } + + /** + * @return array|null + */ + public function getVariableValue() + { + return $this->variableValue; + } + + /** + * @return null|string + */ + public function getOperationName() + { + return $this->operationName; + } +} diff --git a/Event/ExecutorContextEvent.php b/Event/ExecutorContextEvent.php index eb800f720..86e111166 100644 --- a/Event/ExecutorContextEvent.php +++ b/Event/ExecutorContextEvent.php @@ -4,28 +4,24 @@ use Symfony\Component\EventDispatcher\Event; -class ExecutorContextEvent extends Event +final class ExecutorContextEvent extends Event { - private $executorContext = []; + /** @var \ArrayObject */ + private $executorContext; - public function __construct(array $executorContext) + /** + * @param \ArrayObject $executorContext + */ + public function __construct(\ArrayObject $executorContext) { $this->executorContext = $executorContext; } /** - * @return array + * @return \ArrayObject */ public function getExecutorContext() { return $this->executorContext; } - - /** - * @param array $executionContext - */ - public function setExecutorContext(array $executionContext) - { - $this->executorContext = $executionContext; - } } diff --git a/Event/ExecutorResultEvent.php b/Event/ExecutorResultEvent.php new file mode 100644 index 000000000..377fce487 --- /dev/null +++ b/Event/ExecutorResultEvent.php @@ -0,0 +1,25 @@ +result = $result; + } + + /** + * @return ExecutionResult + */ + public function getResult() + { + return $this->result; + } +} diff --git a/EventListener/ClassLoaderListener.php b/EventListener/ClassLoaderListener.php index ee4db36e6..8d57a68c3 100644 --- a/EventListener/ClassLoaderListener.php +++ b/EventListener/ClassLoaderListener.php @@ -4,7 +4,7 @@ use Overblog\GraphQLBundle\Generator\TypeGenerator; -class ClassLoaderListener +final class ClassLoaderListener { private $typeGenerator; diff --git a/EventListener/DebugListener.php b/EventListener/DebugListener.php new file mode 100644 index 000000000..fa3fab846 --- /dev/null +++ b/EventListener/DebugListener.php @@ -0,0 +1,28 @@ +startTime = microtime(true); + $this->startMemoryUsage = memory_get_usage(true); + } + + public function onPostExecutor(ExecutorResultEvent $executorResultEvent) + { + $executorResultEvent->getResult()->extensions['debug'] = [ + 'executionTime' => sprintf('%d ms', round(microtime(true) - $this->startTime, 3) * 1000), + 'memoryUsage' => sprintf('%.2F MiB', (memory_get_usage(true) - $this->startMemoryUsage) / 1024 / 1024), + ]; + } +} diff --git a/EventListener/ErrorHandlerListener.php b/EventListener/ErrorHandlerListener.php new file mode 100644 index 000000000..a94005171 --- /dev/null +++ b/EventListener/ErrorHandlerListener.php @@ -0,0 +1,31 @@ +errorHandler = $errorHandler; + $this->throwException = $throwException; + $this->debug = $debug; + } + + public function onPostExecutor(ExecutorResultEvent $executorResultEvent) + { + $result = $executorResultEvent->getResult(); + $this->errorHandler->handleErrors($result, $this->throwException, $this->debug); + } +} diff --git a/EventListener/ErrorLoggerListener.php b/EventListener/ErrorLoggerListener.php new file mode 100644 index 000000000..135d88ae3 --- /dev/null +++ b/EventListener/ErrorLoggerListener.php @@ -0,0 +1,77 @@ +logger = null === $logger ? new NullLogger() : $logger; + } + + public function onErrorFormatting(ErrorFormattingEvent $event) + { + $error = $event->getError(); + + if ($error->getPrevious()) { + $exception = $error->getPrevious(); + if ($exception instanceof UserError) { + if ($exception->getPrevious()) { + $this->log($exception->getPrevious()); + } + + return; + } + + if ($exception instanceof UserWarning) { + if ($exception->getPrevious()) { + $this->log($exception->getPrevious(), LogLevel::WARNING); + } + + return; + } + $this->log($error->getPrevious(), LogLevel::CRITICAL); + } + } + + /** + * @param \Throwable $throwable + * @param string $errorLevel + */ + public function log($throwable, $errorLevel = LogLevel::ERROR) + { + $this->logger->$errorLevel(self::serializeThrowableObject($throwable), ['throwable' => $throwable]); + } + + /** + * @param \Throwable $throwable + * + * @return string + */ + private static function serializeThrowableObject($throwable) + { + $message = sprintf( + '[GraphQL] %s: %s[%d] (caught throwable) at %s line %s.', + get_class($throwable), + $throwable->getMessage(), + $throwable->getCode(), + $throwable->getFile(), + $throwable->getLine() + ); + + return $message; + } +} diff --git a/EventListener/RequestFilesListener.php b/EventListener/RequestFilesListener.php index d01a193ba..bb96c32d5 100644 --- a/EventListener/RequestFilesListener.php +++ b/EventListener/RequestFilesListener.php @@ -6,7 +6,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; -class RequestFilesListener +final class RequestFilesListener { /** @var RequestStack */ private $requestStack; @@ -26,6 +26,5 @@ public function onExecutorContextEvent(ExecutorContextEvent $event) $context = $event->getExecutorContext(); $context['request_files'] = $request->files; - $event->setExecutorContext($context); } } diff --git a/Executor/Executor.php b/Executor/Executor.php index c839f64c9..3495d7707 100644 --- a/Executor/Executor.php +++ b/Executor/Executor.php @@ -17,21 +17,15 @@ class Executor implements ExecutorInterface public function execute(Schema $schema, $requestString, $rootValue = null, $contextValue = null, $variableValues = null, $operationName = null) { $args = func_get_args(); + array_unshift($args, $this->promiseAdapter); - if (null === $this->promiseAdapter) { - $method = 'executeQuery'; - } else { - array_unshift($args, $this->promiseAdapter); - $method = 'promiseToExecute'; - } - - return call_user_func_array(sprintf('\%s::%s', GraphQL::class, $method), $args); + return call_user_func_array([GraphQL::class, 'promiseToExecute'], $args); } /** * {@inheritdoc} */ - public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null) + public function setPromiseAdapter(PromiseAdapter $promiseAdapter) { $this->promiseAdapter = $promiseAdapter; } @@ -41,6 +35,6 @@ public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null) */ public function setDefaultFieldResolver(callable $fn) { - call_user_func_array(sprintf('\%s::setDefaultFieldResolver', GraphQL::class), func_get_args()); + call_user_func_array([GraphQL::class, 'setDefaultFieldResolver'], func_get_args()); } } diff --git a/Executor/ExecutorInterface.php b/Executor/ExecutorInterface.php index c152029a0..1e9da510e 100644 --- a/Executor/ExecutorInterface.php +++ b/Executor/ExecutorInterface.php @@ -24,7 +24,7 @@ public function execute(Schema $schema, $requestString, $rootValue = null, $cont /** * @param PromiseAdapter|null $promiseAdapter */ - public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null); + public function setPromiseAdapter(PromiseAdapter $promiseAdapter); /** * @param callable $fn diff --git a/README.md b/README.md index 6b149e9ab..7e29b42f4 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,8 @@ Documentation - [Fields public control](Resources/doc/security/fields-public-control.md) - [Limiting query depth](Resources/doc/security/limiting-query-depth.md) - [Query complexity analysis](Resources/doc/security/query-complexity-analysis.md) - - [Errors handling](Resources/doc/security/errors-handling.md) +- [Errors handling](Resources/doc/error-handling/index.md) +- [Events](Resources/doc/events/index.md) Talks and slides to help you start ---------------------------------- diff --git a/Request/Executor.php b/Request/Executor.php index a3e04183a..fd88db1f2 100644 --- a/Request/Executor.php +++ b/Request/Executor.php @@ -3,14 +3,16 @@ namespace Overblog\GraphQLBundle\Request; use GraphQL\Executor\ExecutionResult; +use GraphQL\Executor\Promise\Promise; use GraphQL\Executor\Promise\PromiseAdapter; use GraphQL\Type\Schema; use GraphQL\Validator\DocumentValidator; use GraphQL\Validator\Rules\QueryComplexity; use GraphQL\Validator\Rules\QueryDepth; -use Overblog\GraphQLBundle\Error\ErrorHandler; use Overblog\GraphQLBundle\Event\Events; +use Overblog\GraphQLBundle\Event\ExecutorArgumentsEvent; use Overblog\GraphQLBundle\Event\ExecutorContextEvent; +use Overblog\GraphQLBundle\Event\ExecutorResultEvent; use Overblog\GraphQLBundle\Executor\ExecutorInterface; use Overblog\GraphQLBundle\Executor\Promise\PromiseAdapterInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -26,15 +28,6 @@ class Executor /** @var EventDispatcherInterface|null */ private $dispatcher; - /** @var bool */ - private $throwException; - - /** @var ErrorHandler|null */ - private $errorHandler; - - /** @var bool */ - private $hasDebugInfo; - /** @var ExecutorInterface */ private $executor; @@ -46,18 +39,12 @@ class Executor public function __construct( ExecutorInterface $executor, - EventDispatcherInterface $dispatcher = null, - $throwException = false, - ErrorHandler $errorHandler = null, - $hasDebugInfo = false, - PromiseAdapter $promiseAdapter = null, + EventDispatcherInterface $dispatcher, + PromiseAdapter $promiseAdapter, callable $defaultFieldResolver = null ) { $this->executor = $executor; $this->dispatcher = $dispatcher; - $this->throwException = (bool) $throwException; - $this->errorHandler = $errorHandler; - $hasDebugInfo ? $this->enabledDebugInfo() : $this->disabledDebugInfo(); $this->promiseAdapter = $promiseAdapter; $this->defaultFieldResolver = $defaultFieldResolver; } @@ -69,7 +56,7 @@ public function setExecutor(ExecutorInterface $executor) return $this; } - public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null) + public function setPromiseAdapter(PromiseAdapter $promiseAdapter) { $this->promiseAdapter = $promiseAdapter; @@ -78,6 +65,9 @@ public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null) /** * @param string $name + * @param Schema $schema + * + * @return $this */ public function addSchema($name, Schema $schema) { @@ -86,23 +76,27 @@ public function addSchema($name, Schema $schema) return $this; } - public function enabledDebugInfo() - { - $this->hasDebugInfo = true; - - return $this; - } - - public function disabledDebugInfo() + /** + * @param string|null $name + * + * @return Schema + */ + public function getSchema($name = null) { - $this->hasDebugInfo = false; + if (empty($this->schemas)) { + throw new \RuntimeException('At least one schema should be declare.'); + } - return $this; - } + if (null === $name) { + $schema = array_values($this->schemas)[0]; + } else { + if (!isset($this->schemas[$name])) { + throw new NotFoundHttpException(sprintf('Could not found "%s" schema.', $name)); + } + $schema = $this->schemas[$name]; + } - public function hasDebugInfo() - { - return $this->hasDebugInfo; + return $schema; } public function setMaxQueryDepth($maxQueryDepth) @@ -120,113 +114,105 @@ public function setMaxQueryComplexity($maxQueryComplexity) } /** - * @param bool $throwException + * @param null|string $schemaName + * @param array $request + * @param null|array|\ArrayObject|object $rootValue * - * @return $this + * @return ExecutionResult */ - public function setThrowException($throwException) + public function execute($schemaName, array $request, $rootValue = null) { - $this->throwException = (bool) $throwException; - - return $this; - } + $executorArgumentsEvent = $this->preExecute( + $this->getSchema($schemaName), + isset($request[ParserInterface::PARAM_QUERY]) ? $request[ParserInterface::PARAM_QUERY] : null, + new \ArrayObject(), + $rootValue, + $request[ParserInterface::PARAM_VARIABLES], + isset($request[ParserInterface::PARAM_OPERATION_NAME]) ? $request[ParserInterface::PARAM_OPERATION_NAME] : null + ); - public function execute(array $data, array $context = [], $schemaName = null) - { - if (null !== $this->dispatcher) { - $event = new ExecutorContextEvent($context); - $this->dispatcher->dispatch(Events::EXECUTOR_CONTEXT, $event); - $context = $event->getExecutorContext(); - } + $result = $this->executor->execute( + $executorArgumentsEvent->getSchema(), + $executorArgumentsEvent->getRequestString(), + $executorArgumentsEvent->getRootValue(), + $executorArgumentsEvent->getContextValue(), + $executorArgumentsEvent->getVariableValue(), + $executorArgumentsEvent->getOperationName() + ); - if ($this->promiseAdapter) { - if (!$this->promiseAdapter instanceof PromiseAdapterInterface && !is_callable([$this->promiseAdapter, 'wait'])) { - throw new \RuntimeException( - sprintf( - 'PromiseAdapter should be an object instantiating "%s" or "%s" with a "wait" method.', - PromiseAdapterInterface::class, - PromiseAdapter::class - ) - ); - } - } + $result = $this->postExecute($result); - $schema = $this->getSchema($schemaName); + return $result; + } - $startTime = microtime(true); - $startMemoryUsage = memory_get_usage(true); + /** + * @param Schema $schema + * @param string $requestString + * @param \ArrayObject $contextValue + * @param mixed $rootValue + * @param array|null $variableValue + * @param string|null $operationName + * + * @return ExecutorArgumentsEvent + */ + private function preExecute( + Schema $schema, + $requestString, + \ArrayObject $contextValue, + $rootValue = null, + array $variableValue = null, + $operationName = null + ) { + $this->checkPromiseAdapter(); $this->executor->setPromiseAdapter($this->promiseAdapter); // this is needed when not using only generated types if ($this->defaultFieldResolver) { $this->executor->setDefaultFieldResolver($this->defaultFieldResolver); } + $this->dispatcher->dispatch(Events::EXECUTOR_CONTEXT, new ExecutorContextEvent($contextValue)); - $result = $this->executor->execute( - $schema, - isset($data[ParserInterface::PARAM_QUERY]) ? $data[ParserInterface::PARAM_QUERY] : null, - $context, - $context, - $data[ParserInterface::PARAM_VARIABLES], - isset($data[ParserInterface::PARAM_OPERATION_NAME]) ? $data[ParserInterface::PARAM_OPERATION_NAME] : null + return $this->dispatcher->dispatch( + Events::PRE_EXECUTOR, + ExecutorArgumentsEvent::create($schema, $requestString, $contextValue, $rootValue, $variableValue, $operationName) ); - - if ($this->promiseAdapter) { - $result = $this->promiseAdapter->wait($result); - } - - if (!is_object($result) || !$result instanceof ExecutionResult) { - throw new \RuntimeException( - sprintf('Execution result should be an object instantiating "%s".', ExecutionResult::class) - ); - } - - return $this->prepareResult($result, $startTime, $startMemoryUsage); } /** - * @param ExecutionResult $result - * @param int $startTime - * @param int $startMemoryUsage + * @param ExecutionResult|Promise $result * * @return ExecutionResult */ - private function prepareResult($result, $startTime, $startMemoryUsage) + private function postExecute($result) { - if ($this->hasDebugInfo()) { - $result->extensions['debug'] = [ - 'executionTime' => sprintf('%d ms', round(microtime(true) - $startTime, 3) * 1000), - 'memoryUsage' => sprintf('%.2F MiB', (memory_get_usage(true) - $startMemoryUsage) / 1024 / 1024), - ]; + if ($result instanceof Promise) { + $result = $this->promiseAdapter->wait($result); } - if (null !== $this->errorHandler) { - $this->errorHandler->handleErrors($result, $this->throwException); - } + $this->checkExecutionResult($result); - return $result; + return $this->dispatcher->dispatch(Events::POST_EXECUTOR, new ExecutorResultEvent($result))->getResult(); } - /** - * @param string|null $name - * - * @return Schema - */ - public function getSchema($name = null) + private function checkPromiseAdapter() { - if (empty($this->schemas)) { - throw new \RuntimeException('At least one schema should be declare.'); + if ($this->promiseAdapter && !$this->promiseAdapter instanceof PromiseAdapterInterface && !is_callable([$this->promiseAdapter, 'wait'])) { + throw new \RuntimeException( + sprintf( + 'PromiseAdapter should be an object instantiating "%s" or "%s" with a "wait" method.', + PromiseAdapterInterface::class, + PromiseAdapter::class + ) + ); } + } - if (null === $name) { - $schema = array_values($this->schemas)[0]; - } else { - if (!isset($this->schemas[$name])) { - throw new NotFoundHttpException(sprintf('Could not found "%s" schema.', $name)); - } - $schema = $this->schemas[$name]; + private function checkExecutionResult($result) + { + if (!is_object($result) || !$result instanceof ExecutionResult) { + throw new \RuntimeException( + sprintf('Execution result should be an object instantiating "%s".', ExecutionResult::class) + ); } - - return $schema; } } diff --git a/Resources/config/services.yml b/Resources/config/services.yml index b0d6a640d..dae45a3cf 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -2,15 +2,6 @@ parameters: overblog_graphql_types.config: [] services: - overblog_graphql.error_handler: - class: Overblog\GraphQLBundle\Error\ErrorHandler - public: false - arguments: - - ~ - - "@?logger" - - [] - - false - overblog_graphql.executor.default: class: Overblog\GraphQLBundle\Executor\Executor public: false @@ -21,9 +12,6 @@ services: arguments: - "@overblog_graphql.executor" - "@event_dispatcher" - - "%kernel.debug%" - - "@overblog_graphql.error_handler" - - false - "@overblog_graphql.promise_adapter" - "%overblog_graphql.default_resolver%" calls: @@ -104,7 +92,7 @@ services: - ["addImplement", ["Overblog\\GraphQLBundle\\Definition\\Type\\GeneratedTypeInterface"]] - ["setExpressionLanguage", ["@overblog_graphql.expression_language"]] - overblog_graphql.event_listener.request_file_listener: + Overblog\GraphQLBundle\EventListener\RequestFilesListener: class: Overblog\GraphQLBundle\EventListener\RequestFilesListener public: true arguments: diff --git a/Resources/doc/security/errors-handling.md b/Resources/doc/error-handling/index.md similarity index 59% rename from Resources/doc/security/errors-handling.md rename to Resources/doc/error-handling/index.md index dd44950d0..a6159fbba 100644 --- a/Resources/doc/security/errors-handling.md +++ b/Resources/doc/error-handling/index.md @@ -1,7 +1,7 @@ Errors handling =============== -In no debug mode all errors will be logged and replace by a generic error message. +By default in no debug mode all errors will be logged and replace by a generic error message. Only query parsed error won't be replaced. If you want to send explicit error or warnings messages to your users you can use exceptions: @@ -89,21 +89,10 @@ class CharacterResolver Warnings can be found in the response under `extensions.warnings` map. -You can also custom the generic error message - -```yaml -#app/config/config.yml -overblog_graphql: - #... - definitions: - internal_error_message: "An error occurred, please retry later or contact us!" -``` - 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: @@ -118,5 +107,67 @@ overblog_graphql: - "InvalidArgumentException" ``` +You can custom the default errors handler using configuration: + +```yaml +overblog_graphql: + errors_handler: + enabled: true # false will totally disabled errors handling + internal_error_message: ~ # custom generic error message + rethrow_internal_exceptions: false # re-throw internal exception + debug: false # will add trace stack and debugMessage to error + log: true # false will disabled the default logging behavior + logger_service: logger # the service to use to log +``` + The message of those exceptions are then shown to the user like other `UserError`s or `UserWarning`s. + +Custom error Formatting +------------------------- + +see [error formatting Event](../events/index.md#error-formatting) + +Custom error handling / formatting +----------------------------------- + +This can also be done by using events. +* First totally disabled default errors handler: + ```yaml + overblog_graphql: + errors_handler: false + ``` +* Listen to [executor result event](../events/index.md#executor-result) + ```yaml + App\EventListener\MyErrorHandler: + tags: + - { name: kernel.event_listener, event: graphql.post_executor, method: onPostExecutor } + ``` + + ```php + getResult() + ->setErrorFormatter($myErrorFormatter) + ->setErrorsHandler($myErrorHandler); + } + } + ``` diff --git a/Resources/doc/events/index.md b/Resources/doc/events/index.md new file mode 100644 index 000000000..566931819 --- /dev/null +++ b/Resources/doc/events/index.md @@ -0,0 +1,162 @@ +Events +========= + +[Events](http://symfony.com/doc/master/event_dispatcher.html) are a way to hook +into the execution flow. This bundle provides various events from executor +and context value initialization, executor result formatting (data, errors, extensions), +errors/warnings formatting. + +With this in mind we now turn to explain each one of them. + +Executor context value +---------------------- + +*Event:* `graphql.executor.context` + +This event can be listen to initialize executor `contextValue` argument. + + +Executor initialisation +----------------------- + +*Event:* `graphql.pre_executor` + +This event can be listen to initialize executor execute arguments +(`schema`, `requestString`, `rootValue`, `contextValue`, `variableValue`, `operationName`). + +For example: + +* Initializing `rootValue` with the current user (we assume that user is fully authenticated) + + ```yaml + App\EventListener\RootValueInitialization: + tags: + - { name: kernel.event_listener, event: graphql.pre_executor, method: onPreExecutor } + ``` + + ```php + token = $token; + } + + public function onPreExecutor(ExecutorArgumentsEvent $event) + { + $event->setRootValue($this->token->getUser()); + } + } + +Executor result +--------------- + +*Event:* `graphql.post_executor` + +This event can be listen to set custom error handling and formatting, it can also be use to +add information to `extensions` section. + +For example: + +* How to add `credits` entry in `extensions` section + + ```yaml + App\EventListener\Credits: + tags: + - { name: kernel.event_listener, event: graphql.post_executor, method: onPostExecutor } + ``` + + ```php + getResult()->extensions['credits'] = 'This api was powered by "OverblogGraphQLBundle".'; + } + } + ``` + + result: + ```json + { + "data": {"foo": "bar"}, + "extensions": { + "credits": "This api was powered by \"OverblogGraphQLBundle\"." + } + } + ``` + +Error formatting +---------------- + +*Event:* `graphql.error_formatting` + +This event can be listen to add or remove fields from result `errors` and `extensions.warnings` +sections, it can also be use to log errors or exception. Each single error or warning will trigger +an event. + +For example: + +* How to add error code: + + ```yaml + App\EventListener\ErrorCode: + tags: + - { name: kernel.event_listener, event: graphql.error_formatting, method: onErrorFormatting } + ``` + + ```php + getError(); + if ($error->getPrevious()) { + $code = $error->getPrevious()->getCode(); + } else { + $code = $error->getCode(); + } + $formattedError = $event->getFormattedError(); + $formattedError->offsetSet('code', $code); // or $formattedError['code'] = $code; + } + } + ``` + + result: + ```json + { + "data": null, + "errors": [ + { + "code": 123, + "category": "internal", + "message": "Internal server Error" + } + ] + } + ``` + +*Note:* +- This event is dispatch by this bundle default error handler, that the reason why, disabling +error handler will also disable this event. diff --git a/Resources/doc/security/index.md b/Resources/doc/security/index.md index 415a68a57..be362d504 100644 --- a/Resources/doc/security/index.md +++ b/Resources/doc/security/index.md @@ -6,4 +6,5 @@ Security * [Fields public control](fields-public-control.md) * [Limiting query depth](limiting-query-depth.md) * [Query complexity analysis](query-complexity-analysis.md) -* [Errors handling](errors-handling.md) + +Next step [handling errors](../error-handling/index.md) diff --git a/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php b/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php index 9665f7c51..59d260168 100644 --- a/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php +++ b/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php @@ -2,10 +2,10 @@ namespace Overblog\GraphQLBundle\Tests\DependencyInjection; +use GraphQL\Error\UserError; use Overblog\GraphQLBundle\Config\Processor\InheritanceProcessor; use Overblog\GraphQLBundle\DependencyInjection\OverblogGraphQLExtension; use Overblog\GraphQLBundle\DependencyInjection\OverblogGraphQLTypesExtension; -use Overblog\GraphQLBundle\Error\UserError; use Overblog\GraphQLBundle\Error\UserWarning; use Overblog\GraphQLBundle\Tests\DependencyInjection\Builder\PagerArgs; use Overblog\GraphQLBundle\Tests\DependencyInjection\Builder\RawIdField; @@ -86,7 +86,7 @@ public function testCustomExceptions() $ext->load( [ [ - 'definitions' => [ + 'errors_handler' => [ 'exceptions' => [ 'warnings' => [ ResourceNotFoundException::class, diff --git a/Tests/Error/ErrorHandlerTest.php b/Tests/Error/ErrorHandlerTest.php index 6b67aae3b..87bbaf39c 100644 --- a/Tests/Error/ErrorHandlerTest.php +++ b/Tests/Error/ErrorHandlerTest.php @@ -10,15 +10,21 @@ use Overblog\GraphQLBundle\Error\UserErrors; use Overblog\GraphQLBundle\Error\UserWarning; use PHPUnit\Framework\TestCase; +use Symfony\Component\EventDispatcher\EventDispatcher; class ErrorHandlerTest extends TestCase { /** @var ErrorHandler */ private $errorHandler; + /** @var EventDispatcher|\PHPUnit_Framework_MockObject_MockObject */ + private $dispatcher; + public function setUp() { - $this->errorHandler = new ErrorHandler(); + $this->dispatcher = $this->getMockBuilder(EventDispatcher::class)->setMethods(['dispatch'])->getMock(); + $this->dispatcher->expects($this->any())->method('dispatch')->willReturnArgument(1); + $this->errorHandler = new ErrorHandler($this->dispatcher); } public function testMaskErrorWithThrowExceptionSetToFalse() @@ -29,9 +35,9 @@ public function testMaskErrorWithThrowExceptionSetToFalse() new GraphQLError('Error without wrapped exception'), new GraphQLError('Error with wrapped exception', null, null, null, null, new \Exception('My Exception message')), new GraphQLError('Error with wrapped user error', null, null, null, null, new UserError('My User Error')), + new GraphQLError('Error with wrapped base user error', null, null, null, null, new GraphQLUserError('My bases User Error')), new GraphQLError('', null, null, null, null, new UserErrors(['My User Error 1', 'My User Error 2', new UserError('My User Error 3')])), new GraphQLError('Error with wrapped user warning', null, null, null, null, new UserWarning('My User Warning')), - new GraphQLError('Invalid value!', null, null, null, null, new GraphQLUserError('Invalid value!')), ] ); @@ -41,30 +47,38 @@ public function testMaskErrorWithThrowExceptionSetToFalse() 'errors' => [ [ 'message' => 'Error without wrapped exception', + 'category' => 'graphql', ], [ 'message' => ErrorHandler::DEFAULT_ERROR_MESSAGE, + 'category' => 'internal', ], [ 'message' => 'Error with wrapped user error', + 'category' => 'user', + ], + [ + 'message' => 'Error with wrapped base user error', + 'category' => 'user', ], [ 'message' => 'My User Error 1', + 'category' => 'user', ], [ 'message' => 'My User Error 2', + 'category' => 'user', ], [ 'message' => 'My User Error 3', - ], - [ - 'message' => 'Invalid value!', + 'category' => 'user', ], ], 'extensions' => [ 'warnings' => [ [ 'message' => 'Error with wrapped user warning', + 'category' => 'user', ], ], ], @@ -91,7 +105,7 @@ public function testMaskErrorWithWrappedExceptionAndThrowExceptionSetToTrue() public function testInvalidUserErrorsItem() { $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('Error must be string or instance of Overblog\GraphQLBundle\Error\UserError.'); + $this->expectExceptionMessage(sprintf('Error must be string or instance of %s', GraphQLUserError::class)); new UserErrors([ 'Some Error', @@ -114,6 +128,7 @@ public function testMaskErrorWithWrappedUserErrorAndThrowExceptionSetToTrue() 'errors' => [ [ 'message' => 'Error with wrapped user error', + 'category' => 'user', ], ], ]; @@ -121,6 +136,29 @@ public function testMaskErrorWithWrappedUserErrorAndThrowExceptionSetToTrue() $this->assertEquals($expected, $executionResult->toArray()); } + public function testDebugEnabled() + { + try { + throw new \Exception(); + } catch (\Exception $exception) { + $executionResult = new ExecutionResult( + null, + [ + new GraphQLError('Error wrapped exception', null, null, null, null, $exception), + ] + ); + + $this->errorHandler->handleErrors($executionResult, false, true); + + $errors = $executionResult->toArray()['errors']; + $this->assertCount(1, $errors); + $this->assertArrayHasKey('debugMessage', $errors[0]); + $this->assertEquals('Error wrapped exception', $errors[0]['debugMessage']); + $this->assertEquals(ErrorHandler::DEFAULT_ERROR_MESSAGE, $errors[0]['message']); + $this->assertArrayHasKey('trace', $errors[0]); + } + } + public function testMaskErrorWithoutWrappedExceptionAndThrowExceptionSetToTrue() { $executionResult = new ExecutionResult( @@ -136,6 +174,7 @@ public function testMaskErrorWithoutWrappedExceptionAndThrowExceptionSetToTrue() 'errors' => [ [ 'message' => 'Error without wrapped exception', + 'category' => 'graphql', ], ], ]; @@ -145,7 +184,7 @@ public function testMaskErrorWithoutWrappedExceptionAndThrowExceptionSetToTrue() public function testConvertExceptionToUserWarning() { - $errorHandler = new ErrorHandler(null, null, [\InvalidArgumentException::class => UserWarning::class]); + $errorHandler = new ErrorHandler($this->dispatcher, null, [\InvalidArgumentException::class => UserWarning::class]); $executionResult = new ExecutionResult( null, @@ -161,6 +200,7 @@ public function testConvertExceptionToUserWarning() 'warnings' => [ [ 'message' => 'Error with invalid argument exception', + 'category' => 'user', ], ], ], @@ -178,7 +218,7 @@ public function testConvertExceptionToUserWarning() */ public function testConvertExceptionUsingParentExceptionMatchesAlwaysFirstExactExceptionOtherwiseMatchesParent(array $exceptionMap, $mapExceptionsToParent, $expectedUserError) { - $errorHandler = new ErrorHandler(null, null, $exceptionMap, $mapExceptionsToParent); + $errorHandler = new ErrorHandler($this->dispatcher, null, $exceptionMap, $mapExceptionsToParent); $executionResult = new ExecutionResult( null, @@ -219,6 +259,7 @@ public function parentExceptionMappingDataProvider() 'errors' => [ [ 'message' => 'Error with invalid argument exception', + 'category' => 'user', ], ], ], @@ -244,6 +285,7 @@ public function parentExceptionMappingDataProvider() 'errors' => [ [ 'message' => 'Error with invalid argument exception', + 'category' => 'user', ], ], ], @@ -258,6 +300,7 @@ public function parentExceptionMappingDataProvider() 'warnings' => [ [ 'message' => 'Error with invalid argument exception', + 'category' => 'user', ], ], ], @@ -273,6 +316,7 @@ public function parentExceptionMappingDataProvider() 'errors' => [ [ 'message' => 'Error with invalid argument exception', + 'category' => 'user', ], ], ], @@ -288,6 +332,7 @@ public function parentExceptionMappingDataProvider() 'warnings' => [ [ 'message' => 'Error with invalid argument exception', + 'category' => 'user', ], ], ], @@ -303,6 +348,7 @@ public function parentExceptionMappingDataProvider() 'errors' => [ [ 'message' => 'Error with invalid argument exception', + 'category' => 'user', ], ], ], diff --git a/Tests/EventListener/ErrorLoggerListenerTest.php b/Tests/EventListener/ErrorLoggerListenerTest.php new file mode 100644 index 000000000..4dcc76bc9 --- /dev/null +++ b/Tests/EventListener/ErrorLoggerListenerTest.php @@ -0,0 +1,74 @@ +logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); + $this->listener = new ErrorLoggerListener($this->logger); + } + + /** + * @param Error $error + * @param Invocation $loggerExpects + * @param Constraint|string $loggerMethod + * @param array|null $with + * + * @dataProvider fixtures + */ + public function testOnErrorFormatting(Error $error, $loggerExpects, $loggerMethod, array $with = null) + { + $mock = $this->logger->expects($loggerExpects)->method($loggerMethod); + if ($with) { + $mock->with(...$with); + } + + $this->listener->onErrorFormatting(new ErrorFormattingEvent($error, [])); + } + + public function fixtures() + { + try { + throw new \Exception('Ko!'); + } catch (\Exception $exception) { + } + $with = [ + sprintf('[GraphQL] %s: %s[%d] (caught throwable) at %s line %s.', \Exception::class, 'Ko!', 0, __FILE__, $exception->getLine()), + ['throwable' => $exception], + ]; + + return [ + [self::createError('Basic error'), $this->never(), $this->anything()], + [self::createError('Wrapped Base UserError without previous', new \GraphQL\Error\UserError('User error message')), $this->never(), $this->anything()], + [self::createError('Wrapped UserError without previous', new UserError('User error message')), $this->never(), $this->anything()], + [self::createError('Wrapped UserWarning without previous', new UserWarning('User warning message')), $this->never(), $this->anything()], + [self::createError('Wrapped unknown exception', $exception), $this->once(), 'critical', $with], + [self::createError('Wrapped Base UserError with previous', new \GraphQL\Error\UserError('User error message', 0, $exception)), $this->once(), 'error', $with], + [self::createError('Wrapped UserError with previous', new UserError('User error message', 0, $exception)), $this->once(), 'error', $with], + [self::createError('Wrapped UserWarning with previous', new UserWarning('User warning message', 0, $exception)), $this->once(), 'warning', $with], + ]; + } + + private static function createError($message, \Exception $exception = null) + { + return new Error($message, null, null, null, null, $exception); + } +} diff --git a/Tests/Functional/App/Exception/ExampleException.php b/Tests/Functional/App/Exception/ExampleException.php index e2c29e93b..8538506ff 100644 --- a/Tests/Functional/App/Exception/ExampleException.php +++ b/Tests/Functional/App/Exception/ExampleException.php @@ -6,6 +6,6 @@ class ExampleException { public function __invoke() { - throw new \InvalidArgumentException('Invalid argument exception'); + throw new \InvalidArgumentException('Invalid argument exception', 321); } } diff --git a/Tests/Functional/App/config/config.yml b/Tests/Functional/App/config/config.yml index d3d1ad89a..f2e9aaa7c 100644 --- a/Tests/Functional/App/config/config.yml +++ b/Tests/Functional/App/config/config.yml @@ -9,6 +9,8 @@ framework: enabled: false overblog_graphql: + errors_handler: + debug: false definitions: config_validation: true auto_mapping: false diff --git a/Tests/Functional/App/config/debug/config.yml b/Tests/Functional/App/config/debug/config.yml new file mode 100644 index 000000000..dd7932985 --- /dev/null +++ b/Tests/Functional/App/config/debug/config.yml @@ -0,0 +1,7 @@ +imports: + - { resource: ../connection/config.yml } + +overblog_graphql: + definitions: + class_namespace: "Overblog\\GraphQLBundle\\Debug\\__DEFINITIONS__" + show_debug_info: true diff --git a/Tests/Functional/App/config/exception/config.yml b/Tests/Functional/App/config/exception/config.yml index 6a0a5b609..fc1cd0533 100644 --- a/Tests/Functional/App/config/exception/config.yml +++ b/Tests/Functional/App/config/exception/config.yml @@ -3,11 +3,12 @@ imports: - { resource: services.yml } overblog_graphql: - definitions: - class_namespace: "Overblog\\GraphQLBundle\\Exception\\__DEFINITIONS__" + errors_handler: exceptions: errors: - - "InvalidArgumentException" + - 'InvalidArgumentException' + definitions: + class_namespace: 'Overblog\GraphQLBundle\Exception\__DEFINITIONS__' schema: query: Query mutation: ~ @@ -15,4 +16,4 @@ overblog_graphql: types: - type: yaml - dir: "%kernel.root_dir%/config/exception/mapping" + dir: '%kernel.root_dir%/config/exception/mapping' diff --git a/Tests/Functional/App/config/mutation/config.yml b/Tests/Functional/App/config/mutation/config.yml index f098943c9..976d8c041 100644 --- a/Tests/Functional/App/config/mutation/config.yml +++ b/Tests/Functional/App/config/mutation/config.yml @@ -3,9 +3,10 @@ imports: - { resource: services.yml } overblog_graphql: + errors_handler: + internal_error_message: "Mutation has failed." definitions: class_namespace: "Overblog\\GraphQLBundle\\Mutation\\__DEFINITIONS__" - internal_error_message: "Mutation has failled." schema: query: RootMutation mutation: RootMutation diff --git a/Tests/Functional/EventListener/DebugListenerTest.php b/Tests/Functional/EventListener/DebugListenerTest.php new file mode 100644 index 000000000..55a3796f3 --- /dev/null +++ b/Tests/Functional/EventListener/DebugListenerTest.php @@ -0,0 +1,26 @@ + 'connection']); + $response = $this->sendRequest($client, Introspection::getIntrospectionQuery(), true); + $this->assertArrayNotHasKey('extensions', $response); + } + + public function testEnabledDebugInfo() + { + $client = static::createClient(['test_case' => 'debug']); + $response = $this->sendRequest($client, Introspection::getIntrospectionQuery(), true); + $this->assertArrayHasKey('extensions', $response); + $this->assertArrayHasKey('debug', $response['extensions']); + $this->assertArrayHasKey('executionTime', $response['extensions']['debug']); + $this->assertArrayHasKey('memoryUsage', $response['extensions']['debug']); + } +} diff --git a/Tests/Functional/Exception/ExceptionTest.php b/Tests/Functional/Exception/ExceptionTest.php index 97862f2a2..5e60a6dfe 100644 --- a/Tests/Functional/Exception/ExceptionTest.php +++ b/Tests/Functional/Exception/ExceptionTest.php @@ -35,6 +35,7 @@ public function testExceptionIsMappedToAWarning() ], ], 'path' => ['test'], + 'category' => 'user', ], ]; diff --git a/Tests/Functional/Security/AccessTest.php b/Tests/Functional/Security/AccessTest.php index a7bd64d1d..825d03f48 100644 --- a/Tests/Functional/Security/AccessTest.php +++ b/Tests/Functional/Security/AccessTest.php @@ -80,6 +80,7 @@ public function testNotAuthenticatedUserAccessToUserName() 'message' => 'Access denied to this field.', 'locations' => [['line' => 1, 'column' => 16]], 'path' => ['user', 'name'], + 'category' => 'user', ], ], ], @@ -141,6 +142,7 @@ public function testUserForbiddenField() ], ], 'path' => ['user', 'forbidden'], + 'category' => 'user', ], ], ], @@ -212,6 +214,7 @@ public function testMutationAllowedButNoRightsToDisplayPayload() ], ], 'path' => ['simpleMutationWithThunkFields', 'result'], + 'category' => 'user', ], ], ], @@ -237,6 +240,7 @@ public function testMutationNotAllowedUser() ], ], 'path' => ['simpleMutationWithThunkFields'], + 'category' => 'user', ], ], ]; diff --git a/Tests/Functional/Security/QueryComplexityTest.php b/Tests/Functional/Security/QueryComplexityTest.php index 71bba36ae..2f67add50 100644 --- a/Tests/Functional/Security/QueryComplexityTest.php +++ b/Tests/Functional/Security/QueryComplexityTest.php @@ -40,6 +40,7 @@ public function testComplexityReachLimitation() 'errors' => [ [ 'message' => 'Max query complexity should be 10 but got 54.', + 'category' => 'graphql', ], ], ]; @@ -53,6 +54,7 @@ public function testComplexityReachLimitationEnv() 'errors' => [ [ 'message' => 'Max query complexity should be 10 but got 54.', + 'category' => 'graphql', ], ], ]; diff --git a/Tests/Functional/Security/QueryMaxDepthTest.php b/Tests/Functional/Security/QueryMaxDepthTest.php index 71cde4a2d..43ee3eb69 100644 --- a/Tests/Functional/Security/QueryMaxDepthTest.php +++ b/Tests/Functional/Security/QueryMaxDepthTest.php @@ -47,6 +47,7 @@ public function testMaxDepthReachLimitation() 'errors' => [ [ 'message' => 'Max query depth should be 3 but got 6.', + 'category' => 'graphql', ], ], ]; @@ -60,6 +61,7 @@ public function testMaxDepthReachLimitationEnv() 'errors' => [ [ 'message' => 'Max query depth should be 3 but got 6.', + 'category' => 'graphql', ], ], ]; diff --git a/Tests/Functional/TestCase.php b/Tests/Functional/TestCase.php index 4076cec2e..183f95034 100644 --- a/Tests/Functional/TestCase.php +++ b/Tests/Functional/TestCase.php @@ -3,6 +3,7 @@ namespace Overblog\GraphQLBundle\Tests\Functional; use Overblog\GraphQLBundle\Tests\Functional\App\TestKernel; +use Symfony\Bundle\FrameworkBundle\Client; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\HttpFoundation\Request; @@ -59,22 +60,20 @@ protected function tearDown() static::$kernel = null; } - protected static function executeGraphQLRequest($query, $rootValue = [], $throwException = false) + protected static function executeGraphQLRequest($query, $rootValue = []) { $request = new Request(); $request->query->set('query', $query); $req = static::getContainer()->get('overblog_graphql.request_parser')->parse($request); - $executor = static::getContainer()->get('overblog_graphql.request_executor'); - $executor->setThrowException($throwException); - $res = $executor->execute($req, $rootValue); + $res = static::getContainer()->get('overblog_graphql.request_executor')->execute(null, $req, $rootValue); return $res->toArray(); } protected static function assertGraphQL($query, array $expectedData = null, array $expectedErrors = null, $rootValue = []) { - $result = static::executeGraphQLRequest($query, $rootValue, true); + $result = static::executeGraphQLRequest($query, $rootValue/*, true*/); $expected = []; @@ -119,12 +118,18 @@ protected static function createClientAuthenticated($username, $testCase, $passw protected static function assertResponse($query, array $expected, $username, $testCase, $password = self::DEFAULT_PASSWORD) { $client = self::createClientAuthenticated($username, $testCase, $password); - $client->request('GET', '/', ['query' => $query]); - - $result = $client->getResponse()->getContent(); + $result = self::sendRequest($client, $query); static::assertEquals($expected, json_decode($result, true), $result); return $client; } + + protected static function sendRequest(Client $client, $query, $isDecoded = false) + { + $client->request('GET', '/', ['query' => $query]); + $result = $client->getResponse()->getContent(); + + return $isDecoded ? json_decode($result, true) : $result; + } } diff --git a/Tests/Request/ExecutorTest.php b/Tests/Request/ExecutorTest.php index fff81684d..42d719be1 100644 --- a/Tests/Request/ExecutorTest.php +++ b/Tests/Request/ExecutorTest.php @@ -3,23 +3,31 @@ namespace Overblog\GraphQLBundle\Tests\Request; use GraphQL\Executor\Promise\Adapter\ReactPromiseAdapter; +use GraphQL\Executor\Promise\PromiseAdapter; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; use GraphQL\Type\Schema; use Overblog\GraphQLBundle\Executor\Executor; use Overblog\GraphQLBundle\Request\Executor as RequestExecutor; use PHPUnit\Framework\TestCase; +use Symfony\Component\EventDispatcher\EventDispatcher; class ExecutorTest extends TestCase { /** @var RequestExecutor */ private $executor; + /** @var EventDispatcher|\PHPUnit_Framework_MockObject_MockObject */ + private $dispatcher; + private $request = ['query' => 'query debug{ myField }', 'variables' => [], 'operationName' => null]; public function setUp() { - $this->executor = new RequestExecutor(new Executor()); + $this->dispatcher = $this->getMockBuilder(EventDispatcher::class)->setMethods(['dispatch'])->getMock(); + $this->dispatcher->expects($this->any())->method('dispatch')->willReturnArgument(1); + + $this->executor = $this->createRequestExecutor(); $queryType = new ObjectType([ 'name' => 'Query', 'fields' => [ @@ -41,7 +49,7 @@ public function setUp() public function testInvalidExecutorReturnNotObject() { $this->executor->setExecutor($this->createExecutorExecuteMock(false)); - $this->executor->execute($this->request); + $this->executor->execute(null, $this->request); } /** @@ -51,7 +59,7 @@ public function testInvalidExecutorReturnNotObject() public function testInvalidExecutorReturnInvalidObject() { $this->executor->setExecutor($this->createExecutorExecuteMock(new \stdClass())); - $this->executor->execute($this->request); + $this->executor->execute(null, $this->request); } /** @@ -61,21 +69,7 @@ public function testInvalidExecutorReturnInvalidObject() public function testInvalidExecutorAdapterPromise() { $this->executor->setPromiseAdapter(new ReactPromiseAdapter()); - $this->executor->execute($this->request); - } - - public function testDisabledDebugInfo() - { - $this->assertArrayNotHasKey('debug', $this->executor->disabledDebugInfo()->execute($this->request)->extensions); - } - - public function testEnabledDebugInfo() - { - $result = $this->executor->enabledDebugInfo()->execute($this->request); - - $this->assertArrayHasKey('debug', $result->extensions); - $this->assertArrayHasKey('executionTime', $result->extensions['debug']); - $this->assertArrayHasKey('memoryUsage', $result->extensions['debug']); + $this->executor->execute(null, $this->request); } /** @@ -84,7 +78,7 @@ public function testEnabledDebugInfo() */ public function testGetSchemaNoSchemaFound() { - (new RequestExecutor(new Executor()))->getSchema('fake'); + $this->createRequestExecutor()->getSchema('fake'); } private function createExecutorExecuteMock($returnValue) @@ -97,4 +91,14 @@ private function createExecutorExecuteMock($returnValue) return $mock; } + + private function createRequestExecutor() + { + /** @var PromiseAdapter|\PHPUnit_Framework_MockObject_MockObject $promiseAdapter */ + $promiseAdapter = $this->getMockBuilder(PromiseAdapter::class) + ->setMethods(array_merge(['wait'], get_class_methods(PromiseAdapter::class))) + ->getMock(); + + return new RequestExecutor(new Executor(), $this->dispatcher, $promiseAdapter); + } } diff --git a/UPGRADE-0.11.md b/UPGRADE-0.11.md index fbeccf796..9103d5a8c 100644 --- a/UPGRADE-0.11.md +++ b/UPGRADE-0.11.md @@ -4,6 +4,8 @@ UPGRADE FROM 0.10 to 0.11 # Table of Contents - [GraphiQL](#graphiql) +- [Errors Handler](#errors-handler) +- [Promise adapter interface](#promise-adapter-interface) ### GraphiQL @@ -40,3 +42,45 @@ UPGRADE FROM 0.10 to 0.11 ``` bin/console graphql:dump-schema --modern ``` + +### Errors Handler + + * Made errors handler more customizable + + Upgrading: + - User + - Delete configuration to override base user exception classes. + ```diff + overblog_graphql: + definitions: + exceptions: + - types: + - warnings: ~ + - errors: ~ + ``` + - Move `internal_error_message`, `map_exceptions_to_parent` and `exceptions` configurations + from `definitions` to new dedicated `error_handler` section. + ```diff + overblog_graphql: + definitions: + - internal_error_message: ~ + - map_exceptions_to_parent: ~ + - exceptions: ~ + + errors_handler: + + internal_error_message: ~ + + map_exceptions_to_parent: ~ + + exceptions: ~ + ``` + + +### Promise adapter interface + + * Changed the promise adapter interface (`Overblog\GraphQLBundle\Executor\ExecutorInterface`) + as the promiseAdapter is not nullable in the bundle context. + + Upgrading: + - `setPromiseAdapter` method no more nullable. + ```diff + - public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null); + + public function setPromiseAdapter(PromiseAdapter $promiseAdapter); + ``` diff --git a/composer.json b/composer.json index ac8c552d5..7fb4d336d 100644 --- a/composer.json +++ b/composer.json @@ -35,6 +35,7 @@ "symfony/cache": "^3.1 || ^4.0", "symfony/config": "^3.1 || ^4.0", "symfony/dependency-injection": "^3.1 || ^4.0", + "symfony/event-dispatcher": "^3.1 || ^4.0", "symfony/expression-language": "^3.1 || ^4.0", "symfony/framework-bundle": "^3.1 || ^4.0", "symfony/options-resolver": "^3.1 || ^4.0", @@ -48,6 +49,7 @@ }, "require-dev": { "phpunit/phpunit": "^5.5 || ^6.0", + "psr/log": "^1.0", "react/promise": "^2.5", "sensio/framework-extra-bundle": "^3.0", "symfony/asset": "^3.1 || ^4.0",