Skip to content

Prevent further invocation of data fetchers when GraphQL request is cancelled #1153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bclozel opened this issue Mar 14, 2025 · 0 comments
Closed
Assignees
Labels
in: core Issues related to config and core support type: enhancement A general enhancement
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Mar 14, 2025

In #1149, we fixed the fact that reactive controllers were not getting cancel signals from the transport layer because the CompletableFuture part of the GraphQL engine was not communicating that information back to data fetchers. As of #450, Spring for GraphQL ships with a TimeoutWebGraphQlInterceptor that triggers a cancel signal if the GraphQL exchange exceeds a configured duration.

While that cancel signal will consistently abort the processing of all Publisher based data fetchers, this is not the case for other types of data fetchers. Currently, the ContextDataFetcherDecorator visits all registered data fetchers and wraps them to save/restore the context propagation. Wrapped data fetchers can return non-reactive types, and in this case there is not consistent way to cancel their execution right now.

There are existing solutions used by the community, but we're wondering how applicable they are in general. Thread.interrupt requires coordination unlike Thread.stop. In both cases, the data fetcher is probably using a blocking HTTP client or a database driver that themselves are scheduling work on a separate thread. Here, work will not be cancelled. Manually scheduling work with such a wrapper also creates other disadvantages: the data fetcher loses all current context (no traceId in logs, no security context) and there might be a significant performance degradation.

Because data fetching often happens in stages, we could consider a different solution where we check the current cancellation status before returning a resolved value. Instead of returning a value and triggering subsequent data fetchers, we could return an error and abort further execution. While this won't cancel the current stage of data fetching, this will prevent further processing.

@bclozel bclozel added in: core Issues related to config and core support type: enhancement A general enhancement labels Mar 14, 2025
@bclozel bclozel added this to the 1.4.x milestone Mar 14, 2025
@bclozel bclozel self-assigned this Mar 14, 2025
@bclozel bclozel modified the milestones: 1.4.x, 1.4.0-RC1 Mar 27, 2025
@bclozel bclozel changed the title Abort non-reactive controller methods when GraphQL request is cancelled Prevent further invocation of data fetchers when GraphQL request is cancelled Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues related to config and core support type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant