GraphQL Subscriptions + graphql-java-extended-validation directives throws IllegalStateException rather than validation error

[iparadiso]

Hi Spring-graphql team. I ran into an incompatibility in using graphql-java-extended-validation directives in a GraphQL subscription API.

Expected behavior

• GraphQL subscriptions with an input field using a graphql-java-extended-validation annotation that conflicts with the validation directive returns a GraphQL error response with the validation error populated.

Actual Behavior

• The ContextDataFetcherDecorator throws java.lang.IllegalStateException: Expected Publisher for a subscription

Steps To Reproduce:

I've included a reproducer below to demonstrate the following example. First create a subscription type with a validation directive.

type Subscription {
    subscribeToBooks(authorFilter: String! @Size(min:5, max:50)): Book!
}

Then, call the subscription using an input parameter that violates the @Size directive.

 subscription {
                subscribeToBooks(authorFilter: "test") {
}

I created a test app with a test case that reproduces this problem.

From debugging this test, the exception is thrown in ContextDataFetcherDecorator::get() where it calls ReactiveAdapterRegistryHelper.toSubscriptionFlux because the API call is a subscription.

if (this.subscription) {
			return ReactiveAdapterRegistryHelper.toSubscriptionFlux(value)
					.onErrorResume((exception) -> {
						// Already handled, e.g. controller methods?
						if (exception instanceof SubscriptionPublisherException) {
							return Mono.error(exception);
						}
						return this.subscriptionExceptionResolver.resolveException(exception)
								.flatMap((errors) -> Mono.error(new SubscriptionPublisherException(errors, exception)));
					})
					.contextWrite(snapshot::updateContext);
		}

However, the value here isn't a reactive type. graphql-java-extended-validation returns a DataFetcherResult containing the GraphQL error for the validation directive. This is returned here.

The ReactiveAdapterRegistryHelper.toSubscriptionFlux fails to return a Flux type and looks to find a ReactiveAdapter from the ReactiveAdapterRegistry which doesn't have an adapter for DataFetcherResult and thus throws the IllegalStateException.

	public static Flux<?> toSubscriptionFlux(@Nullable Object result) {
		if (result == null) {
			return Flux.empty();
		}
		if (result instanceof Publisher<?> publisher) {
			return Flux.from(publisher);
		}
		ReactiveAdapter adapter = registry.getAdapter(result.getClass());
		Assert.state(adapter != null, "Expected Publisher for a subscription");
		return Flux.from(adapter.toPublisher(result));
	}

Workarounds attempted

I tried creating a reactive adapter for DataFetcherResult, as shown below

public class DataFetcherResultAdapter {

    public static void registerAdapter() {
        ReactiveAdapterRegistry registry = ReactiveAdapterRegistry.getSharedInstance();
        // Define the descriptor for DataFetcherResult
        ReactiveTypeDescriptor descriptor = ReactiveTypeDescriptor.singleOptionalValue(DataFetcherResult.class, () -> null);
        Function<Object, Publisher<?>> toAdapter = source -> {
            if (source instanceof DataFetcherResult) {
                DataFetcherResult<?> result = (DataFetcherResult<?>) source;
                if (result.hasErrors()) {
                    throw new SubscriptionPublisherException(result.getErrors(), new Throwable());
                }
                return Flux.just(result);
            }
            return Flux.empty();
        };
        Function<Publisher<?>, Object> fromAdapter = source -> Flux.from(source);
        registry.registerReactiveType(descriptor, toAdapter, fromAdapter);
    }
}

but that impacts the non-subscription flow, and this adapter gets picked up for all graphQL API's like mutations, as shown in this test example which breaks the existing behavior of that test.

It is also non-intuitive and hacky to write a reactive adapter for a non-reactive type, so this workaround doesn't sound like the right solution.

Second workaround is to abandon validation directives and use bean validation annotations in the java code directly. That does work correctly, although it returns a different GraphQL error and defeats the purpose of graphql-java-extended-validation.

It feels like the if (this.subscription) block above should try and find an adapter, but if it fails to find one just pass through and let the value object return normally.

[iparadiso]

I realized my reproducer repo was private. I just changed it to public. Please ping me if you have trouble accessing it.