Skip to content

Commit 018e798

Browse files
bbakermanbbakerman
committed
Breaking - changes the error handler such that is returns a value rather than rely on side effects
1 parent 864d381 commit 018e798

9 files changed

Lines changed: 162 additions & 89 deletions

‎src/main/java/graphql/execution/DataFetcherExceptionHandler.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,22 @@
11
package graphql.execution;
22

3-
import graphql.GraphQLError;
43
import graphql.PublicSpi;
54
import graphql.schema.DataFetcher;
65
import graphql.schema.DataFetchingEnvironment;
76

8-
import java.util.function.Consumer;
9-
107
/**
118
* This is called when an exception is thrown during {@link graphql.schema.DataFetcher#get(DataFetchingEnvironment)} execution
129
*/
1310
@PublicSpi
14-
public interface DataFetcherExceptionHandler extends Consumer<DataFetcherExceptionHandlerParameters> {
11+
public interface DataFetcherExceptionHandler {
1512

1613
/**
1714
* When an exception during a call to a {@link DataFetcher} then this handler
1815
* is called back to shape the error that should be placed in the list of errors
19-
* via {@link ExecutionContext#addError(GraphQLError)}
2016
*
2117
* @param handlerParameters the parameters to this callback
18+
*
19+
* @return a result that can contain custom formatted {@link graphql.GraphQLError}s
2220
*/
23-
@Override
24-
void accept(DataFetcherExceptionHandlerParameters handlerParameters);
21+
DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandlerParameters handlerParameters);
2522
}

‎src/main/java/graphql/execution/DataFetcherExceptionHandlerParameters.java‎

Lines changed: 16 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package graphql.execution;
22

33
import graphql.PublicApi;
4+
import graphql.language.SourceLocation;
45
import graphql.schema.DataFetchingEnvironment;
56
import graphql.schema.GraphQLFieldDefinition;
67

@@ -12,105 +13,65 @@
1213
@PublicApi
1314
public class DataFetcherExceptionHandlerParameters {
1415

15-
private final ExecutionContext executionContext;
1616
private final DataFetchingEnvironment dataFetchingEnvironment;
17-
private final MergedField field;
18-
private final GraphQLFieldDefinition fieldDefinition;
19-
private final Map<String, Object> argumentValues;
20-
private final ExecutionPath path;
2117
private final Throwable exception;
2218

23-
public DataFetcherExceptionHandlerParameters(ExecutionContext executionContext, DataFetchingEnvironment dataFetchingEnvironment, MergedField field, GraphQLFieldDefinition fieldDefinition, Map<String, Object> argumentValues, ExecutionPath path, Throwable exception) {
24-
this.executionContext = executionContext;
25-
this.dataFetchingEnvironment = dataFetchingEnvironment;
26-
this.field = field;
27-
this.fieldDefinition = fieldDefinition;
28-
this.argumentValues = argumentValues;
29-
this.path = path;
30-
this.exception = exception;
19+
private DataFetcherExceptionHandlerParameters(Builder builder) {
20+
this.exception = builder.exception;
21+
this.dataFetchingEnvironment = builder.dataFetchingEnvironment;
3122
}
3223

33-
public static Builder newExceptionParameters() {
34-
return new Builder();
24+
public Throwable getException() {
25+
return exception;
3526
}
3627

37-
public ExecutionContext getExecutionContext() {
38-
return executionContext;
28+
public ExecutionPath getPath() {
29+
return dataFetchingEnvironment.getExecutionStepInfo().getPath();
3930
}
4031

4132
public DataFetchingEnvironment getDataFetchingEnvironment() {
4233
return dataFetchingEnvironment;
4334
}
4435

4536
public MergedField getField() {
46-
return field;
37+
return dataFetchingEnvironment.getMergedField();
4738
}
4839

4940
public GraphQLFieldDefinition getFieldDefinition() {
50-
return fieldDefinition;
41+
return dataFetchingEnvironment.getFieldDefinition();
5142
}
5243

5344
public Map<String, Object> getArgumentValues() {
54-
return argumentValues;
45+
return dataFetchingEnvironment.getArguments();
5546
}
5647

57-
public ExecutionPath getPath() {
58-
return path;
48+
public SourceLocation getSourceLocation() {
49+
return getField().getSingleField().getSourceLocation();
5950
}
6051

61-
public Throwable getException() {
62-
return exception;
52+
public static Builder newExceptionParameters() {
53+
return new Builder();
6354
}
6455

6556
public static class Builder {
66-
ExecutionContext executionContext;
6757
DataFetchingEnvironment dataFetchingEnvironment;
68-
MergedField field;
69-
GraphQLFieldDefinition fieldDefinition;
70-
Map<String, Object> argumentValues;
71-
ExecutionPath path;
7258
Throwable exception;
7359

7460
private Builder() {
7561
}
7662

77-
public Builder executionContext(ExecutionContext executionContext) {
78-
this.executionContext = executionContext;
79-
return this;
80-
}
81-
8263
public Builder dataFetchingEnvironment(DataFetchingEnvironment dataFetchingEnvironment) {
8364
this.dataFetchingEnvironment = dataFetchingEnvironment;
8465
return this;
8566
}
8667

87-
public Builder field(MergedField field) {
88-
this.field = field;
89-
return this;
90-
}
91-
92-
public Builder fieldDefinition(GraphQLFieldDefinition fieldDefinition) {
93-
this.fieldDefinition = fieldDefinition;
94-
return this;
95-
}
96-
97-
public Builder argumentValues(Map<String, Object> argumentValues) {
98-
this.argumentValues = argumentValues;
99-
return this;
100-
}
101-
102-
public Builder path(ExecutionPath path) {
103-
this.path = path;
104-
return this;
105-
}
106-
10768
public Builder exception(Throwable exception) {
10869
this.exception = exception;
10970
return this;
11071
}
11172

11273
public DataFetcherExceptionHandlerParameters build() {
113-
return new DataFetcherExceptionHandlerParameters(executionContext, dataFetchingEnvironment, field, fieldDefinition, argumentValues, path, exception);
74+
return new DataFetcherExceptionHandlerParameters(this);
11475
}
11576
}
11677
}

‎src/main/java/graphql/execution/DataFetcherExceptionHandlerResult.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package graphql.execution;
2+
3+
import graphql.GraphQLError;
4+
import graphql.PublicApi;
5+
6+
import java.util.ArrayList;
7+
import java.util.Collections;
8+
import java.util.List;
9+
10+
import static graphql.Assert.assertNotNull;
11+
12+
/**
13+
* The result object for {@link graphql.execution.DataFetcherExceptionHandler}s
14+
*/
15+
@PublicApi
16+
public class DataFetcherExceptionHandlerResult {
17+
18+
private final List<GraphQLError> errors;
19+
20+
private DataFetcherExceptionHandlerResult(Builder builder) {
21+
this.errors = builder.errors;
22+
}
23+
24+
public List<GraphQLError> getErrors() {
25+
return errors;
26+
}
27+
28+
public static Builder newResult() {
29+
return new Builder();
30+
}
31+
32+
public static class Builder {
33+
34+
private final List<GraphQLError> errors = new ArrayList<>();
35+
36+
public Builder errors(List<GraphQLError> errors) {
37+
this.errors.addAll(assertNotNull(errors));
38+
return this;
39+
}
40+
41+
public Builder error(GraphQLError error) {
42+
return errors(Collections.singletonList(assertNotNull(error)));
43+
}
44+
45+
public DataFetcherExceptionHandlerResult build() {
46+
return new DataFetcherExceptionHandlerResult(this);
47+
}
48+
49+
}
50+
51+
}

‎src/main/java/graphql/execution/ExecutionStrategy.java‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ protected CompletableFuture<FetchedValue> fetchField(ExecutionContext executionC
270270
.handle((result, exception) -> {
271271
fetchCtx.onCompleted(result, exception);
272272
if (exception != null) {
273-
handleFetchingException(executionContext, parameters, field, fieldDef, argumentValues, environment, exception);
273+
handleFetchingException(executionContext, parameters, environment, exception);
274274
return null;
275275
} else {
276276
return result;
@@ -311,22 +311,15 @@ FetchedValue unboxPossibleDataFetcherResult(ExecutionContext executionContext,
311311

312312
private void handleFetchingException(ExecutionContext executionContext,
313313
ExecutionStrategyParameters parameters,
314-
MergedField field,
315-
GraphQLFieldDefinition fieldDef,
316-
Map<String, Object> argumentValues,
317314
DataFetchingEnvironment environment,
318315
Throwable e) {
319316
DataFetcherExceptionHandlerParameters handlerParameters = DataFetcherExceptionHandlerParameters.newExceptionParameters()
320-
.executionContext(executionContext)
321317
.dataFetchingEnvironment(environment)
322-
.argumentValues(argumentValues)
323-
.field(field)
324-
.fieldDefinition(fieldDef)
325-
.path(parameters.getPath())
326318
.exception(e)
327319
.build();
328320

329-
dataFetcherExceptionHandler.accept(handlerParameters);
321+
DataFetcherExceptionHandlerResult handlerResult = dataFetcherExceptionHandler.onException(handlerParameters);
322+
handlerResult.getErrors().forEach(executionContext::addError);
330323

331324
parameters.deferredErrorSupport().onFetchingException(parameters, e);
332325
}

‎src/main/java/graphql/execution/SimpleDataFetcherExceptionHandler.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,18 @@
1010
* into the error collection
1111
*/
1212
public class SimpleDataFetcherExceptionHandler implements DataFetcherExceptionHandler {
13+
1314
private static final Logger log = LoggerFactory.getLogger(SimpleDataFetcherExceptionHandler.class);
1415

1516
@Override
16-
public void accept(DataFetcherExceptionHandlerParameters handlerParameters) {
17+
public DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandlerParameters handlerParameters) {
1718
Throwable exception = handlerParameters.getException();
18-
SourceLocation sourceLocation = handlerParameters.getField().getSingleField().getSourceLocation();
19+
SourceLocation sourceLocation = handlerParameters.getSourceLocation();
1920
ExecutionPath path = handlerParameters.getPath();
2021

2122
ExceptionWhileDataFetching error = new ExceptionWhileDataFetching(path, exception, sourceLocation);
22-
handlerParameters.getExecutionContext().addError(error);
2323
log.warn(error.getMessage(), exception);
24+
25+
return DataFetcherExceptionHandlerResult.newResult().error(error).build();
2426
}
2527
}

‎src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
import graphql.Assert;
44
import graphql.ExecutionResult;
55
import graphql.ExecutionResultImpl;
6+
import graphql.GraphQLError;
67
import graphql.PublicApi;
78
import graphql.TrivialDataFetcher;
89
import graphql.execution.Async;
910
import graphql.execution.DataFetcherExceptionHandler;
1011
import graphql.execution.DataFetcherExceptionHandlerParameters;
12+
import graphql.execution.DataFetcherExceptionHandlerResult;
1113
import graphql.execution.ExecutionContext;
1214
import graphql.execution.ExecutionPath;
1315
import graphql.execution.ExecutionStepInfo;
@@ -283,25 +285,23 @@ private CompletableFuture<FetchedValues> fetchData(ExecutionContext executionCon
283285
return fetchedValue
284286
.thenApply((result) -> assertResult(parentResults, result))
285287
.whenComplete(fetchCtx::onCompleted)
286-
.handle(handleResult(executionContext, parameters, parentResults, fields, fieldDef, argumentValues, environment));
288+
.handle(handleResult(executionContext, parameters, parentResults, environment));
287289
}
288290

289-
private BiFunction<List<Object>, Throwable, FetchedValues> handleResult(ExecutionContext executionContext, ExecutionStrategyParameters parameters, List<MapOrList> parentResults, MergedField fields, GraphQLFieldDefinition fieldDef, Map<String, Object> argumentValues, DataFetchingEnvironment environment) {
291+
private BiFunction<List<Object>, Throwable, FetchedValues> handleResult(ExecutionContext executionContext, ExecutionStrategyParameters parameters, List<MapOrList> parentResults, DataFetchingEnvironment environment) {
290292
return (result, exception) -> {
291293
if (exception != null) {
292294
if (exception instanceof CompletionException) {
293295
exception = exception.getCause();
294296
}
295297
DataFetcherExceptionHandlerParameters handlerParameters = DataFetcherExceptionHandlerParameters.newExceptionParameters()
296-
.executionContext(executionContext)
297298
.dataFetchingEnvironment(environment)
298-
.argumentValues(argumentValues)
299-
.field(fields)
300-
.fieldDefinition(fieldDef)
301-
.path(parameters.getPath())
302299
.exception(exception)
303300
.build();
304-
dataFetcherExceptionHandler.accept(handlerParameters);
301+
302+
DataFetcherExceptionHandlerResult handlerResult = dataFetcherExceptionHandler.onException(handlerParameters);
303+
handlerResult.getErrors().forEach(executionContext::addError);
304+
305305
result = Collections.nCopies(parentResults.size(), null);
306306
}
307307
List<Object> values = result;

‎src/test/groovy/graphql/execution/DataFetcherExceptionHandlerTest.groovy‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package graphql.execution
2+
3+
import graphql.ErrorType
4+
import graphql.ExecutionInput
5+
import graphql.GraphQLError
6+
import graphql.TestUtil
7+
import graphql.language.SourceLocation
8+
import graphql.schema.DataFetcher
9+
import spock.lang.Specification
10+
11+
class DataFetcherExceptionHandlerTest extends Specification {
12+
13+
class CustomError implements GraphQLError {
14+
String msg
15+
SourceLocation sourceLocation
16+
17+
CustomError(String msg, SourceLocation sourceLocation) {
18+
this.msg = msg
19+
this.sourceLocation = sourceLocation
20+
}
21+
22+
@Override
23+
String getMessage() {
24+
return msg
25+
}
26+
27+
@Override
28+
List<SourceLocation> getLocations() {
29+
return [sourceLocation]
30+
}
31+
32+
@Override
33+
ErrorType getErrorType() {
34+
return ErrorType.DataFetchingException
35+
}
36+
}
37+
38+
def "integration test to prove custom error handle can be made"() {
39+
DataFetcherExceptionHandler handler = {
40+
params -> DataFetcherExceptionHandlerResult.newResult().error(new CustomError("The thing went " + params.getException().getMessage(), params.getSourceLocation())).build()
41+
}
42+
43+
def dataFetchers = [
44+
Query: [field: { env -> throw new RuntimeException("BANG") } as DataFetcher]
45+
]
46+
47+
def graphQL = TestUtil.graphQL('''
48+
type Query {
49+
field : String
50+
}
51+
''', dataFetchers)
52+
.queryExecutionStrategy(new AsyncExecutionStrategy(handler))
53+
.build()
54+
when:
55+
def result = graphQL.execute(ExecutionInput.newExecutionInput().query(' { field }'))
56+
then:
57+
!result.errors.isEmpty()
58+
result.errors[0].message == "The thing went BANG"
59+
}
60+
}

0 commit comments

Comments
 (0)