Extract error handling logic into interface to make it extensible (fixes #30, fixes #32)

Author: apottere

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

@@ -0,0 +1,57 @@
+/**
+ * Copyright 2016 Yurii Rashkovskii
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ */
+package graphql.servlet;
+
+import graphql.ExceptionWhileDataFetching;
+import graphql.GraphQLError;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * @author Andrew Potter
+ */
+public class DefaultGraphQLErrorHandler implements GraphQLErrorHandler {
+
+    public static final Logger log = LoggerFactory.getLogger(DefaultGraphQLErrorHandler.class);
+
+    @Override
+    public List<GraphQLError> processErrors(List<GraphQLError> errors) {
+        final List<GraphQLError> clientErrors = filterGraphQLErrors(errors);
+        if (clientErrors.size() < errors.size()) {
+
+            // Some errors were filtered out to hide implementation - put a generic error in place.
+            clientErrors.add(new GenericGraphQLError("Internal Server Error(s) while executing query"));
+
+            errors.stream()
+                .filter(error -> !isClientError(error))
+                .forEach(error -> log.error("Error executing query ({}): {}", error.getClass().getSimpleName(), error.getMessage()));
+        }
+
+        return clientErrors;
+    }
+
+    protected List<GraphQLError> filterGraphQLErrors(List<GraphQLError> errors) {
+        return errors.stream()
+            .filter(this::isClientError)
+            .collect(Collectors.toList());
+    }
+
+    protected boolean isClientError(GraphQLError error) {
+        return !(error instanceof ExceptionWhileDataFetching);
+    }
+}

src/main/java/graphql/servlet/GraphQLErrorHandler.java

@@ -0,0 +1,30 @@
+/**
+ * Copyright 2016 Yurii Rashkovskii
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ */
+package graphql.servlet;
+
+import graphql.GraphQLError;
+
+import java.util.List;
+
+/**
+ * @author Andrew Potter
+ */
+public interface GraphQLErrorHandler {
+    default boolean errorsPresent(List<GraphQLError> errors) {
+        return errors != null && !errors.isEmpty();
+    }
+
+    List<GraphQLError> processErrors(List<GraphQLError> errors);
+}

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

@@ -78,6 +78,7 @@ public abstract class GraphQLServlet extends HttpServlet implements Servlet, Gra
     protected abstract ExecutionStrategyProvider getExecutionStrategyProvider();
     protected abstract Instrumentation getInstrumentation();
     protected abstract Map<String, Object> transformVariables(GraphQLSchema schema, String query, Map<String, Object> variables);
+    protected abstract GraphQLErrorHandler getGraphQLErrorHandler();
 
     private final List<GraphQLServletListener> listeners;
     private final ServletFileUpload fileUpload;
@@ -286,7 +287,7 @@ private void query(String query, String operationName, Map<String, Object> varia
             resp.setStatus(STATUS_OK);
             resp.getWriter().write(response);
 
-            if(errorsPresent(errors)) {
+            if(getGraphQLErrorHandler().errorsPresent(errors)) {
                 runCallbacks(operationCallbacks, c -> c.onError(context, operationName, query, variables, data, errors));
             } else {
                 runCallbacks(operationCallbacks, c -> c.onSuccess(context, operationName, query, variables, data));
@@ -301,38 +302,13 @@ private Map<String, Object> createResultFromDataAndErrors(Object data, List<Grap
         final Map<String, Object> result = new HashMap<>();
         result.put("data", data);
 
-        if (errorsPresent(errors)) {
-            final List<GraphQLError> clientErrors = filterGraphQLErrors(errors);
-            if (clientErrors.size() < errors.size()) {
-                // Some errors were filtered out to hide implementation - put a generic error in place.
-                clientErrors.add(new GenericGraphQLError("Internal Server Error(s) while executing query"));
-
-                errors.stream()
-                    .filter(error -> !isClientError(error))
-                    .forEach(error -> {
-                        log.error("Error executing query ({}): {}", error.getClass().getSimpleName(), error.getMessage());
-                    });
-            }
-            result.put("errors", clientErrors);
+        if (getGraphQLErrorHandler().errorsPresent(errors)) {
+            result.put("errors", getGraphQLErrorHandler().processErrors(errors));
         }
 
         return result;
     }
 
-    private boolean errorsPresent(List<GraphQLError> errors) {
-        return errors != null && !errors.isEmpty();
-    }
-
-    protected List<GraphQLError> filterGraphQLErrors(List<GraphQLError> errors) {
-        return errors.stream()
-            .filter(this::isClientError)
-            .collect(Collectors.toList());
-    }
-
-    protected boolean isClientError(GraphQLError error) {
-        return error instanceof InvalidSyntaxError || error instanceof ValidationError;
-    }
-
     private <R> List<R> runListeners(Function<? super GraphQLServletListener, R> action) {
         if (listeners == null) {
             return Collections.emptyList();

src/main/java/graphql/servlet/OsgiGraphQLServlet.java

@@ -48,6 +48,7 @@ public class OsgiGraphQLServlet extends GraphQLServlet {
     private GraphQLContextBuilder contextBuilder = new DefaultGraphQLContextBuilder();
     private ExecutionStrategyProvider executionStrategyProvider = new EnhancedExecutionStrategyProvider();
     private InstrumentationProvider instrumentationProvider = new NoOpInstrumentationProvider();
+    private GraphQLErrorHandler errorHandler = new DefaultGraphQLErrorHandler();
 
     private GraphQLSchemaProvider schemaProvider;
 
@@ -174,6 +175,14 @@ public void unsetInstrumentationProvider(InstrumentationProvider provider) {
         instrumentationProvider = new NoOpInstrumentationProvider();
     }
 
+    @Reference(cardinality = ReferenceCardinality.OPTIONAL)
+    public void setErrorHandler(GraphQLErrorHandler errorHandler) {
+        this.errorHandler = errorHandler;
+    }
+    public void unsetErrorHandler(GraphQLErrorHandler errorHandler) {
+        this.errorHandler = new DefaultGraphQLErrorHandler();
+    }
+
     @Override
     protected GraphQLSchemaProvider getSchemaProvider() {
         return schemaProvider;
@@ -197,4 +206,9 @@ protected Instrumentation getInstrumentation() {
     protected Map<String, Object> transformVariables(GraphQLSchema schema, String query, Map<String, Object> variables) {
         return new GraphQLVariables(schema, query, variables);
     }
+
+    @Override
+    protected GraphQLErrorHandler getGraphQLErrorHandler() {
+        return errorHandler;
+    }
 }

src/main/java/graphql/servlet/SimpleGraphQLServlet.java

@@ -15,7 +15,6 @@
 package graphql.servlet;
 
 import graphql.execution.ExecutionStrategy;
-import graphql.execution.SimpleExecutionStrategy;
 import graphql.execution.instrumentation.Instrumentation;
 import graphql.execution.instrumentation.NoOpInstrumentation;
 import graphql.schema.GraphQLSchema;
@@ -40,14 +39,14 @@ public SimpleGraphQLServlet(GraphQLSchema schema, ExecutionStrategy executionStr
     }
 
     public SimpleGraphQLServlet(GraphQLSchema schema, ExecutionStrategyProvider executionStrategyProvider) {
-        this(schema, executionStrategyProvider, null, null);
+        this(schema, executionStrategyProvider, null, null, null);
     }
 
-    public SimpleGraphQLServlet(final GraphQLSchema schema, ExecutionStrategyProvider executionStrategyProvider, List<GraphQLServletListener> listeners, Instrumentation instrumentation) {
-        this(new DefaultGraphQLSchemaProvider(schema), executionStrategyProvider, listeners, instrumentation);
+    public SimpleGraphQLServlet(final GraphQLSchema schema, ExecutionStrategyProvider executionStrategyProvider, List<GraphQLServletListener> listeners, Instrumentation instrumentation, GraphQLErrorHandler errorHandler) {
+        this(new DefaultGraphQLSchemaProvider(schema), executionStrategyProvider, listeners, instrumentation, errorHandler);
     }
 
-    public SimpleGraphQLServlet(GraphQLSchemaProvider schemaProvider, ExecutionStrategyProvider executionStrategyProvider, List<GraphQLServletListener> listeners, Instrumentation instrumentation) {
+    public SimpleGraphQLServlet(GraphQLSchemaProvider schemaProvider, ExecutionStrategyProvider executionStrategyProvider, List<GraphQLServletListener> listeners, Instrumentation instrumentation, GraphQLErrorHandler errorHandler) {
         super(listeners, null);
 
         this.schemaProvider = schemaProvider;
@@ -58,11 +57,18 @@ public SimpleGraphQLServlet(GraphQLSchemaProvider schemaProvider, ExecutionStrat
         } else {
             this.instrumentation = instrumentation;
         }
+
+        if(errorHandler == null) {
+            this.errorHandler = new DefaultGraphQLErrorHandler();
+        } else {
+            this.errorHandler = errorHandler;
+        }
     }
 
     private final GraphQLSchemaProvider schemaProvider;
     private final ExecutionStrategyProvider executionStrategyProvider;
     private final Instrumentation instrumentation;
+    private final GraphQLErrorHandler errorHandler;
 
     @Override
     protected GraphQLSchemaProvider getSchemaProvider() {
@@ -88,4 +94,9 @@ protected Instrumentation getInstrumentation() {
     protected Map<String, Object> transformVariables(GraphQLSchema schema, String query, Map<String, Object> variables) {
         return variables;
     }
+
+    @Override
+    protected GraphQLErrorHandler getGraphQLErrorHandler() {
+        return errorHandler;
+    }
 }