I've run into a situation where I've had to duplicate code to achieve the same ExecutionStrategy#handleDataFetchingException behavior in multiple ExecutionStrategy implementations. Right now I can only reduce this duplication by pulling the code out into a static method on a utility class.
During local development, we use a simple extension of SimpleExecutionStrategy which overrides handleDataFetchingException in order to delegate to our custom handler (which maps regular and auth exceptions to different GraphQLErrors). During local development we want our data fetching to occur serially and in a single thread for ease of debugging.
In production, we use an extension of ExecutionStrategy, which is essentially a copy of ExecutorServiceExecutionStrategy, but ensures that the Spring Request context is passed to each child thread. It also overrides handleDataFetchingException with the same exact code as above.
If graphql-java were to utilize the "composition over inheritance" pattern for this ExecutionStrategy behavior (and elsewhere?), than end-user implementations can be DRY'd up without static method calls. I think @bbakerman was bringing up a similar use case for composition in this comment on PR #481.
More concretely, this could look something like this, where we set up the wiring in our code:
@Bean
public ExecutionStrategy devExecutionStrategy() {
return new SimpleExecutionStrategy(executionExceptionHandler());
}
@Bean
public ExecutionStrategy prodExecutionStrategy() {
return new RequestContextAwareExecutionStrategy(executionExceptionHandler());
}
@Bean
public ExecutionExceptionHandler executionExceptionHandler() {
return (executionContext, fieldDef, argumentValues, path, e) -> {
if (e instanceof AuthorizationException) {
executionContext.addError(new AuthorizationError((AuthorizationException) e));
} else {
executionContext.addError(new ExceptionWhileDataFetching(e));
}
};
}And graphql-java does the following:
@PublicSpi
public abstract class ExecutionStrategy {
// ...
public ExecutionStrategy() {
this.exceptionHandler = this::handleDataFetchingException; // for backwards compatibility, if necessary
}
public ExecutionStrategy(ExecutionExceptionHandler exceptionHandler) {
this.exceptionHandler = exceptionHandler;
}
@Deprecated
protected void handleDataFetchingException(
ExecutionContext executionContext,
GraphQLFieldDefinition fieldDef,
Map<String, Object> argumentValues,
ExecutionPath path,
Exception e) {
ExceptionWhileDataFetching error = new ExceptionWhileDataFetching(path, e);
executionContext.addError(error);
log.warn(error.getMessage(), e);
}
// ...
protected ExecutionResult resolveField(ExecutionContext executionContext, ExecutionParameters parameters, List<Field> fields) {
try {
DataFetcher dataFetcher = fieldDef.getDataFetcher();
resolvedValue = dataFetcher.get(environment);
fetchCtx.onEnd(resolvedValue);
} catch (Exception e) {
fetchCtx.onEnd(e);
exceptionHandler.handle(executionContext, fieldDef, argumentValues, parameters.path(), e);
}
// ...
}
}
I've run into a situation where I've had to duplicate code to achieve the same
ExecutionStrategy#handleDataFetchingExceptionbehavior in multipleExecutionStrategyimplementations. Right now I can only reduce this duplication by pulling the code out into a static method on a utility class.During local development, we use a simple extension of
SimpleExecutionStrategywhich overrideshandleDataFetchingExceptionin order to delegate to our custom handler (which maps regular and auth exceptions to different GraphQLErrors). During local development we want our data fetching to occur serially and in a single thread for ease of debugging.In production, we use an extension of
ExecutionStrategy, which is essentially a copy ofExecutorServiceExecutionStrategy, but ensures that the Spring Request context is passed to each child thread. It also overrideshandleDataFetchingExceptionwith the same exact code as above.If
graphql-javawere to utilize the "composition over inheritance" pattern for this ExecutionStrategy behavior (and elsewhere?), than end-user implementations can be DRY'd up without static method calls. I think @bbakerman was bringing up a similar use case for composition in this comment on PR #481.More concretely, this could look something like this, where we set up the wiring in our code:
And
graphql-javadoes the following: