Spring/spring-graphql
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Do not rewrap CompletionException in DataFetcher instrumentation

Browse Source 
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-780
This commit is contained in:
Brian Clozel
2023-08-28 12:25:38 +02:00
parent 1032f99b6d
commit 32ed3e967e
2 changed files with 17 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationInstrumentation.java
View File

@@ -147,9 +147,15 @@ public class GraphQlObservationInstrumentation extends SimplePerformantInstrumen
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;

15
spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java
View File

@@ -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<ExecutionGraphQlResponse> responseMono = graphQlSetup
.exceptionResolver(resolver)
.queryFetcher("bookById", env ->
CompletableFuture.failedStage(new IllegalStateException("book fetching failure")))
.queryFetcher("bookById", dataFetcher)
.toGraphQlService()
.execute(document);
ResponseHelper response = ResponseHelper.forResponse(responseMono);
@@ -190,13 +190,14 @@ class GraphQlObservationInstrumentationTests {
}
static Stream<Arguments> failureDataFetchers() {
DataFetcher<Book> 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"))))
);
}