onFinally() callback called too early, before the GraphQL query has finished executing #293
Description
Describe the bug
Due to changes made in #259, the onFinally() callback of GraphQLServletListener is called before the GraphQL query has finished executing. This affects releases 10.1.0 and 11.0.0.
More exactly, in AbstractGraphQLHttpServlet, we have the place where onFinally() callback is called:
private void doRequest(HttpServletRequest request, HttpServletResponse response) {
List<GraphQLServletListener.RequestCallback> requestCallbacks = runListeners(
l -> l.onRequest(request, response));
try {
getConfiguration().getHttpRequestHandler().handle(request, response);
runCallbacks(requestCallbacks, c -> c.onSuccess(request, response));
} catch (Exception t) {
log.error("Error executing GraphQL request!", t);
runCallbacks(requestCallbacks, c -> c.onError(request, response, t));
} finally {
runCallbacks(requestCallbacks, c -> c.onFinally(request, response));
}
}
In previous versions, the line getConfiguration().getHttpRequestHandler().handle(request, response) would block until the GraphQL query was finished. But nowadays, it just starts the query without blocking, as can be seen in HttpRequestInvokerImpl, where the execution is started using CompletableFutures, without blocking the flow:
public void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request,
HttpServletResponse response) {
if (request.isAsyncSupported()) {
AsyncContext asyncContext = request.isAsyncStarted()
? request.getAsyncContext()
: request.startAsync(request, response);
asyncContext.setTimeout(configuration.getAsyncTimeout());
invoke(invocationInput, request, response)
.thenAccept(result -> writeResultResponse(invocationInput, result, request, response))
.exceptionally(t -> writeErrorResponse(t, response))
.thenAccept(aVoid -> asyncContext.complete());
} else {
...
}
}
(please note that under Spring Boot, request.isAsyncSupported() is by default true)
To Reproduce
Put a breakpoint (or a logging statement) in AbstractGraphQLHttpServlet at the line where onFinally() is called, and another one in HttpRequestInvokerImpl, inside the method writeResultResponse(). You'll notice that onFinally() is called before the GraphQL query has finished executing (i.e. before writeResultResponse() has been called).
Expected behavior
This is a regression. The expectation is that onFinally() is called after the GraphQL query has finished. This is useful for example, in order to release resources used by GraphQL