Skip to content

Removing the deprecated DataFetcherExceptionHandler sync method #3232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bbakerman merged 2 commits into from
May 30, 2023
Merged

Removing the deprecated DataFetcherExceptionHandler sync method #3232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
86d3e6a
Removing the deprecated DataFetcherExceptionHandler sync method
bbakerman May 29, 2023
1021219
require them to implement the method
bbakerman May 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 1 addition & 22 deletions src/main/java/graphql/execution/DataFetcherExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,6 @@
@PublicSpi
public interface DataFetcherExceptionHandler {

/**
* When an exception occurs during a call to a {@link DataFetcher} then this handler
* is called to shape the errors that should be placed in the {@link ExecutionResult#getErrors()}
* list of errors.
*
* @param handlerParameters the parameters to this callback
*
* @return a result that can contain custom formatted {@link graphql.GraphQLError}s
*
* @deprecated use {@link #handleException(DataFetcherExceptionHandlerParameters)} instead which as an asynchronous
* version
*/
@Deprecated
@DeprecatedAt("2021-06-23")
default DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandlerParameters handlerParameters) {
return SimpleDataFetcherExceptionHandler.defaultImpl.onException(handlerParameters);
}

/**
* When an exception occurs during a call to a {@link DataFetcher} then this handler
* is called to shape the errors that should be placed in the {@link ExecutionResult#getErrors()}
Expand All @@ -41,8 +23,5 @@ default DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandle
*
* @return a result that can contain custom formatted {@link graphql.GraphQLError}s
*/
default CompletableFuture<DataFetcherExceptionHandlerResult> handleException(DataFetcherExceptionHandlerParameters handlerParameters) {
DataFetcherExceptionHandlerResult result = onException(handlerParameters);
return CompletableFuture.completedFuture(result);
}
CompletableFuture<DataFetcherExceptionHandlerResult> handleException(DataFetcherExceptionHandlerParameters handlerParameters);
Copy link
Copy Markdown
Member Author

@bbakerman bbakerman May 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You MUST now implement the method if you make your own. Its no longer defaulted

}
5 changes: 2 additions & 3 deletions src/main/java/graphql/execution/SimpleDataFetcherExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ public class SimpleDataFetcherExceptionHandler implements DataFetcherExceptionHa

static final SimpleDataFetcherExceptionHandler defaultImpl = new SimpleDataFetcherExceptionHandler();

@Override
public DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandlerParameters handlerParameters) {
private DataFetcherExceptionHandlerResult handleExceptionImpl(DataFetcherExceptionHandlerParameters handlerParameters) {
Throwable exception = unwrap(handlerParameters.getException());
SourceLocation sourceLocation = handlerParameters.getSourceLocation();
ResultPath path = handlerParameters.getPath();
Expand All @@ -34,7 +33,7 @@ public DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandler

@Override
public CompletableFuture<DataFetcherExceptionHandlerResult> handleException(DataFetcherExceptionHandlerParameters handlerParameters) {
return CompletableFuture.completedFuture(onException(handlerParameters));
return CompletableFuture.completedFuture(handleExceptionImpl(handlerParameters));
}

/**
Expand Down
38 changes: 9 additions & 29 deletions src/test/groovy/graphql/execution/SimpleDataFetcherExceptionHandlerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,49 +21,29 @@ class SimpleDataFetcherExceptionHandlerTest extends Specification {
def "will wrap general exceptions"() {
when:
def handlerParameters = mkParams(new RuntimeException("RTE"))
def result = handler.onException(handlerParameters)
def result = handler.handleException(handlerParameters)

then:
result.errors[0] instanceof ExceptionWhileDataFetching
result.errors[0].getMessage().contains("RTE")
result.join().errors[0] instanceof ExceptionWhileDataFetching
result.join().errors[0].getMessage().contains("RTE")
}

def "can unwrap certain exceptions"() {
when:
def result = handler.onException(mkParams(new CompletionException(new RuntimeException("RTE"))))
def result = handler.handleException(mkParams(new CompletionException(new RuntimeException("RTE"))))

then:
result.errors[0] instanceof ExceptionWhileDataFetching
result.errors[0].getMessage().contains("RTE")
result.join().errors[0] instanceof ExceptionWhileDataFetching
result.join().errors[0].getMessage().contains("RTE")
}

def "wont unwrap other exceptions"() {
when:
def result = handler.onException(mkParams(new RuntimeException("RTE",new RuntimeException("BANG"))))
def result = handler.handleException(mkParams(new RuntimeException("RTE",new RuntimeException("BANG"))))

then:
result.errors[0] instanceof ExceptionWhileDataFetching
! result.errors[0].getMessage().contains("BANG")
}

static class MyHandler implements DataFetcherExceptionHandler {}

def "a class can work without implementing anything"() {
when:
DataFetcherExceptionHandler handler = new MyHandler()
def handlerParameters = mkParams(new RuntimeException("RTE"))
def result = handler.onException(handlerParameters) // Retain deprecated method for test coverage

then:
result.errors[0] instanceof ExceptionWhileDataFetching
result.errors[0].getMessage().contains("RTE")

when:
def resultCF = handler.handleException(handlerParameters)

then:
resultCF.join().errors[0] instanceof ExceptionWhileDataFetching
resultCF.join().errors[0].getMessage().contains("RTE")
result.join().errors[0] instanceof ExceptionWhileDataFetching
! result.join().errors[0].getMessage().contains("BANG")
}

private static DataFetcherExceptionHandlerParameters mkParams(Exception exception) {
Expand Down
7 changes: 6 additions & 1 deletion src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderHangingTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,13 @@ class DataLoaderHangingTest extends Specification {
"""

DataFetcherExceptionHandler customExceptionHandlerThatThrows = new DataFetcherExceptionHandler() {

@Override
DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandlerParameters handlerParameters) { // Retain for test coverage, intentionally using sync version.
CompletableFuture<DataFetcherExceptionHandlerResult> handleException(DataFetcherExceptionHandlerParameters handlerParameters) {
//
// this is a weird test case - its not actually handling the exception - its a test
// case where the handler code itself throws an exception during the handling
// and that will not stop the DataLoader from being dispatched
throw handlerParameters.exception
}
}
Expand Down
9 changes: 6 additions & 3 deletions src/test/groovy/readme/ExecutionExamples.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,16 @@ private void exceptionHandler() {
DataFetcherExceptionHandler handler = new DataFetcherExceptionHandler() {

@Override
public DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandlerParameters handlerParameters) {
public CompletableFuture<DataFetcherExceptionHandlerResult> handleException(DataFetcherExceptionHandlerParameters handlerParameters) {
//
// do your custom handling here. The parameters have all you need
GraphQLError buildCustomError = buildCustomError(handlerParameters);

return DataFetcherExceptionHandlerResult.newResult()
.error(buildCustomError).build();
DataFetcherExceptionHandlerResult exceptionResult = DataFetcherExceptionHandlerResult
.newResult()
.error(buildCustomError)
.build();
return CompletableFuture.completedFuture(exceptionResult);
}
};
ExecutionStrategy executionStrategy = new AsyncExecutionStrategy(handler);
Expand Down