From 9b61d2808e075031147376f644f154db41facc0c Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 28 Aug 2023 12:25:38 +0200 Subject: [PATCH] Do not rewrap CompletionException in DataFetcher instrumentation This commit ensures that, when an instrumented DataFetcher returns a `CompletionException`, we do not re-wrap it with the same exception type. This aligns with the behavior enforced in the JDK `CompletableFuture`. Fixes gh-784 --- .../GraphQlObservationInstrumentation.java | 12 +++++++++--- .../GraphQlObservationInstrumentationTests.java | 15 ++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationInstrumentation.java b/spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationInstrumentation.java index 2c6ee1db..bc2cfa32 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationInstrumentation.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationInstrumentation.java @@ -147,9 +147,15 @@ public class GraphQlObservationInstrumentation extends SimpleInstrumentation { return completion.handle((result, error) -> { observationContext.setValue(result); if (error != null) { - dataFetcherObservation.error(error); - dataFetcherObservation.stop(); - throw new CompletionException(error); + if (error instanceof CompletionException completionException) { + dataFetcherObservation.error(error.getCause()); + dataFetcherObservation.stop(); + throw completionException; + } else { + dataFetcherObservation.error(error); + dataFetcherObservation.stop(); + throw new CompletionException(error); + } } dataFetcherObservation.stop(); return result; diff --git a/spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java b/spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java index 1634031b..962b5f1e 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java @@ -37,6 +37,7 @@ import org.springframework.graphql.execution.ErrorType; import reactor.core.publisher.Mono; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; @@ -168,8 +169,7 @@ class GraphQlObservationInstrumentationTests { .errorType(ErrorType.BAD_REQUEST).build()); Mono responseMono = graphQlSetup .exceptionResolver(resolver) - .queryFetcher("bookById", env -> - CompletableFuture.failedStage(new IllegalStateException("book fetching failure"))) + .queryFetcher("bookById", dataFetcher) .toGraphQlService() .execute(TestExecutionRequest.forDocument(document)); ResponseHelper response = ResponseHelper.forResponse(responseMono); @@ -190,13 +190,14 @@ class GraphQlObservationInstrumentationTests { } static Stream failureDataFetchers() { - DataFetcher bookDataFetcher = environment -> { - throw new IllegalStateException("book fetching failure"); - }; return Stream.of( - Arguments.of(bookDataFetcher), + Arguments.of((DataFetcher) environment -> { + throw new IllegalStateException("book fetching failure"); + }), Arguments.of((DataFetcher) environment -> - CompletableFuture.failedStage(new IllegalStateException("book fetching failure"))) + CompletableFuture.failedStage(new IllegalStateException("book fetching failure"))), + Arguments.of((DataFetcher) environment -> + CompletableFuture.failedStage(new CompletionException(new IllegalStateException("book fetching failure")))) ); }