Support timeout for GraphQL request handling and propagate cancellation to controller methods

[tarun3300]

Hi,

We would like to setup custom timeout for the async GraphQL query like we used to do in the kickstart GraphQL as below.

Graphql query timeout settings

graphql.servlet.async.timeout=5s

[rstoyanchev]

Is your goal to control malicious queries, as mentioned in this issue? A simple timeout is a very basic mechanism that can impact other scenarios as well such as a slow network that have nothing to do with that concern.

GraphQL Java provides Query Complexity and Query Depth instrumentations. We think that's a better mechanism to control malicious queries. There is a Boot issue spring-projects/spring-graphql#469 for an enhancement in that area, and we can consider improved support here as well if there are concrete suggestions.

That said if you really wanted, you can apply a timeout through an interceptor. For example:

@Bean
WebGraphQlInterceptor interceptor() {
    return (request, chain) -> chain.next(request)
            .timeout(Duration.ofSeconds(2), Mono.error(new ResponseStatusException(HttpStatus.SERVICE_UNAVAILABLE)));
}

This ends the response and should also cancel GraphQL request processing. However, note there is currently an issue that prevents it from working. You can watch https://github.com/reactor/reactor-core/issues/3138, and when that's fixed, it should work as expected.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[spring-projects-issues]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

[turboezh]

@rstoyanchev Imho, special property for global request timeout is the first thing anyone would try to lookup. Reactor way is very non-obvious. Could it be reasonable to built in and autoconfigure such an interceptor based on corresponding property?

[amruta98]

Hello @rstoyanchev, is there any update on this. I have tried above solution but it is not working

[rstoyanchev]

My apologies, indeed the above doesn't work when data is fetched synchronously because blocking occurs as part of the initial call while composing the reactive chain.

I've confirmed the following works:

public class RequestTimeoutInterceptor implements WebGraphQlInterceptor {

    private final Duration timeout;

    public RequestTimeoutInterceptor(Duration timeout) {
        this.timeout = timeout;
    }

    @Override
    public Mono<WebGraphQlResponse> intercept(WebGraphQlRequest request, Chain chain) {
        return chain.next(request)
                .subscribeOn(Schedulers.boundedElastic())
                .timeout(this.timeout, Mono.error(new ResponseStatusException(HttpStatus.SERVICE_UNAVAILABLE)));
    }

}

I am re-opening this in order to make such an interceptor available. Using an interceptor has the advantage of targeting GraphQL request handling vs any other HTTP request.

[amruta98]

Hi @rstoyanchev, Thanks for reopening this issue. I tried the interceptor option but it didn't work. The query that I am working on has following structure:

query getData {
  appData {
    name
    appConfig{
      name
      id
    }
  }
}

Now appConfig resolver's batch loader is taking too much time to fetch data because of which whole query gets aborted after 30 seconds with AsyncRequestTimeoutException.

[rstoyanchev]

@amruta98 can you double check that the interceptor is registered and is getting invoked, e.g. by adding a log message or putting a breakpoint? It should be as simple as declaring it as a bean but just to be sure.

[amruta98]

@rstoyanchev I checked it by adding logs and by debugging. It is getting invoked but not working as expected

[rstoyanchev]

If you can create an isolated sample, I will have a look.

[amruta98]

@rstoyanchev Actually I already had Interceptor in my project for passing custom context and I modified that with timeout related code. Please check below code

@Component
@EnableAutoConfiguration
public class DemoInterceptor implements  WebGraphQlInterceptor{
 @Override
    public Mono<WebGraphQlResponse> intercept(WebGraphQlRequest request, WebGraphQlInterceptor.Chain chain) {

       request.configureExecutionInput((executionInput, builder) ->
                builder.graphQLContext(context -> {
                            CustomContext customContext = createCustomContext(request);
                            context.put("customContext", customContext);
                        }).build());
        return chain.next(request).subscribeOn(Schedulers.boundedElastic())
                .timeout(Duration.ofMillis(60000), Mono.error(new ResponseStatusException(HttpStatus.SERVICE_UNAVAILABLE)));
    }
}

[bclozel]

@amruta98 What do you mean? Is your custom interceptor working? If it's not, can you share a minimal sample?

[amruta98]

Hi @bclozel , My custom interceptor is working, I am able to fetch custom context in my api also, but the changes related to timeout are not working as expected. .subscribeOn(Schedulers.boundedElastic()) .timeout(Duration.ofMillis(60000), Mono.error(new ResponseStatusException(HttpStatus.SERVICE_UNAVAILABLE)))

[bclozel]

ok then please provide a minimal sample.

[rstoyanchev]

@amruta98 as mentioned previously, I have tested the above. There must be some other reason why it doesn't work for you. Are you able to provide a small sample, e.g. via https://start.spring.io that demonstrates the issue?

[tstocker-black-cape]

I'm also having trouble getting this to work. By following your example, I'm able to get the client to receive the request timeout, but the cancellation does not propagate downstream. I have potentially expensive database queries that need to be aborted after a timeout.

My use case is that I am required to timeout requests after X number of seconds. We are seeing a snowball effect on the server because requests that have "timed out" are still doing work. If I set a timeout directly on my mono in my service, things work as expected. It seems to me that either spring isn't propagating this timeout correctly, or I just don't know what I'm doing... Honestly could be either.

In summary, I've been looking for how to set a simple request timeout for a day or so now and I'm disappointed that something so simple doesn't just have a config option.

[bclozel]

@tstocker-black-cape we've been waiting for a minimal sample for 6 months now. If you can provide one, maybe we can make progress on this.

[tstocker-black-cape]

I'm getting this error based on the first provided example in this thread. I'll drop the snippets here. This can be recreated with just a few lines of code.

For the interceptor

@Component
public class TimeoutInterceptor implements WebGraphQlInterceptor {
    @NonNull
    @Override
    public Mono<WebGraphQlResponse> intercept(
            @NonNull WebGraphQlRequest request,
            @NonNull Chain chain) {
        // Handle the request
        return chain.next(request)
                // Set the request timeout
                .timeout(Duration.ofSeconds(1))
                // Map the timeout exception
                .onErrorMap(TimeoutException.class, e -> new ResponseStatusException(REQUEST_TIMEOUT));
    }
}

For the GraphQL controller

@Controller
public class HelloController {
    @QueryMapping
    public Mono<String> greeting() {
        long start = System.nanoTime();
        return Mono.just("Hello")
                .delayElement(Duration.ofSeconds(5))
                .doOnSubscribe(s -> System.out.println("Handling greeting request"))
                .doOnNext(greeting -> System.out.println("returning: " + greeting))
                .doOnCancel(() -> System.out.println("Cancelled! (this never happens...)"))
                .doOnError(System.out::println)
                .doFinally(signal -> System.out.println("Finished with signal: %s after %d ms".formatted(
                        signal, NANOSECONDS.toMillis(System.nanoTime() - start))));
    }
}

Please let me know if you need more than this. What happens when I trigger this request is that I do get a timeout exception after 1 second as expected, however, the Mono created in the controller is NEVER cancelled.

The service logs

The postman response

[tstocker-black-cape]

@bclozel Is the code I provided sufficient for you to recreate this issue?

[bclozel]

Thanks @tstocker-black-cape , this is enough to reproduce. I managed to isolate this behavior in a minimal sample and it appears the signal is lost somewhere in GraphQL. I have started a discussion on graphql-java as I'm not sure about the expected behavior here.

[bclozel]

I was mistaken, it seems this is a well-known limitation of CompletableFuture and there's nothing we can do about it here. Now we should consider whether providing a timeout interceptor in spring-graphql is relevant still because of this behavior.

[tstocker-black-cape]

Buuuumer... Thanks for looking into this. Here's a minimal example of our workaround. I'm not sure if you'd be interested in integrating this into spring boot. There may be edge cases we're missing, but this works for our use cases.

// The interceptor configures the timeout on the mono and manually passes state to a sink that is stored as a context value
/**
 * Interceptor to set the request timeout
 */
@Configuration
public class TimeoutInterceptor implements WebGraphQlInterceptor {
    // Constants
    private static final String IS_CANCELLED_CONTEXT_KEY = "isCancelled";
    private static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(10);

    @NonNull
    @Override
    public Mono<WebGraphQlResponse> intercept(
            @NonNull WebGraphQlRequest request,
            @NonNull Chain chain) {
        // Create the is cancelled flag
        Sinks.One<Boolean> isCancelled = Sinks.one();

        // Hook the cancellation into the context
        request.configureExecutionInput((executionInput, builder) -> builder
                .graphQLContext(Map.of(IS_CANCELLED_CONTEXT_KEY, isCancelled.asMono()))
                .build());

        // Execute the call
        return chain.next(request)
                // Set the timeout
                .timeout(REQUEST_TIMEOUT,
                        // Send the cancellation signal
                        Mono.fromRunnable(() -> isCancelled.tryEmitValue(true))
                                // Throw the timeout error
                                .then(Mono.error(new ResponseStatusException(HttpStatus.REQUEST_TIMEOUT))));
    }
}

// We wrap each controller method with a function to apply that state via the `takeUntilOther` mechanism
public class ControllerUtils {
    public static <T> Mono<T> wrapResponse(
            Mono<Void> isCancelled,
            Mono<T> source) {
        // Return the input source
        return source
                // Add the cancellation hook
                .takeUntilOther(isCancelled);
    }

    public static <T> Flux<T> wrapResponse(
            Mono<Void> isCancelled,
            Flux<T> source) {
        // Return the input source
        return source
                // Add the cancellation hook
                .takeUntilOther(isCancelled);
    }
}

[rstoyanchev]

Thanks for all the feedback. We can do something along the lines of https://github.com/spring-projects/spring-graphql/issues/450#issuecomment-2310152689, and propagating the cancellation from ContextDataFetcherDecorator, which already deals with similar concerns.

This would have an important limitation though since we can only propagate cancellation via reactive types. There is no way to cancel a controller method executing synchronously. For that at best, we could have a flag in the GraphQLContext the controller method, and the controller method would have to check it periodically. I don't see any other option.