Skip to content

Commit e3cf655

Browse files
committed
Improve logging in ExceptionResolversExceptionHandler
- Resolved exceptions at DEBUG - Unresolved at ERROR - Consistent handling of failure from a resolver Closes gh-336
1 parent 8e7931b commit e3cf655

File tree

1 file changed

+38
-22
lines changed

1 file changed

+38
-22
lines changed

spring-graphql/src/main/java/org/springframework/graphql/execution/ExceptionResolversExceptionHandler.java

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
2626
import graphql.execution.DataFetcherExceptionHandler;
2727
import graphql.execution.DataFetcherExceptionHandlerParameters;
2828
import graphql.execution.DataFetcherExceptionHandlerResult;
29+
import graphql.execution.ExecutionId;
2930
import graphql.schema.DataFetchingEnvironment;
3031
import org.apache.commons.logging.Log;
3132
import org.apache.commons.logging.LogFactory;
@@ -67,44 +68,59 @@ public DataFetcherExceptionHandlerResult onException(DataFetcherExceptionHandler
6768

6869
@Override
6970
public CompletableFuture<DataFetcherExceptionHandlerResult> handleException(DataFetcherExceptionHandlerParameters params) {
70-
Throwable exception = getException(params);
71-
DataFetchingEnvironment environment = params.getDataFetchingEnvironment();
71+
Throwable exception = unwrapException(params);
72+
DataFetchingEnvironment env = params.getDataFetchingEnvironment();
7273
try {
73-
if (logger.isDebugEnabled()) {
74-
logger.debug("Resolving exception", exception);
75-
}
7674
return Flux.fromIterable(this.resolvers)
77-
.flatMap((resolver) -> resolver.resolveException(exception, environment))
75+
.flatMap(resolver -> resolver.resolveException(exception, env))
76+
.map(errors -> DataFetcherExceptionHandlerResult.newResult().errors(errors).build())
7877
.next()
79-
.map((errors) -> DataFetcherExceptionHandlerResult.newResult().errors(errors).build())
80-
.switchIfEmpty(Mono.fromCallable(() -> createInternalError(exception, environment)))
78+
.doOnNext(result -> logResolvedException(exception, result))
79+
.onErrorResume(resolverEx -> Mono.just(handleResolverError(resolverEx, exception, env)))
80+
.switchIfEmpty(Mono.fromCallable(() -> createInternalError(exception, env)))
8181
.contextWrite((context) -> {
82-
ContextView contextView = ReactorContextManager.getReactorContext(environment);
82+
ContextView contextView = ReactorContextManager.getReactorContext(env);
8383
return (contextView.isEmpty() ? context : context.putAll(contextView));
8484
})
8585
.toFuture();
8686
}
87-
catch (Exception ex) {
88-
if (logger.isWarnEnabled()) {
89-
logger.warn("Failed to handle " + exception.getMessage(), ex);
90-
}
91-
return CompletableFuture.completedFuture(createInternalError(exception, environment));
87+
catch (Exception resolverEx) {
88+
return CompletableFuture.completedFuture(handleResolverError(resolverEx, exception, env));
9289
}
9390
}
9491

95-
private Throwable getException(DataFetcherExceptionHandlerParameters params) {
92+
private DataFetcherExceptionHandlerResult handleResolverError(
93+
Throwable resolverException, Throwable originalException, DataFetchingEnvironment environment) {
94+
95+
if (logger.isWarnEnabled()) {
96+
logger.warn("Failure while resolving " + originalException.getMessage(), resolverException);
97+
}
98+
return createInternalError(originalException, environment);
99+
}
100+
101+
private Throwable unwrapException(DataFetcherExceptionHandlerParameters params) {
96102
Throwable ex = params.getException();
97103
return ((ex instanceof CompletionException) ? ex.getCause() : ex);
98104
}
99105

100-
private DataFetcherExceptionHandlerResult createInternalError(Throwable ex, DataFetchingEnvironment env) {
106+
private void logResolvedException(Throwable ex, DataFetcherExceptionHandlerResult result) {
107+
if (logger.isDebugEnabled()) {
108+
logger.debug("Resolved " + ex.getClass().getSimpleName() +
109+
" to GraphQL error(s): " + result.getErrors(), ex);
110+
}
111+
}
101112

102-
GraphQLError error = GraphqlErrorBuilder.newError(env)
103-
.errorType(ErrorType.INTERNAL_ERROR)
104-
.message((StringUtils.hasText(ex.getMessage()) ? ex.getMessage() : ex.getClass().getSimpleName()))
113+
private DataFetcherExceptionHandlerResult createInternalError(Throwable ex, DataFetchingEnvironment environment) {
114+
if (logger.isErrorEnabled()) {
115+
ExecutionId id = environment.getExecutionId();
116+
logger.error("Unresolved " + ex.getClass().getSimpleName() + ", executionId= " + id, ex);
117+
}
118+
return DataFetcherExceptionHandlerResult
119+
.newResult(GraphqlErrorBuilder.newError(environment)
120+
.errorType(ErrorType.INTERNAL_ERROR)
121+
.message((StringUtils.hasText(ex.getMessage()) ? ex.getMessage() : ex.getClass().getSimpleName()))
122+
.build())
105123
.build();
106-
107-
return DataFetcherExceptionHandlerResult.newResult(error).build();
108124
}
109125

110126
}

0 commit comments

Comments
 (0)