enum coercing throws correct exception

andimarek · bbakerman · commit deedb889e3f0 · 2017-06-15T09:40:25.000+10:00
enum serialization exceptions are handled correctly
diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java
@@ -145,7 +145,7 @@ protected ExecutionResult completeValue(ExecutionContext executionContext, Execu
         } else if (fieldType instanceof GraphQLScalarType) {
             return completeValueForScalar((GraphQLScalarType) fieldType, parameters, result, executionContext);
         } else if (fieldType instanceof GraphQLEnumType) {
-            return completeValueForEnum((GraphQLEnumType) fieldType, parameters, result);
+            return completeValueForEnum((GraphQLEnumType) fieldType, parameters, result, executionContext);
         }
 
 
@@ -221,8 +221,14 @@ protected GraphQLObjectType resolveTypeForUnion(TypeResolutionParameters params)
         return result;
     }
 
-    protected ExecutionResult completeValueForEnum(GraphQLEnumType enumType, ExecutionStrategyParameters parameters, Object result) {
-        Object serialized = enumType.getCoercing().serialize(result);
+    protected ExecutionResult completeValueForEnum(GraphQLEnumType enumType, ExecutionStrategyParameters parameters, Object result, ExecutionContext context) {
+        Object serialized;
+        try {
+            serialized = enumType.getCoercing().serialize(result);
+        } catch (CoercingSerializeException e) {
+            context.addError(new SerializationError(e));
+            serialized = null;
+        }
         serialized = parameters.nonNullFieldValidator().validate(serialized);
         return new ExecutionResultImpl(serialized, null);
     }
diff --git a/src/main/java/graphql/schema/GraphQLEnumType.java b/src/main/java/graphql/schema/GraphQLEnumType.java
@@ -2,7 +2,6 @@
 
 
 import graphql.AssertException;
-import graphql.GraphQLException;
 import graphql.Internal;
 import graphql.PublicApi;
 import graphql.language.EnumTypeDefinition;
@@ -80,14 +79,14 @@ private void buildMap(List<GraphQLEnumValueDefinition> values) {
     private Object getValueByName(Object value) {
         GraphQLEnumValueDefinition enumValueDefinition = valueDefinitionMap.get(value.toString());
         if (enumValueDefinition != null) return enumValueDefinition.getValue();
-        throw new GraphQLException("Invalid input for Enum '" + name + "'. No value found for name " + value.toString());
+        throw new CoercingParseValueException("Invalid input for Enum '" + name + "'. No value found for name '" + value.toString() + "'");
     }
 
     private Object getNameByValue(Object value) {
         for (GraphQLEnumValueDefinition valueDefinition : valueDefinitionMap.values()) {
             if (value.equals(valueDefinition.getValue())) return valueDefinition.getName();
         }
-        throw new GraphQLException("Invalid input for Enum '" + name + "'. Unknown value " + value);
+        throw new CoercingSerializeException("Invalid input for Enum '" + name + "'. Unknown value '" + value + "'");
     }
 
 
diff --git a/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy b/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy
@@ -5,11 +5,13 @@ import graphql.Scalars
 import graphql.SerializationError
 import graphql.language.Field
 import graphql.schema.Coercing
+import graphql.schema.GraphQLEnumType
 import graphql.schema.GraphQLList
 import graphql.schema.GraphQLScalarType
 import spock.lang.Specification
 
 import static ExecutionStrategyParameters.newParameters
+import static graphql.schema.GraphQLEnumType.newEnum
 import static graphql.schema.GraphQLNonNull.nonNull
 
 @SuppressWarnings("GroovyPointlessBoolean")
@@ -70,7 +72,6 @@ class ExecutionStrategyTest extends Specification {
         executionResult.data == ["test"]
     }
 
-    // this is wrong: we should return null with an error
     def "completing value with serializing throwing exception"() {
         given:
         ExecutionContext executionContext = buildContext()
@@ -96,6 +97,31 @@ class ExecutionStrategyTest extends Specification {
 
     }
 
+    def "completing enum with serializing throwing exception"() {
+        given:
+        ExecutionContext executionContext = buildContext()
+        GraphQLEnumType enumType = newEnum().name("Enum").value("value").build()
+        def typeInfo = TypeInfo.newTypeInfo().type(enumType).build()
+        NonNullableFieldValidator nullableFieldValidator = new NonNullableFieldValidator(executionContext, typeInfo)
+        String result = "not a enum number"
+
+        def parameters = newParameters()
+                .typeInfo(typeInfo)
+                .source(result)
+                .nonNullFieldValidator(nullableFieldValidator)
+                .fields(["dummy": []])
+                .build()
+
+        when:
+        def executionResult = executionStrategy.completeValue(executionContext, parameters, [new Field()])
+
+        then:
+        executionResult.data == null
+        executionContext.errors.size() == 1
+        executionContext.errors[0] instanceof SerializationError
+
+    }
+
     def "completing a scalar null value for a non null type throws an exception"() {
 
         GraphQLScalarType NullProducingScalar = new GraphQLScalarType("Custom", "It Can Produce Nulls", new Coercing<Double, Double>() {
diff --git a/src/test/groovy/graphql/schema/GraphQLEnumTypeTest.groovy b/src/test/groovy/graphql/schema/GraphQLEnumTypeTest.groovy
@@ -1,7 +1,6 @@
 package graphql.schema
 
 import graphql.AssertException
-import graphql.GraphQLException
 import graphql.language.EnumValue
 import graphql.language.StringValue
 import spock.lang.Specification
@@ -23,7 +22,7 @@ class GraphQLEnumTypeTest extends Specification {
         enumType.getCoercing().parseValue("UNKNOWN")
 
         then:
-        thrown(GraphQLException)
+        thrown(CoercingParseValueException)
     }
 
 
@@ -41,7 +40,7 @@ class GraphQLEnumTypeTest extends Specification {
         when:
         enumType.getCoercing().serialize(12)
         then:
-        thrown(GraphQLException)
+        thrown(CoercingSerializeException)
     }
 
 

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,6 @@`
`2`	`2`
`3`	`3`
`4`	`4`	`import graphql.AssertException;`
`5`		`-import graphql.GraphQLException;`
`6`	`5`	`import graphql.Internal;`
`7`	`6`	`import graphql.PublicApi;`
`8`	`7`	`import graphql.language.EnumTypeDefinition;`
`@@ -80,14 +79,14 @@ private void buildMap(List<GraphQLEnumValueDefinition> values) {`
`80`	`79`	`private Object getValueByName(Object value) {`
`81`	`80`	`GraphQLEnumValueDefinition enumValueDefinition = valueDefinitionMap.get(value.toString());`
`82`	`81`	`if (enumValueDefinition != null) return enumValueDefinition.getValue();`
`83`		`- throw new GraphQLException("Invalid input for Enum '" + name + "'. No value found for name " + value.toString());`
	`82`	`+ throw new CoercingParseValueException("Invalid input for Enum '" + name + "'. No value found for name '" + value.toString() + "'");`
`84`	`83`	`}`
`85`	`84`
`86`	`85`	`private Object getNameByValue(Object value) {`
`87`	`86`	`for (GraphQLEnumValueDefinition valueDefinition : valueDefinitionMap.values()) {`
`88`	`87`	`if (value.equals(valueDefinition.getValue())) return valueDefinition.getName();`
`89`	`88`	`}`
`90`		`- throw new GraphQLException("Invalid input for Enum '" + name + "'. Unknown value " + value);`
	`89`	`+ throw new CoercingSerializeException("Invalid input for Enum '" + name + "'. Unknown value '" + value + "'");`
`91`	`90`	`}`
`92`	`91`
`93`	`92`