fix(#469): support async timeout with proper error

Author: oliemansm

build.gradle

@@ -31,7 +31,6 @@ plugins {
     id "org.sonarqube" version "3.2.0"
     id "jacoco"
     id 'io.codearte.nexus-staging' version '0.30.0'
-    id 'com.github.sherter.google-java-format' version '0.9' apply false
 }
 
 sonarqube {
@@ -49,7 +48,6 @@ subprojects {
     apply plugin: 'java'
     apply plugin: 'maven-publish'
     apply plugin: 'signing'
-    apply plugin: 'com.github.sherter.google-java-format'
 
     repositories {
         mavenLocal()
@@ -80,8 +78,6 @@ subprojects {
 
     compileJava.dependsOn(processResources)
 
-    compileJava.mustRunAfter verifyGoogleJavaFormat
-
     test {
         useJUnitPlatform()
 

graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLInvoker.java

@@ -12,7 +12,9 @@
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
 
+@Slf4j
 @RequiredArgsConstructor
 public class GraphQLInvoker {
 

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

@@ -0,0 +1,14 @@
+package graphql.kickstart.servlet;
+
+import java.io.IOException;
+import javax.servlet.AsyncEvent;
+import javax.servlet.AsyncListener;
+
+interface AsyncTimeoutListener extends AsyncListener {
+
+  default void onComplete(AsyncEvent event) throws IOException {}
+
+  default void onError(AsyncEvent event) throws IOException {}
+
+  default void onStartAsync(AsyncEvent event) throws IOException {}
+}

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

@@ -2,16 +2,20 @@
 
 import static graphql.kickstart.servlet.HttpRequestHandler.STATUS_BAD_REQUEST;
 
+import graphql.ExecutionResultImpl;
 import graphql.kickstart.execution.GraphQLInvoker;
 import graphql.kickstart.execution.GraphQLQueryResult;
+import graphql.kickstart.execution.error.GenericGraphQLError;
 import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput;
 import graphql.kickstart.execution.input.GraphQLInvocationInput;
 import graphql.kickstart.execution.input.GraphQLSingleInvocationInput;
 import graphql.kickstart.servlet.input.BatchInputPreProcessResult;
 import graphql.kickstart.servlet.input.BatchInputPreProcessor;
 import java.io.IOException;
 import java.io.UncheckedIOException;
+import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.atomic.AtomicReference;
 import javax.servlet.AsyncContext;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -31,43 +35,65 @@ public void execute(
       GraphQLInvocationInput invocationInput,
       HttpServletRequest request,
       HttpServletResponse response) {
+    ListenerHandler listenerHandler =
+        ListenerHandler.start(request, response, configuration.getListeners());
     if (request.isAsyncSupported()) {
-      AsyncContext asyncContext =
-          request.isAsyncStarted()
-              ? request.getAsyncContext()
-              : request.startAsync(request, response);
-      asyncContext.setTimeout(configuration.getAsyncTimeout());
-      invokeAndHandle(invocationInput, request, response)
-          .thenAccept(aVoid -> asyncContext.complete());
+      invokeAndHandleAsync(invocationInput, request, response, listenerHandler);
     } else {
-      invokeAndHandle(invocationInput, request, response).join();
+      invokeAndHandle(invocationInput, request, response, listenerHandler).join();
     }
   }
 
+  private void invokeAndHandleAsync(
+      GraphQLInvocationInput invocationInput,
+      HttpServletRequest request,
+      HttpServletResponse response,
+      ListenerHandler listenerHandler) {
+    AsyncContext asyncContext =
+        request.isAsyncStarted()
+            ? request.getAsyncContext()
+            : request.startAsync(request, response);
+    asyncContext.setTimeout(configuration.getAsyncTimeout());
+    AtomicReference<CompletableFuture<Void>> futureHolder = new AtomicReference<>();
+    AsyncTimeoutListener timeoutListener =
+        event -> {
+          Optional.ofNullable(futureHolder.get()).ifPresent(it -> it.cancel(true));
+          writeResultResponse(
+              invocationInput,
+              GraphQLQueryResult.create(
+                  new ExecutionResultImpl(new GenericGraphQLError("Timeout"))),
+              (HttpServletRequest) event.getAsyncContext().getRequest(),
+              (HttpServletResponse) event.getAsyncContext().getResponse());
+          listenerHandler.onError(event.getThrowable());
+        };
+    asyncContext.addListener(timeoutListener);
+    asyncContext.start(
+        () ->
+            futureHolder.set(
+                invokeAndHandle(invocationInput, request, response, listenerHandler)
+                    .thenAccept(aVoid -> asyncContext.complete())));
+  }
+
   private CompletableFuture<Void> invokeAndHandle(
       GraphQLInvocationInput invocationInput,
       HttpServletRequest request,
-      HttpServletResponse response) {
-    ListenerHandler listenerHandler =
-        ListenerHandler.start(request, response, configuration.getListeners());
+      HttpServletResponse response,
+      ListenerHandler listenerHandler) {
     return invoke(invocationInput, request, response)
-        .thenAccept(
-            result ->
-                writeResultResponse(invocationInput, result, request, response, listenerHandler))
-        .exceptionally(t -> writeErrorResponse(t, response, listenerHandler))
-        .thenAccept(aVoid -> listenerHandler.onFinally());
+        .thenAccept(it -> writeResultResponse(invocationInput, it, request, response))
+        .thenAccept(it -> listenerHandler.onSuccess())
+        .exceptionally(t -> writeBadRequestError(t, response, listenerHandler))
+        .thenAccept(it -> listenerHandler.onFinally());
   }
 
   private void writeResultResponse(
       GraphQLInvocationInput invocationInput,
       GraphQLQueryResult queryResult,
       HttpServletRequest request,
-      HttpServletResponse response,
-      ListenerHandler listenerHandler) {
+      HttpServletResponse response) {
     QueryResponseWriter queryResponseWriter = createWriter(invocationInput, queryResult);
     try {
       queryResponseWriter.write(request, response);
-      listenerHandler.onSuccess();
     } catch (IOException e) {
       throw new UncheckedIOException(e);
     }
@@ -78,12 +104,18 @@ protected QueryResponseWriter createWriter(
     return queryResponseWriterFactory.createWriter(invocationInput, queryResult, configuration);
   }
 
-  private Void writeErrorResponse(
+  private Void writeBadRequestError(
       Throwable t, HttpServletResponse response, ListenerHandler listenerHandler) {
-    response.setStatus(STATUS_BAD_REQUEST);
-    log.info(
-        "Bad request: path was not \"/schema.json\" or no query variable named \"query\" given", t);
-    listenerHandler.onError(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);
+      listenerHandler.onError(t);
+    } else {
+      log.warn(
+          "Cannot write GraphQL response, because the HTTP response is already committed. It most likely timed out.");
+    }
     return null;
   }
 

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

@@ -23,5 +23,6 @@ public void write(HttpServletRequest request, HttpServletResponse response) thro
     byte[] contentBytes = graphQLObjectMapper.serializeResultAsBytes(result);
     response.setContentLength(contentBytes.length);
     response.getOutputStream().write(contentBytes);
+    response.getOutputStream().flush();
   }
 }