fix: RecursiveTypeMapper returns exception which can't be handled by StandardServer (#646)

* fix: RecursiveTypeMapper returns exception which can't be handled by StandardServer

RecursiveTypeMapper throws CannotMapTypeException when fails to find class by name. This exception goes through the GraphQl executor instead of catching and wrapping into ExecutionResult. Non wrapped exception does not trigger HttpCodeDecider.
Schema test trust more to input than configuration which is not optimal, great possibility to improve configuration validation.

Fixes: #336

* feat: new special Exception class when request type unknown

Fixes: #336

* fix: move asserts

Fixes: #336

* polish changes

Fixes: #336

Author: fogrye

src/Mappers/RecursiveTypeMapper.php

@@ -521,6 +521,6 @@ public function mapNameToType(string $typeName): Type&NamedType
             return $this->mapClassToInterfaceOrType($className, null);
         }
 
-        throw CannotMapTypeException::createForName($typeName);
+        throw TypeNotFoundException::createError($typeName);
     }
 }

src/Mappers/TypeNotFoundException.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TheCodingMachine\GraphQLite\Mappers;
+
+use GraphQL\Error\Error;
+use GraphQL\Utils\Utils;
+
+/**
+ * TypeNotFoundException thrown when RecursiveTypeMapper fails to find a type.
+ *
+ * While CannotMapTypeException is more about error with configuration this exception can occur when request
+ * contains a type which is unknown to the server.
+ * Should not be handled by the user, webonyx/graphql-php will transform this under 'errors' key in the response.
+ */
+class TypeNotFoundException extends Error
+{
+    public static function createError(string $typeName): Error
+    {
+        return static::createLocatedError('Unknown type ' . Utils::printSafe($typeName));
+    }
+}

tests/Mappers/RecursiveTypeMapperTest.php

@@ -93,7 +93,7 @@ public function testMapNameToType2(): void
         $this->assertSame('ClassA', $recursiveMapper->mapNameToType('ClassA')->name);
         $this->assertSame('ClassAInterface', $recursiveMapper->mapNameToType('ClassAInterface')->name);
 
-        $this->expectException(CannotMapTypeException::class);
+        $this->expectException(TypeNotFoundException::class);
         $recursiveMapper->mapNameToType('NotExists');
     }
 
@@ -233,6 +233,8 @@ public function testDuplicateDetection(): void
 
     /**
      * Tests that the RecursiveTypeMapper behaves correctly if there are no types to map.
+     *
+     * @see \GraphQL\Server\Helper::promiseToExecuteOperation()
      */
     public function testMapNoTypes(): void
     {
@@ -244,7 +246,7 @@ public function testMapNoTypes(): void
             $this->getAnnotationReader()
         );
 
-        $this->expectException(CannotMapTypeException::class);
+        $this->expectException(TypeNotFoundException::class);
         $recursiveTypeMapper->mapNameToType('Foo');
     }
 

tests/SchemaFactoryTest.php

@@ -1,8 +1,12 @@
 <?php
 
+declare(strict_types=1);
+
 namespace TheCodingMachine\GraphQLite;
 
+use Doctrine\Common\Annotations\AnnotationReader;
 use GraphQL\Error\DebugFlag;
+use GraphQL\Executor\ExecutionResult;
 use GraphQL\GraphQL;
 use GraphQL\Type\SchemaConfig;
 use Mouf\Composer\ClassNameMapper;
@@ -20,22 +24,19 @@
 use TheCodingMachine\GraphQLite\Fixtures\Integration\Types\ContactOtherType;
 use TheCodingMachine\GraphQLite\Fixtures\Integration\Types\ContactType;
 use TheCodingMachine\GraphQLite\Fixtures\Integration\Types\ExtendedContactType;
-use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException;
+use TheCodingMachine\GraphQLite\Fixtures\TestSelfType;
 use TheCodingMachine\GraphQLite\Mappers\CompositeTypeMapper;
 use TheCodingMachine\GraphQLite\Mappers\DuplicateMappingException;
+use TheCodingMachine\GraphQLite\Mappers\Parameters\ParameterMiddlewarePipe;
 use TheCodingMachine\GraphQLite\Mappers\Root\VoidRootTypeMapperFactory;
 use TheCodingMachine\GraphQLite\Mappers\StaticClassListTypeMapperFactory;
 use TheCodingMachine\GraphQLite\Middlewares\FieldMiddlewarePipe;
-use TheCodingMachine\GraphQLite\Mappers\Parameters\ParameterMiddlewarePipe;
 use TheCodingMachine\GraphQLite\Middlewares\InputFieldMiddlewarePipe;
 use TheCodingMachine\GraphQLite\Security\VoidAuthenticationService;
 use TheCodingMachine\GraphQLite\Security\VoidAuthorizationService;
-use TheCodingMachine\GraphQLite\Fixtures\TestSelfType;
-
 
 class SchemaFactoryTest extends TestCase
 {
-
     public function testCreateSchema(): void
     {
         $container = new BasicAutoWiringContainer(new EmptyContainer());
@@ -65,7 +66,7 @@ public function testSetters(): void
 
         $factory->addControllerNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration\\Controllers');
         $factory->addTypeNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration');
-        $factory->setDoctrineAnnotationReader(new \Doctrine\Common\Annotations\AnnotationReader())
+        $factory->setDoctrineAnnotationReader(new AnnotationReader())
                 ->setAuthenticationService(new VoidAuthenticationService())
                 ->setAuthorizationService(new VoidAuthorizationService())
                 ->setNamingStrategy(new NamingStrategy())
@@ -89,8 +90,8 @@ public function testClassNameMapperInjectionWithValidMapper(): void
         $factory = new SchemaFactory(
             new Psr16Cache(new ArrayAdapter()),
             new BasicAutoWiringContainer(
-                new EmptyContainer()
-            )
+                new EmptyContainer(),
+            ),
         );
         $factory->setAuthenticationService(new VoidAuthenticationService())
                 ->setAuthorizationService(new VoidAuthorizationService())
@@ -132,17 +133,16 @@ public function testClassNameMapperInjectionWithInvalidMapper(): void
         $factory = new SchemaFactory(
             new Psr16Cache(new ArrayAdapter()),
             new BasicAutoWiringContainer(
-                new EmptyContainer()
-            )
+                new EmptyContainer(),
+            ),
         );
         $factory->setAuthenticationService(new VoidAuthenticationService())
                 ->setAuthorizationService(new VoidAuthorizationService())
                 ->setClassNameMapper(new ClassNameMapper())
                 ->addControllerNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration\\Controllers')
                 ->addTypeNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration');
 
-        $this->expectException(CannotMapTypeException::class);
-        $this->doTestSchema($factory->createSchema());
+        $this->doTestSchemaWithError($factory->createSchema());
     }
 
     public function testException(): void
@@ -168,9 +168,8 @@ public function testException2(): void
         $factory->createSchema();
     }
 
-    private function doTestSchema(Schema $schema): void
+    private function execTestQuery(Schema $schema): ExecutionResult
     {
-
         $schema->assertValid();
 
         $queryString = '
@@ -185,34 +184,50 @@ private function doTestSchema(Schema $schema): void
         }
         ';
 
-        $result = GraphQL::executeQuery(
+        return GraphQL::executeQuery(
             $schema,
-            $queryString
+            $queryString,
         );
+    }
+
+    private function doTestSchemaWithError(Schema $schema): void
+    {
+        $result = $this->execTestQuery($schema);
+        $resultArr = $result->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS);
+        $this->assertArrayHasKey('errors', $resultArr);
+        $this->assertArrayNotHasKey('data', $resultArr);
+        $this->assertCount(1, $resultArr);
+        $this->assertSame('Unknown type "User"', $resultArr['errors'][0]['message']);
+    }
 
+    private function doTestSchema(Schema $schema): void
+    {
+        $result = $this->execTestQuery($schema);
+        $resultArr = $result->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS);
+        $this->assertArrayHasKey('data', $resultArr);
         $this->assertSame([
             'contacts' => [
                 [
                     'name' => 'Joe',
-                    'uppercaseName' => 'JOE'
+                    'uppercaseName' => 'JOE',
                 ],
                 [
                     'name' => 'Bill',
                     'uppercaseName' => 'BILL',
-                    'email' => '[email protected]'
-                ]
+                    'email' => '[email protected]',
+                ],
 
-            ]
-        ], $result->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS)['data']);
+            ],
+        ], $resultArr['data']);
     }
 
     public function testDuplicateQueryException(): void
     {
         $factory = new SchemaFactory(
             new Psr16Cache(new ArrayAdapter()),
             new BasicAutoWiringContainer(
-                new EmptyContainer()
-            )
+                new EmptyContainer(),
+            ),
         );
         $factory->setAuthenticationService(new VoidAuthenticationService())
                 ->setAuthorizationService(new VoidAuthorizationService())
@@ -229,7 +244,7 @@ public function testDuplicateQueryException(): void
         ';
         GraphQL::executeQuery(
             $schema,
-            $queryString
+            $queryString,
         );
     }
 
@@ -238,8 +253,8 @@ public function testDuplicateQueryInTwoControllersException(): void
         $factory = new SchemaFactory(
             new Psr16Cache(new ArrayAdapter()),
             new BasicAutoWiringContainer(
-                new EmptyContainer()
-            )
+                new EmptyContainer(),
+            ),
         );
         $factory->setAuthenticationService(new VoidAuthenticationService())
             ->setAuthorizationService(new VoidAuthorizationService())
@@ -256,7 +271,7 @@ public function testDuplicateQueryInTwoControllersException(): void
         ';
         GraphQL::executeQuery(
             $schema,
-            $queryString
+            $queryString,
         );
     }
 }