Explicit coverage of root vs cause exception matching in MVC ref docs

jhoeller · jhoeller · commit f2cc70ecf903 · 2018-05-07T23:18:33.000+02:00
Issue: SPR-16743 (cherry picked from commit a200df6)
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/AnnotationDrivenBeanDefinitionParser.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/AnnotationDrivenBeanDefinitionParser.java
@@ -27,7 +27,6 @@
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.BeanDefinitionHolder;
-import org.springframework.beans.factory.config.BeanReference;
 import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.parsing.BeanComponentDefinition;
 import org.springframework.beans.factory.parsing.CompositeComponentDefinition;
@@ -60,13 +59,10 @@
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
-import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.springframework.web.HttpRequestHandler;
 import org.springframework.web.accept.ContentNegotiationManager;
 import org.springframework.web.accept.ContentNegotiationManagerFactoryBean;
-import org.springframework.web.bind.annotation.ExceptionHandler;
-import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
 import org.springframework.web.bind.support.WebArgumentResolver;
 import org.springframework.web.method.support.CompositeUriComponentsContributor;
@@ -117,10 +113,10 @@
  *
  * <p>This class registers the following {@link HandlerExceptionResolver}s:
  * <ul>
- * <li>{@link ExceptionHandlerExceptionResolver} for handling exceptions
- * through @{@link ExceptionHandler} methods.
+ * <li>{@link ExceptionHandlerExceptionResolver} for handling exceptions through
+ * {@link org.springframework.web.bind.annotation.ExceptionHandler} methods.
  * <li>{@link ResponseStatusExceptionResolver} for exceptions annotated
- * with @{@link ResponseStatus}.
+ * with {@link org.springframework.web.bind.annotation.ResponseStatus}.
  * <li>{@link DefaultHandlerExceptionResolver} for resolving known Spring
  * exception types
  * </ul>
@@ -664,27 +660,6 @@ private ManagedList<Object> extractBeanSubElements(Element parentElement, Parser
 		return list;
 	}
 
-    private ManagedList<BeanReference> extractBeanRefSubElements(Element parentElement, ParserContext parserContext){
-        ManagedList<BeanReference> list = new ManagedList<>();
-        list.setSource(parserContext.extractSource(parentElement));
-        for (Element refElement : DomUtils.getChildElementsByTagName(parentElement, "ref")) {
-            BeanReference reference;
-            if (StringUtils.hasText("bean")) {
-                reference = new RuntimeBeanReference(refElement.getAttribute("bean"),false);
-                list.add(reference);
-            }
-			else if (StringUtils.hasText("parent")){
-                reference = new RuntimeBeanReference(refElement.getAttribute("parent"),true);
-                list.add(reference);
-            }
-			else {
-                parserContext.getReaderContext().error("'bean' or 'parent' attribute is required for <ref> element",
-                        parserContext.extractSource(parentElement));
-            }
-        }
-        return list;
-    }
-
 
 	/**
 	 * A FactoryBean for a CompositeUriComponentsContributor that obtains the
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java
@@ -67,8 +67,6 @@
 import org.springframework.web.HttpRequestHandler;
 import org.springframework.web.accept.ContentNegotiationManager;
 import org.springframework.web.bind.WebDataBinder;
-import org.springframework.web.bind.annotation.ExceptionHandler;
-import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
 import org.springframework.web.context.ServletContextAware;
 import org.springframework.web.cors.CorsConfiguration;
@@ -136,10 +134,10 @@
  * <p>Registers a {@link HandlerExceptionResolverComposite} with this chain of
  * exception resolvers:
  * <ul>
- * <li>{@link ExceptionHandlerExceptionResolver} for handling exceptions
- * through @{@link ExceptionHandler} methods.
- * <li>{@link ResponseStatusExceptionResolver} for exceptions annotated
- * with @{@link ResponseStatus}.
+ * <li>{@link ExceptionHandlerExceptionResolver} for handling exceptions through
+ * {@link org.springframework.web.bind.annotation.ExceptionHandler} methods.
+ * <li>{@link ResponseStatusExceptionResolver} for exceptions annotated with
+ * {@link org.springframework.web.bind.annotation.ResponseStatus}.
  * <li>{@link DefaultHandlerExceptionResolver} for resolving known Spring
  * exception types
  * </ul>
@@ -926,12 +924,11 @@ protected void extendHandlerExceptionResolvers(List<HandlerExceptionResolver> ex
 	 * A method available to subclasses for adding default {@link HandlerExceptionResolver}s.
 	 * <p>Adds the following exception resolvers:
 	 * <ul>
-	 * <li>{@link ExceptionHandlerExceptionResolver}
-	 * for handling exceptions through @{@link ExceptionHandler} methods.
-	 * <li>{@link ResponseStatusExceptionResolver}
-	 * for exceptions annotated with @{@link ResponseStatus}.
-	 * <li>{@link DefaultHandlerExceptionResolver}
-	 * for resolving known Spring exception types
+	 * <li>{@link ExceptionHandlerExceptionResolver} for handling exceptions through
+	 * {@link org.springframework.web.bind.annotation.ExceptionHandler} methods.
+	 * <li>{@link ResponseStatusExceptionResolver} for exceptions annotated with
+	 * {@link org.springframework.web.bind.annotation.ResponseStatus}.
+	 * <li>{@link DefaultHandlerExceptionResolver} for resolving known Spring exception types
 	 * </ul>
 	 */
 	protected final void addDefaultHandlerExceptionResolvers(List<HandlerExceptionResolver> exceptionResolvers) {
diff --git a/src/docs/asciidoc/web/webflux.adoc b/src/docs/asciidoc/web/webflux.adoc
@@ -2406,33 +2406,31 @@ controller-specific ``Formatter``'s:
 		public ResponseEntity<String> handle(IOException ex) {
 			// ...
 		}
-
 	}
 ----
 
-The annotation can list the exception types to match. Or simply declare the target
-exception as a method argument as shown above. When multiple exception methods match,
-a root exception match is generally preferred to a cause exception match. More formally
-the `ExceptionDepthComparator` is used to sort exceptions based on their depth from the
-thrown exception type.
+The exception may match against a top-level exception being propagated (i.e. a direct
+`IOException` thrown), or against the immediate cause within a top-level wrapper exception
+(e.g. an `IOException` wrapped inside an `IllegalStateException`).
 
-In a multi-`@ControllerAdvice` arrangement, please declare your primary root exception
-mappings on a `@ControllerAdvice` prioritized with a corresponding order. While a root
-exception match is preferred to a cause, this is mainly among the methods of a given
-controller or `@ControllerAdvice`. That means a cause match on a higher-priority
-`@ControllerAdvice` is preferred to any match (e.g. root) on a lower-priority
-`@ControllerAdvice`.
+For matching exception types, preferably declare the target exception as a method argument
+as shown above. Alternatively, the annotation declaration may narrow the exception types to
+match. We generally recommend to be as specific as possible in the argument signature and to
+declare your primary root exception mappings on a `@ControllerAdvice` prioritized with a
+corresponding order. See <<web.adoc#mvc-ann-exceptionhandler,the MVC section>> for details.
+
+[NOTE]
+====
+An `@ExceptionHandler` method in WebFlux supports the same method arguments and
+return values as an `@RequestMapping` method, with the exception of request body
+and `@ModelAttribute` related method arguments.
+====
 
 Support for `@ExceptionHandler` methods in Spring WebFlux is provided by the
 `HandlerAdapter` for `@RequestMapping` methods. See <<webflux-dispatcher-exceptions>>
 under the `DispatcherHandler` section for more details.
 
 
-An `@ExceptionHandler` method in WebFlux supports the same method arguments and return
-values as an `@RequestMapping` method does with the exception of request body and
-`@ModelAttribute` related method arguments.
-
-
 [[webflux-ann-rest-exceptions]]
 ==== REST API exceptions
 [.small]#<<web.adoc#mvc-ann-rest-exceptions,Same in Spring MVC>>#
diff --git a/src/docs/asciidoc/web/webmvc.adoc b/src/docs/asciidoc/web/webmvc.adoc
@@ -481,7 +481,6 @@ initialization parameters ( `init-param` elements) to the Servlet declaration in
   Note that if <<mvc-default-servlet-handler,default servlet handling>>  is
   also configured, then unresolved requests are always forwarded to the default servlet
   and a 404 would never be raised.
-
 |===
 
 
@@ -2830,22 +2829,75 @@ controller-specific ``Formatter``'s:
 		public ResponseEntity<String> handle(IOException ex) {
 			// ...
 		}
+	}
+----
+
+The exception may match against a top-level exception being propagated (i.e. a direct
+`IOException` thrown), or against the immediate cause within a top-level wrapper exception
+(e.g. an `IOException` wrapped inside an `IllegalStateException`).
+
+For matching exception types, preferably declare the target exception as a method argument
+as shown above. When multiple exception methods match, a root exception match is generally
+preferred to a cause exception match. More specifically, the `ExceptionDepthComparator` is
+used to sort exceptions based on their depth from the thrown exception type.
+
+Alternatively, the annotation declaration may narrow the exception types to match:
+
+[source,java,indent=0]
+[subs="verbatim,quotes"]
+----
+	@ExceptionHandler({FileSystemException.class, RemoteException.class})
+	public ResponseEntity<String> handle(IOException ex) {
+		// ...
+	}
+----
+
+Or even a list of specific exception types with a very generic argument signature:
 
+[source,java,indent=0]
+[subs="verbatim,quotes"]
+----
+	@ExceptionHandler({FileSystemException.class, RemoteException.class})
+	public ResponseEntity<String> handle(Exception ex) {
+		// ...
 	}
 ----
 
-The annotation can list the exception types to match. Or simply declare the target
-exception as a method argument as shown above. When multiple exception methods match,
-a root exception match is generally preferred to a cause exception match. More formally
-the `ExceptionDepthComparator` is used to sort exceptions based on their depth from the
-thrown exception type.
+[NOTE]
+====
+The distinction between root and cause exception matching can be surprising:
+
+In the `IOException` variant above, the method will typically be called with
+the actual `FileSystemException` or `RemoteException` instance as the argument
+since both of them extend from `IOException`. However, if any such matching
+exception is propagated within a wrapper exception which is an `IOException`
+itself, the passed-in exception instance will be that wrapper exception.
+
+The behavior is even simpler in the `handle(Exception)` variant: This will
+always be invoked with the wrapper exception in a wrapping scenario, with the
+actually matching exception to be found through `ex.getCause()` in that case.
+The passed-in exception will only be the actual `FileSystemException` or
+`RemoteException` instance when these are thrown as top-level exceptions.
+====
+
+We generally recommend to be as specific as possible in the argument signature,
+reducing the potential for mismatches between root and cause exception types.
+Consider breaking a multi-matching method into individual `@ExceptionHandler`
+methods, each matching a single specific exception type through its signature.
 
 In a multi-`@ControllerAdvice` arrangement, please declare your primary root exception
 mappings on a `@ControllerAdvice` prioritized with a corresponding order. While a root
-exception match is preferred to a cause, this is mainly among the methods of a given
-controller or `@ControllerAdvice`. That means a cause match on a higher-priority
-`@ControllerAdvice` is preferred to any match (e.g. root) on a lower-priority
-`@ControllerAdvice`.
+exception match is preferred to a cause, this is defined among the methods of a given
+controller or `@ControllerAdvice` class. This means a cause match on a higher-priority
+`@ControllerAdvice` bean is preferred to any match (e.g. root) on a lower-priority
+`@ControllerAdvice` bean.
+
+Last but not least, an `@ExceptionHandler` method implementation may choose to back
+out of dealing with a given exception instance by rethrowing it in its original form.
+This is useful in scenarios where you are only interested in root-level matches or in
+matches within a specific context that cannot be statically determined. A rethrown
+exception will be propagated through the remaining resolution chain, just like if
+the given `@ExceptionHandler` method would not have matched in the first place.
 
 Support for `@ExceptionHandler` methods in Spring MVC is built on the `DispatcherServlet`
 level, <<mvc-exceptionhandlers,HandlerExceptionResolver>> mechanism.
