Skip to content

Commit 6dba688

Browse files
rstoyanchevrstoyanchev
committed
Annotated exception handlers yield correctly
Closes gh-160
1 parent 698713a commit 6dba688

File tree

4 files changed

+60
-21
lines changed

4 files changed

+60
-21
lines changed

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@
6262
import org.springframework.format.support.DefaultFormattingConversionService;
6363
import org.springframework.format.support.FormattingConversionService;
6464
import org.springframework.graphql.data.GraphQlArgumentBinder;
65-
import org.springframework.graphql.execution.SelfDescribingDataFetcher;
6665
import org.springframework.graphql.data.method.HandlerMethod;
6766
import org.springframework.graphql.data.method.HandlerMethodArgumentResolver;
6867
import org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite;
@@ -71,6 +70,7 @@
7170
import org.springframework.graphql.execution.BatchLoaderRegistry;
7271
import org.springframework.graphql.execution.DataFetcherExceptionResolver;
7372
import org.springframework.graphql.execution.RuntimeWiringConfigurer;
73+
import org.springframework.graphql.execution.SelfDescribingDataFetcher;
7474
import org.springframework.graphql.execution.SubscriptionPublisherException;
7575
import org.springframework.lang.Nullable;
7676
import org.springframework.stereotype.Controller;
@@ -617,18 +617,21 @@ else if (result instanceof Flux<?>) {
617617
return result;
618618
}
619619

620-
private Mono<DataFetcherResult<?>> handleException(
620+
private Mono<DataFetcherResult<Object>> handleException(
621621
Throwable ex, DataFetchingEnvironment env, DataFetcherHandlerMethod handlerMethod) {
622622

623623
return this.exceptionResolver.resolveException(ex, env, handlerMethod.getBean())
624-
.map(errors -> DataFetcherResult.newResult().errors(errors).build());
624+
.map(errors -> DataFetcherResult.newResult().errors(errors).build())
625+
.switchIfEmpty(Mono.error(ex));
625626
}
626627

628+
@SuppressWarnings("unchecked")
627629
private <T> Publisher<T> handleSubscriptionError(
628630
Throwable ex, DataFetchingEnvironment env, DataFetcherHandlerMethod handlerMethod) {
629631

630-
return this.exceptionResolver.resolveException(ex, env, handlerMethod.getBean())
631-
.flatMap(errors -> Mono.error(new SubscriptionPublisherException(errors, ex)));
632+
return (Publisher<T>) this.exceptionResolver.resolveException(ex, env, handlerMethod.getBean())
633+
.flatMap(errors -> Mono.error(new SubscriptionPublisherException(errors, ex)))
634+
.switchIfEmpty(Mono.error(ex));
632635
}
633636

634637
@Override

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ private static Map<Class<? extends Throwable>, Method> findExceptionHandlers(Cla
160160
* @param ex the exception to resolve
161161
* @param environment the environment for the invoked {@code DataFetcher}
162162
* @param controller the controller that raised the exception, if applicable
163-
* @return a {@code Mono} with errors as specified in
163+
* @return a {@code Mono} with resolved {@code GraphQLError}s as specified in
164164
* {@link DataFetcherExceptionResolver#resolveException(Throwable, DataFetchingEnvironment)}
165165
*/
166166
public Mono<List<GraphQLError>> resolveException(
@@ -194,7 +194,7 @@ else if (logger.isWarnEnabled()) {
194194
}
195195

196196
if (methodHolder == null) {
197-
return Mono.error(ex);
197+
return Mono.empty();
198198
}
199199

200200
return invokeExceptionHandler(ex, environment, controllerOrAdvice, methodHolder);
@@ -405,21 +405,21 @@ else if (parameterType.equals(Object.class)) {
405405
/** Adapter for a single GraphQLError */
406406
ReturnValueAdapter forSingleError = (result, returnType, ex) ->
407407
(result == null ?
408-
Mono.error(ex) :
408+
Mono.empty() :
409409
Mono.just(Collections.singletonList((GraphQLError) result)));
410410

411411
/** Adapter for a collection of GraphQLError's */
412412
ReturnValueAdapter forCollection = (result, returnType, ex) ->
413413
(result == null ?
414-
Mono.error(ex) :
414+
Mono.empty() :
415415
Mono.just((result instanceof List ?
416416
(List<GraphQLError>) result :
417417
new ArrayList<>((Collection<GraphQLError>) result))));
418418

419419
/** Adapter for Object */
420420
ReturnValueAdapter forObject = (result, returnType, ex) -> {
421421
if (result == null) {
422-
return Mono.error(ex);
422+
return Mono.empty();
423423
}
424424
else if (result instanceof GraphQLError) {
425425
return forSingleError.adapt(result, returnType, ex);
@@ -438,12 +438,12 @@ else if (result instanceof Collection<?>) {
438438

439439
/** Adapter for {@code Mono<Void>} */
440440
ReturnValueAdapter forMonoVoid = (result, returnType, ex) ->
441-
(result == null ? Mono.error(ex) : Mono.just(Collections.emptyList()));
441+
(result == null ? Mono.empty() : Mono.just(Collections.emptyList()));
442442

443443
/** Adapter for a {@code Mono} wrapping any of the other synchronous return value types */
444444
ReturnValueAdapter forMono = (result, returnType, ex) ->
445445
(result == null ?
446-
Mono.error(ex) :
446+
Mono.empty() :
447447
((Mono<?>) result).flatMap(o -> forObject.adapt(o, returnType, ex)).switchIfEmpty(Mono.error(ex)));
448448
}
449449

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,8 @@ void leaveUnresolvedViaNullReturnValue() {
102102
AnnotatedControllerExceptionResolver resolver = exceptionResolver();
103103
resolver.registerController(controller.getClass());
104104

105-
StepVerifier.create(resolver.resolveException(ex, this.environment, controller))
106-
.expectErrorSatisfies(actualEx -> assertThat(actualEx).isSameAs(ex))
107-
.verify();
105+
StepVerifier.create(resolver.resolveException(ex, this.environment, controller)).verifyComplete();
106+
StepVerifier.create(resolver.resolveException(ex, this.environment, controller)).verifyComplete();
108107
}
109108

110109
@Test

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

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@
1515
*/
1616
package org.springframework.graphql.data.method.annotation.support;
1717

18+
import java.util.Collections;
1819
import java.util.List;
1920
import java.util.concurrent.Callable;
2021
import java.util.concurrent.CompletableFuture;
2122
import java.util.concurrent.atomic.AtomicReference;
23+
import java.util.function.BiConsumer;
2224

2325
import graphql.GraphQLContext;
2426
import graphql.GraphQLError;
@@ -49,11 +51,11 @@
4951
import org.springframework.graphql.data.method.annotation.SchemaMapping;
5052
import org.springframework.graphql.data.method.annotation.SubscriptionMapping;
5153
import org.springframework.graphql.execution.BatchLoaderRegistry;
54+
import org.springframework.graphql.execution.DataFetcherExceptionResolver;
5255
import org.springframework.graphql.execution.DefaultBatchLoaderRegistry;
5356
import org.springframework.graphql.execution.ErrorType;
5457
import org.springframework.graphql.execution.SubscriptionPublisherException;
5558
import org.springframework.stereotype.Controller;
56-
import org.springframework.util.Assert;
5759

5860
import static org.assertj.core.api.Assertions.assertThat;
5961

@@ -229,6 +231,34 @@ void handleExceptionFromQuery() {
229231
assertThat(responseHelper.error(0).message()).isEqualTo("Rejected: Bad input");
230232
}
231233

234+
@Test
235+
void handleExceptionWithResolverWhenNoAnnotatedExceptionHandlerMatches() {
236+
String document = "{ " +
237+
" booksByCriteria(criteria: {author:\"Heller\"}) { " +
238+
" id" +
239+
" name" +
240+
" }" +
241+
"}";
242+
243+
DataFetcherExceptionResolver resolver = (ex, env) ->
244+
Mono.just(Collections.singletonList(
245+
GraphQLError.newError().errorType(ErrorType.INTERNAL_ERROR)
246+
.message("Rejected: " + ex.getMessage())
247+
.build()));
248+
249+
ExecutionGraphQlService service = graphQlService((configurer, setup) -> {
250+
setup.exceptionResolver(configurer.getExceptionResolver()); // First @ControllerAdvice (no match)
251+
setup.exceptionResolver(resolver); // Then resolver
252+
});
253+
254+
Mono<ExecutionGraphQlResponse> responseMono = service.execute(TestExecutionRequest.forDocument(document));
255+
256+
ResponseHelper responseHelper = ResponseHelper.forResponse(responseMono);
257+
assertThat(responseHelper.errorCount()).isEqualTo(1);
258+
assertThat(responseHelper.error(0).errorType()).isEqualTo("INTERNAL_ERROR");
259+
assertThat(responseHelper.error(0).message()).isEqualTo("Rejected: Fetch failure");
260+
}
261+
232262
@Test
233263
void handleExceptionFromSubscription() {
234264
String document = "subscription { " +
@@ -257,6 +287,10 @@ void handleExceptionFromSubscription() {
257287

258288

259289
private ExecutionGraphQlService graphQlService() {
290+
return graphQlService((configurer, setup) -> {});
291+
}
292+
293+
private ExecutionGraphQlService graphQlService(BiConsumer<AnnotatedControllerConfigurer, GraphQlSetup> consumer) {
260294
BatchLoaderRegistry registry = new DefaultBatchLoaderRegistry();
261295

262296
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
@@ -269,10 +303,10 @@ private ExecutionGraphQlService graphQlService() {
269303
configurer.setApplicationContext(context);
270304
configurer.afterPropertiesSet();
271305

272-
return GraphQlSetup.schemaResource(BookSource.schema)
273-
.runtimeWiring(configurer)
274-
.dataLoaders(registry)
275-
.toGraphQlService();
306+
GraphQlSetup setup = GraphQlSetup.schemaResource(BookSource.schema).runtimeWiring(configurer);
307+
consumer.accept(configurer, setup);
308+
309+
return setup.dataLoaders(registry).toGraphQlService();
276310
}
277311

278312

@@ -292,7 +326,10 @@ public Book bookById(@Argument Long id) {
292326

293327
@QueryMapping
294328
public List<Book> booksByCriteria(@Argument BookCriteria criteria) {
295-
Assert.isTrue(!criteria.getAuthor().equalsIgnoreCase("Fitzgerald"), "Bad input");
329+
switch (criteria.getAuthor()) {
330+
case "Fitzgerald" -> throw new IllegalArgumentException("Bad input");
331+
case "Heller" -> throw new IllegalStateException("Fetch failure");
332+
}
296333
return BookSource.findBooksByAuthor(criteria.getAuthor());
297334
}
298335

0 commit comments

Comments
 (0)