Skip to content

Commit

Permalink
Do not rewrap CompletionException in DataFetcher instrumentation
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bclozel committed Aug 28, 2023
1 parent c8d1f7c commit 9b61d28
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,15 @@ public DataFetcher<?> instrumentDataFetcher(DataFetcher<?> dataFetcher,
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand Down Expand Up @@ -168,8 +169,7 @@ void instrumentGraphQlRequestWhenDataFetchingFailure(DataFetcher<?> dataFetcher)
.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(TestExecutionRequest.forDocument(document));
ResponseHelper response = ResponseHelper.forResponse(responseMono);
Expand All @@ -190,13 +190,14 @@ void instrumentGraphQlRequestWhenDataFetchingFailure(DataFetcher<?> dataFetcher)
}

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"))),
Arguments.of((DataFetcher<?>) environment ->
CompletableFuture.failedStage(new IllegalStateException("book fetching failure")))
CompletableFuture.failedStage(new CompletionException(new IllegalStateException("book fetching failure"))))
);
}

Expand Down

0 comments on commit 9b61d28

Please sign in to comment.