Annotated exception handlers yield correctly

Closes gh-160

Author: rstoyanchev

spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java

@@ -62,7 +62,6 @@
 import org.springframework.format.support.DefaultFormattingConversionService;
 import org.springframework.format.support.FormattingConversionService;
 import org.springframework.graphql.data.GraphQlArgumentBinder;
-import org.springframework.graphql.execution.SelfDescribingDataFetcher;
 import org.springframework.graphql.data.method.HandlerMethod;
 import org.springframework.graphql.data.method.HandlerMethodArgumentResolver;
 import org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite;
@@ -71,6 +70,7 @@
 import org.springframework.graphql.execution.BatchLoaderRegistry;
 import org.springframework.graphql.execution.DataFetcherExceptionResolver;
 import org.springframework.graphql.execution.RuntimeWiringConfigurer;
+import org.springframework.graphql.execution.SelfDescribingDataFetcher;
 import org.springframework.graphql.execution.SubscriptionPublisherException;
 import org.springframework.lang.Nullable;
 import org.springframework.stereotype.Controller;
@@ -617,18 +617,21 @@ else if (result instanceof Flux<?>) {
 			return result;
 		}
 
-		private Mono<DataFetcherResult<?>> handleException(
+		private Mono<DataFetcherResult<Object>> handleException(
 				Throwable ex, DataFetchingEnvironment env, DataFetcherHandlerMethod handlerMethod) {
 
 			return this.exceptionResolver.resolveException(ex, env, handlerMethod.getBean())
-					.map(errors -> DataFetcherResult.newResult().errors(errors).build());
+					.map(errors -> DataFetcherResult.newResult().errors(errors).build())
+					.switchIfEmpty(Mono.error(ex));
 		}
 
+		@SuppressWarnings("unchecked")
 		private <T> Publisher<T> handleSubscriptionError(
 				Throwable ex, DataFetchingEnvironment env, DataFetcherHandlerMethod handlerMethod) {
 
-			return this.exceptionResolver.resolveException(ex, env, handlerMethod.getBean())
-					.flatMap(errors -> Mono.error(new SubscriptionPublisherException(errors, ex)));
+			return (Publisher<T>) this.exceptionResolver.resolveException(ex, env, handlerMethod.getBean())
+					.flatMap(errors -> Mono.error(new SubscriptionPublisherException(errors, ex)))
+					.switchIfEmpty(Mono.error(ex));
 		}
 
 		@Override

spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java

@@ -160,7 +160,7 @@ private static Map<Class<? extends Throwable>, Method> findExceptionHandlers(Cla
 	 * @param ex the exception to resolve
 	 * @param environment the environment for the invoked {@code DataFetcher}
 	 * @param controller the controller that raised the exception, if applicable
-	 * @return a {@code Mono} with errors as specified in
+	 * @return a {@code Mono} with resolved {@code GraphQLError}s as specified in
 	 * {@link DataFetcherExceptionResolver#resolveException(Throwable, DataFetchingEnvironment)}
 	 */
 	public Mono<List<GraphQLError>> resolveException(
@@ -194,7 +194,7 @@ else if (logger.isWarnEnabled()) {
 		}
 
 		if (methodHolder == null) {
-			return Mono.error(ex);
+			return Mono.empty();
 		}
 
 		return invokeExceptionHandler(ex, environment, controllerOrAdvice, methodHolder);
@@ -405,21 +405,21 @@ else if (parameterType.equals(Object.class)) {
 		/** Adapter for a single GraphQLError */
 		ReturnValueAdapter forSingleError = (result, returnType, ex) ->
 				(result == null ?
-						Mono.error(ex) :
+						Mono.empty() :
 						Mono.just(Collections.singletonList((GraphQLError) result)));
 
 		/** Adapter for a collection of GraphQLError's */
 		ReturnValueAdapter forCollection = (result, returnType, ex) ->
 				(result == null ?
-						Mono.error(ex) :
+						Mono.empty() :
 						Mono.just((result instanceof List ?
 								(List<GraphQLError>) result :
 								new ArrayList<>((Collection<GraphQLError>) result))));
 
 		/** Adapter for Object */
 		ReturnValueAdapter forObject = (result, returnType, ex) -> {
 			if (result == null) {
-				return Mono.error(ex);
+				return Mono.empty();
 			}
 			else if (result instanceof GraphQLError) {
 				return forSingleError.adapt(result, returnType, ex);
@@ -438,12 +438,12 @@ else if (result instanceof Collection<?>) {
 
 		/** Adapter for {@code Mono<Void>} */
 		ReturnValueAdapter forMonoVoid = (result, returnType, ex) ->
-				(result == null ? Mono.error(ex) : Mono.just(Collections.emptyList()));
+				(result == null ? Mono.empty() : Mono.just(Collections.emptyList()));
 
 		/** Adapter for a {@code Mono} wrapping any of the other synchronous return value types */
 		ReturnValueAdapter forMono = (result, returnType, ex) ->
 				(result == null ?
-						Mono.error(ex) :
+						Mono.empty() :
 						((Mono<?>) result).flatMap(o -> forObject.adapt(o, returnType, ex)).switchIfEmpty(Mono.error(ex)));
 	}
 

spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolverTests.java

@@ -102,9 +102,8 @@ void leaveUnresolvedViaNullReturnValue() {
 		AnnotatedControllerExceptionResolver resolver = exceptionResolver();
 		resolver.registerController(controller.getClass());
 
-		StepVerifier.create(resolver.resolveException(ex, this.environment, controller))
-				.expectErrorSatisfies(actualEx -> assertThat(actualEx).isSameAs(ex))
-				.verify();
+		StepVerifier.create(resolver.resolveException(ex, this.environment, controller)).verifyComplete();
+		StepVerifier.create(resolver.resolveException(ex, this.environment, controller)).verifyComplete();
 	}
 
 	@Test

spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingInvocationTests.java

@@ -15,10 +15,12 @@
  */
 package org.springframework.graphql.data.method.annotation.support;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BiConsumer;
 
 import graphql.GraphQLContext;
 import graphql.GraphQLError;
@@ -49,11 +51,11 @@
 import org.springframework.graphql.data.method.annotation.SchemaMapping;
 import org.springframework.graphql.data.method.annotation.SubscriptionMapping;
 import org.springframework.graphql.execution.BatchLoaderRegistry;
+import org.springframework.graphql.execution.DataFetcherExceptionResolver;
 import org.springframework.graphql.execution.DefaultBatchLoaderRegistry;
 import org.springframework.graphql.execution.ErrorType;
 import org.springframework.graphql.execution.SubscriptionPublisherException;
 import org.springframework.stereotype.Controller;
-import org.springframework.util.Assert;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -229,6 +231,34 @@ void handleExceptionFromQuery() {
 		assertThat(responseHelper.error(0).message()).isEqualTo("Rejected: Bad input");
 	}
 
+	@Test
+	void handleExceptionWithResolverWhenNoAnnotatedExceptionHandlerMatches() {
+		String document = "{ " +
+				"  booksByCriteria(criteria: {author:\"Heller\"}) { " +
+				"    id" +
+				"    name" +
+				"  }" +
+				"}";
+
+		DataFetcherExceptionResolver resolver = (ex, env) ->
+				Mono.just(Collections.singletonList(
+						GraphQLError.newError().errorType(ErrorType.INTERNAL_ERROR)
+								.message("Rejected: " + ex.getMessage())
+								.build()));
+
+		ExecutionGraphQlService service = graphQlService((configurer, setup) -> {
+			setup.exceptionResolver(configurer.getExceptionResolver()); // First @ControllerAdvice (no match)
+			setup.exceptionResolver(resolver); // Then resolver
+		});
+
+		Mono<ExecutionGraphQlResponse> responseMono = service.execute(TestExecutionRequest.forDocument(document));
+
+		ResponseHelper responseHelper = ResponseHelper.forResponse(responseMono);
+		assertThat(responseHelper.errorCount()).isEqualTo(1);
+		assertThat(responseHelper.error(0).errorType()).isEqualTo("INTERNAL_ERROR");
+		assertThat(responseHelper.error(0).message()).isEqualTo("Rejected: Fetch failure");
+	}
+
 	@Test
 	void handleExceptionFromSubscription() {
 		String document = "subscription { " +
@@ -257,6 +287,10 @@ void handleExceptionFromSubscription() {
 
 
 	private ExecutionGraphQlService graphQlService() {
+		return graphQlService((configurer, setup) -> {});
+	}
+
+	private ExecutionGraphQlService graphQlService(BiConsumer<AnnotatedControllerConfigurer, GraphQlSetup> consumer) {
 		BatchLoaderRegistry registry = new DefaultBatchLoaderRegistry();
 
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
@@ -269,10 +303,10 @@ private ExecutionGraphQlService graphQlService() {
 		configurer.setApplicationContext(context);
 		configurer.afterPropertiesSet();
 
-		return GraphQlSetup.schemaResource(BookSource.schema)
-				.runtimeWiring(configurer)
-				.dataLoaders(registry)
-				.toGraphQlService();
+		GraphQlSetup setup = GraphQlSetup.schemaResource(BookSource.schema).runtimeWiring(configurer);
+		consumer.accept(configurer, setup);
+
+		return setup.dataLoaders(registry).toGraphQlService();
 	}
 
 
@@ -292,7 +326,10 @@ public Book bookById(@Argument Long id) {
 
 		@QueryMapping
 		public List<Book> booksByCriteria(@Argument BookCriteria criteria) {
-			Assert.isTrue(!criteria.getAuthor().equalsIgnoreCase("Fitzgerald"), "Bad input");
+			switch (criteria.getAuthor()) {
+				case "Fitzgerald" -> throw new IllegalArgumentException("Bad input");
+				case "Heller" -> throw new IllegalStateException("Fetch failure");
+			}
 			return BookSource.findBooksByAuthor(criteria.getAuthor());
 		}