Skip to content

Commit e9ed974

Browse files
committed
Serialize TypeInfo correctly but also hide Throwable errors by default (fixes #37)
1 parent 5ff7417 commit e9ed974

File tree

4 files changed

+71
-27
lines changed

4 files changed

+71
-27
lines changed

.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ build/
55
*.iws
66
.idea/
77
target/
8+
/out/

src/main/java/graphql/servlet/DefaultGraphQLErrorHandler.java

+8-2
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,13 @@ public List<GraphQLError> processErrors(List<GraphQLError> errors) {
3939

4040
errors.stream()
4141
.filter(error -> !isClientError(error))
42-
.forEach(error -> log.error("Error executing query ({}): {}", error.getClass().getSimpleName(), error.getMessage()));
42+
.forEach(error -> {
43+
if(error instanceof Throwable) {
44+
log.error("Error executing query!", (Throwable) error);
45+
} else {
46+
log.error("Error executing query ({}): {}", error.getClass().getSimpleName(), error.getMessage());
47+
}
48+
});
4349
}
4450

4551
return clientErrors;
@@ -52,6 +58,6 @@ protected List<GraphQLError> filterGraphQLErrors(List<GraphQLError> errors) {
5258
}
5359

5460
protected boolean isClientError(GraphQLError error) {
55-
return !(error instanceof ExceptionWhileDataFetching);
61+
return !(error instanceof ExceptionWhileDataFetching || error instanceof Throwable);
5662
}
5763
}

src/main/java/graphql/servlet/GraphQLServlet.java

+17-1
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,23 @@
1414
*/
1515
package graphql.servlet;
1616

17+
import com.fasterxml.jackson.core.JsonGenerator;
1718
import com.fasterxml.jackson.core.JsonParser;
19+
import com.fasterxml.jackson.core.JsonProcessingException;
1820
import com.fasterxml.jackson.core.type.TypeReference;
1921
import com.fasterxml.jackson.databind.DeserializationContext;
2022
import com.fasterxml.jackson.databind.JsonDeserializer;
23+
import com.fasterxml.jackson.databind.JsonSerializer;
2124
import com.fasterxml.jackson.databind.ObjectMapper;
2225
import com.fasterxml.jackson.databind.RuntimeJsonMappingException;
26+
import com.fasterxml.jackson.databind.SerializationFeature;
27+
import com.fasterxml.jackson.databind.SerializerProvider;
2328
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
29+
import com.fasterxml.jackson.databind.module.SimpleModule;
2430
import graphql.ExecutionResult;
2531
import graphql.GraphQL;
2632
import graphql.GraphQLError;
33+
import graphql.execution.TypeInfo;
2734
import graphql.execution.instrumentation.Instrumentation;
2835
import graphql.introspection.IntrospectionQuery;
2936
import graphql.schema.GraphQLFieldDefinition;
@@ -68,7 +75,16 @@ public abstract class GraphQLServlet extends HttpServlet implements Servlet, Gra
6875
public static final int STATUS_OK = 200;
6976
public static final int STATUS_BAD_REQUEST = 400;
7077

71-
private static final ObjectMapper mapper = new ObjectMapper();
78+
private static final ObjectMapper mapper = new ObjectMapper().disable(SerializationFeature.FAIL_ON_EMPTY_BEANS).registerModule(new SimpleModule().addSerializer(TypeInfo.class, new JsonSerializer<TypeInfo>() {
79+
@Override
80+
public void serialize(TypeInfo value, JsonGenerator gen, SerializerProvider serializers) throws IOException {
81+
gen.writeStartObject();
82+
serializers.defaultSerializeField("type", value.type(), gen);
83+
serializers.defaultSerializeField("parentTypeInfo", value.parentTypeInfo(), gen);
84+
serializers.defaultSerializeField("typeIsNonNull", value.typeIsNonNull(), gen);
85+
gen.writeEndObject();
86+
}
87+
}));
7288

7389
protected abstract GraphQLSchemaProvider getSchemaProvider();
7490
protected abstract GraphQLContext createContext(Optional<HttpServletRequest> request, Optional<HttpServletResponse> response);

src/test/groovy/graphql/servlet/GraphQLServletSpec.groovy

+45-24
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,18 @@
1515
package graphql.servlet
1616

1717
import com.fasterxml.jackson.databind.ObjectMapper
18-
import graphql.GraphQLError
1918
import graphql.Scalars
20-
import graphql.execution.SimpleExecutionStrategy
19+
import graphql.execution.TypeInfo
2120
import graphql.schema.DataFetcher
21+
import graphql.schema.GraphQLFieldDefinition
22+
import graphql.schema.GraphQLNonNull
2223
import graphql.schema.GraphQLObjectType
2324
import graphql.schema.GraphQLSchema
2425
import org.springframework.mock.web.MockHttpServletRequest
2526
import org.springframework.mock.web.MockHttpServletResponse
2627
import spock.lang.Shared
2728
import spock.lang.Specification
2829

29-
import javax.servlet.http.HttpServletRequest
30-
import javax.servlet.http.HttpServletResponse
31-
3230
/**
3331
* @author Andrew Potter
3432
*/
@@ -55,29 +53,34 @@ class GraphQLServletSpec extends Specification {
5553
def createServlet(DataFetcher queryDataFetcher = { env -> env.arguments.arg }, DataFetcher mutationDataFetcher = { env -> env.arguments.arg }) {
5654
GraphQLObjectType query = GraphQLObjectType.newObject()
5755
.name("Query")
58-
.field { field ->
59-
field.name("echo")
60-
field.type(Scalars.GraphQLString)
61-
field.argument { argument ->
62-
argument.name("arg")
63-
argument.type(Scalars.GraphQLString)
56+
.field { GraphQLFieldDefinition.Builder field ->
57+
field.name("echo")
58+
field.type(Scalars.GraphQLString)
59+
field.argument { argument ->
60+
argument.name("arg")
61+
argument.type(Scalars.GraphQLString)
62+
}
63+
field.dataFetcher(queryDataFetcher)
6464
}
65-
field.dataFetcher(queryDataFetcher)
66-
}
67-
.build()
65+
.field { GraphQLFieldDefinition.Builder field ->
66+
field.name("returnsNullIncorrectly")
67+
field.type(new GraphQLNonNull(Scalars.GraphQLString))
68+
field.dataFetcher({env -> null})
69+
}
70+
.build()
6871

6972
GraphQLObjectType mutation = GraphQLObjectType.newObject()
7073
.name("Mutation")
7174
.field { field ->
72-
field.name("echo")
73-
field.type(Scalars.GraphQLString)
74-
field.argument { argument ->
75-
argument.name("arg")
76-
argument.type(Scalars.GraphQLString)
75+
field.name("echo")
76+
field.type(Scalars.GraphQLString)
77+
field.argument { argument ->
78+
argument.name("arg")
79+
argument.type(Scalars.GraphQLString)
80+
}
81+
field.dataFetcher(mutationDataFetcher)
7782
}
78-
field.dataFetcher(mutationDataFetcher)
79-
}
80-
.build()
83+
.build()
8184

8285
return new SimpleGraphQLServlet(new GraphQLSchema(query, mutation, [query, mutation].toSet()))
8386
}
@@ -416,7 +419,25 @@ class GraphQLServletSpec extends Specification {
416419
resp.errors != null
417420
}
418421

419-
private byte[] createContent(String data) {
420-
data.split('\\n').collect { it.replaceAll('^\\s+', '') }.join('\n').getBytes()
422+
def "NonNullableFieldWasNullException is masked by default"() {
423+
setup:
424+
request.addParameter('query', 'query { returnsNullIncorrectly }')
425+
426+
when:
427+
servlet.doGet(request, response)
428+
429+
then:
430+
response.getStatus() == STATUS_OK
431+
response.getContentType() == CONTENT_TYPE_JSON_UTF8
432+
def resp = getResponseContent()
433+
resp.containsKey("data")
434+
resp.data == null
435+
resp.errors != null
436+
resp.errors.first().message.contains('Internal Server Error')
437+
}
438+
439+
def "typeInfo is serialized correctly"() {
440+
expect:
441+
GraphQLServlet.mapper.writeValueAsString(TypeInfo.newTypeInfo().type(new GraphQLNonNull(Scalars.GraphQLString)).build()) != "{}"
421442
}
422443
}

0 commit comments

Comments
 (0)