Polishing in exception handling

See gh-996

Author: rstoyanchev

spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java

@@ -175,46 +175,47 @@ Mono<List<GraphQLError>> resolveException(
 			Throwable ex, DataFetchingEnvironment environment, @Nullable Object controller) {
 
 		Object controllerOrAdvice = null;
-		MethodHolder methodHolder = null;
+		MethodReturnValueAdapter methodReturnValueAdapter = null;
 		Class<?> controllerType = null;
 
 		if (controller != null) {
 			controllerType = ClassUtils.getUserClass(controller.getClass());
 			MethodResolver methodResolver = this.controllerCache.get(controllerType);
 			if (methodResolver != null) {
 				controllerOrAdvice = controller;
-				methodHolder = methodResolver.resolveMethod(ex);
+				methodReturnValueAdapter = methodResolver.resolveMethod(ex);
 			}
 			else if (logger.isWarnEnabled()) {
 				logger.warn("No registration for controller type: " + controllerType.getName());
 			}
 		}
 
-		if (methodHolder == null) {
+		if (methodReturnValueAdapter == null) {
 			for (Map.Entry<ControllerAdviceBean, MethodResolver> entry : this.controllerAdviceCache.entrySet()) {
 				ControllerAdviceBean advice = entry.getKey();
 				if (controller == null || advice.isApplicableToBeanType(controllerType)) {
-					methodHolder = entry.getValue().resolveMethod(ex);
-					if (methodHolder != null) {
+					methodReturnValueAdapter = entry.getValue().resolveMethod(ex);
+					if (methodReturnValueAdapter != null) {
 						controllerOrAdvice = advice.resolveBean();
 						break;
 					}
 				}
 			}
 		}
 
-		if (methodHolder == null) {
+		if (methodReturnValueAdapter == null) {
 			return Mono.empty();
 		}
 
-		return invokeExceptionHandler(ex, environment, controllerOrAdvice, methodHolder);
+		return invokeExceptionHandler(ex, environment, controllerOrAdvice, methodReturnValueAdapter);
 	}
 
 	private Mono<List<GraphQLError>> invokeExceptionHandler(
-			Throwable exception, DataFetchingEnvironment env, Object controllerOrAdvice, MethodHolder methodHolder) {
+			Throwable exception, DataFetchingEnvironment env, Object controllerOrAdvice,
+			MethodReturnValueAdapter methodReturnValueAdapter) {
 
 		DataFetcherHandlerMethod exceptionHandler = new DataFetcherHandlerMethod(
-				new HandlerMethod(controllerOrAdvice, methodHolder.getMethod()), this.argumentResolvers,
+				new HandlerMethod(controllerOrAdvice, methodReturnValueAdapter.getMethod()), this.argumentResolvers,
 				null, null, false);
 
 		List<Throwable> exceptions = new ArrayList<>();
@@ -236,7 +237,7 @@ private Mono<List<GraphQLError>> invokeExceptionHandler(
 
 			Object result = exceptionHandler.invoke(env, arguments);
 
-			return methodHolder.adapt(result, exception);
+			return methodReturnValueAdapter.adapt(result, exception);
 		}
 		catch (Throwable invocationEx) {
 			// Any other than the original exception (or a cause) is unintended here,
@@ -256,17 +257,17 @@ private Mono<List<GraphQLError>> invokeExceptionHandler(
 	private static final class MethodResolver {
 
 		@SuppressWarnings("DataFlowIssue")
-		private static final MethodHolder NO_MATCH =
-				new MethodHolder(ReflectionUtils.findMethod(MethodResolver.class, "noMatch"));
+		private static final MethodReturnValueAdapter NO_MATCH =
+				new MethodReturnValueAdapter(ReflectionUtils.findMethod(MethodResolver.class, "noMatch"));
 
 
-		private final Map<Class<? extends Throwable>, MethodHolder> exceptionMappings = new HashMap<>(16);
+		private final Map<Class<? extends Throwable>, MethodReturnValueAdapter> exceptionMappings = new HashMap<>(16);
 
-		private final Map<Class<? extends Throwable>, MethodHolder> resolvedExceptionCache = new ConcurrentReferenceHashMap<>(16);
+		private final Map<Class<? extends Throwable>, MethodReturnValueAdapter> resolvedExceptionCache = new ConcurrentReferenceHashMap<>(16);
 
 		MethodResolver(Map<Class<? extends Throwable>, Method> methodMap) {
 			methodMap.forEach((exceptionType, method) ->
-					this.exceptionMappings.put(exceptionType, new MethodHolder(method)));
+					this.exceptionMappings.put(exceptionType, new MethodReturnValueAdapter(method)));
 		}
 
 		/**
@@ -276,8 +277,8 @@ private static final class MethodResolver {
 		 * @return the exception handler to use, or {@code null} if no match
 		 */
 		@Nullable
-		MethodHolder resolveMethod(Throwable exception) {
-			MethodHolder method = resolveMethodByExceptionType(exception.getClass());
+		MethodReturnValueAdapter resolveMethod(Throwable exception) {
+			MethodReturnValueAdapter method = resolveMethodByExceptionType(exception.getClass());
 			if (method == null) {
 				Throwable cause = exception.getCause();
 				if (cause != null) {
@@ -288,16 +289,16 @@ MethodHolder resolveMethod(Throwable exception) {
 		}
 
 		@Nullable
-		private MethodHolder resolveMethodByExceptionType(Class<? extends Throwable> exceptionType) {
-			MethodHolder method = this.resolvedExceptionCache.get(exceptionType);
+		private MethodReturnValueAdapter resolveMethodByExceptionType(Class<? extends Throwable> exceptionType) {
+			MethodReturnValueAdapter method = this.resolvedExceptionCache.get(exceptionType);
 			if (method == null) {
 				method = getMappedMethod(exceptionType);
 				this.resolvedExceptionCache.put(exceptionType, method);
 			}
 			return (method != NO_MATCH) ? method : null;
 		}
 
-		private MethodHolder getMappedMethod(Class<? extends Throwable> exceptionType) {
+		private MethodReturnValueAdapter getMappedMethod(Class<? extends Throwable> exceptionType) {
 			List<Class<? extends Throwable>> matches = new ArrayList<>();
 			for (Class<? extends Throwable> mappedException : this.exceptionMappings.keySet()) {
 				if (mappedException.isAssignableFrom(exceptionType)) {
@@ -323,17 +324,17 @@ private void noMatch() {
 
 
 	/**
-	 * Container for an exception handler method, and an adapter for its return values.
+	 * Helps to adapt the return value of an exception handler method.
 	 */
-	private static class MethodHolder {
+	private static class MethodReturnValueAdapter {
 
 		private final Method method;
 
 		private final MethodParameter returnType;
 
 		private final ReturnValueAdapter adapter;
 
-		MethodHolder(Method method) {
+		MethodReturnValueAdapter(Method method) {
 			Assert.notNull(method, "Method is required");
 			this.method = method;
 			this.returnType = new MethodParameter(method, -1);

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

@@ -96,16 +96,15 @@ public final Mono<List<GraphQLError>> resolveException(Throwable ex, DataFetchin
 	@Nullable
 	@SuppressWarnings("deprecation")
 	private List<GraphQLError> resolveInternal(Throwable exception, DataFetchingEnvironment env) {
-		if (!this.threadLocalContextAware) {
-			return resolveToMultipleErrors(exception, env);
-		}
 		try {
-			return ContextSnapshot.captureFrom(env.getGraphQlContext())
-					.wrap(() -> resolveToMultipleErrors(exception, env))
-					.call();
+			return (this.threadLocalContextAware) ?
+					ContextSnapshot.captureFrom(env.getGraphQlContext())
+							.wrap(() -> resolveToMultipleErrors(exception, env))
+							.call() :
+					resolveToMultipleErrors(exception, env);
 		}
 		catch (Exception ex2) {
-			this.logger.warn("Failed to resolve " + exception, ex2);
+			this.logger.warn("Failure while resolving " + exception.getMessage(), ex2);
 			return null;
 		}
 	}