fix(#298): convert exception to graphql error

Author: oliemansm

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestInvokerImpl.java

@@ -1,8 +1,8 @@
 package graphql.kickstart.servlet;
 
-import static graphql.kickstart.servlet.HttpRequestHandler.STATUS_BAD_REQUEST;
-
+import graphql.ExecutionResult;
 import graphql.ExecutionResultImpl;
+import graphql.GraphQLException;
 import graphql.kickstart.execution.GraphQLInvoker;
 import graphql.kickstart.execution.GraphQLQueryResult;
 import graphql.kickstart.execution.error.GenericGraphQLError;
@@ -15,6 +15,7 @@
 import java.io.UncheckedIOException;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.servlet.AsyncContext;
 import javax.servlet.http.HttpServletRequest;
@@ -82,7 +83,8 @@ private CompletableFuture<Void> invokeAndHandle(
     return invoke(invocationInput, request, response)
         .thenAccept(it -> writeResultResponse(invocationInput, it, request, response))
         .thenAccept(it -> listenerHandler.onSuccess())
-        .exceptionally(t -> writeBadRequestError(t, response, listenerHandler))
+        .exceptionally(
+            t -> writeErrorResponse(invocationInput, request, response, listenerHandler, t))
         .thenAccept(it -> listenerHandler.onFinally());
   }
 
@@ -99,18 +101,18 @@ private void writeResultResponse(
     }
   }
 
-  protected QueryResponseWriter createWriter(
-      GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult) {
-    return queryResponseWriterFactory.createWriter(invocationInput, queryResult, configuration);
-  }
-
-  private Void writeBadRequestError(
-      Throwable t, HttpServletResponse response, ListenerHandler listenerHandler) {
+  private Void writeErrorResponse(
+      GraphQLInvocationInput invocationInput,
+      HttpServletRequest request,
+      HttpServletResponse response,
+      ListenerHandler listenerHandler,
+      Throwable t) {
     if (!response.isCommitted()) {
-      response.setStatus(STATUS_BAD_REQUEST);
-      log.info(
-          "Bad request: path was not \"/schema.json\" or no query variable named \"query\" given",
-          t);
+      writeResultResponse(
+          invocationInput,
+          GraphQLQueryResult.create(toErrorResult(t)),
+          request,
+          response);
       listenerHandler.onError(t);
     } else {
       log.warn(
@@ -119,6 +121,19 @@ private Void writeBadRequestError(
     return null;
   }
 
+  private ExecutionResult toErrorResult(Throwable t) {
+    String message =
+        t instanceof CompletionException && t.getCause() != null
+            ? t.getCause().getMessage()
+            : t.getMessage();
+    return new ExecutionResultImpl(new GenericGraphQLError(message));
+  }
+
+  protected QueryResponseWriter createWriter(
+      GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult) {
+    return queryResponseWriterFactory.createWriter(invocationInput, queryResult, configuration);
+  }
+
   private CompletableFuture<GraphQLQueryResult> invoke(
       GraphQLInvocationInput invocationInput,
       HttpServletRequest request,

graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy

@@ -1,12 +1,15 @@
 package graphql.kickstart.servlet.cache
 
+import graphql.ExecutionResult
 import graphql.kickstart.execution.GraphQLInvoker
+import graphql.kickstart.execution.GraphQLObjectMapper
 import graphql.kickstart.execution.GraphQLQueryResult
 import graphql.kickstart.execution.input.GraphQLSingleInvocationInput
 import graphql.kickstart.servlet.GraphQLConfiguration
 import graphql.kickstart.servlet.HttpRequestInvoker
 import spock.lang.Specification
 
+import javax.servlet.ServletOutputStream
 import javax.servlet.http.HttpServletRequest
 import javax.servlet.http.HttpServletResponse
 import java.util.concurrent.CompletableFuture
@@ -22,6 +25,8 @@ class CachingHttpRequestInvokerTest extends Specification {
   def httpRequestInvokerMock
   def graphqlInvoker
   def configuration
+  def graphqlObjectMapper
+  def outputStreamMock
 
   def setup() {
     cacheReaderMock = Mock(CacheReader)
@@ -32,11 +37,17 @@ class CachingHttpRequestInvokerTest extends Specification {
     configuration = Mock(GraphQLConfiguration)
     httpRequestInvokerMock = Mock(HttpRequestInvoker)
     graphqlInvoker = Mock(GraphQLInvoker)
+    graphqlObjectMapper = Mock(GraphQLObjectMapper)
+    outputStreamMock = Mock(ServletOutputStream)
     cachingInvoker = new CachingHttpRequestInvoker(configuration, httpRequestInvokerMock, cacheReaderMock)
 
     configuration.getResponseCacheManager() >> responseCacheManagerMock
     configuration.getGraphQLInvoker() >> graphqlInvoker
+    configuration.getObjectMapper() >> graphqlObjectMapper
+    graphqlObjectMapper.serializeResultAsBytes(_ as ExecutionResult) >> new byte[0]
     graphqlInvoker.queryAsync(invocationInputMock) >> CompletableFuture.completedFuture(Mock(GraphQLQueryResult))
+
+    responseMock.getOutputStream() >> outputStreamMock
   }
 
   def "should execute regular invoker if cache not exists"() {