From 0345d734e63e33096ec91b8ef62b6868b0e5fc36 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 23 Jun 2016 14:12:37 +0200 Subject: [PATCH] Improve exception logging in HandlerExceptionResolvers This commit updates AbstractHandlerExceptionResolver to only log at the WARN level exceptions that are actually resolved by the ExceptionResolver. In case developers wish to log each time an ExceptionResolver is called, a DEBUG level log is still available. Issue: SPR-14392 --- .../AbstractHandlerExceptionResolver.java | 13 ++++++++----- .../ResponseStatusExceptionResolverTests.java | 8 +++++++- .../ExceptionHandlerExceptionResolverTests.java | 16 +++++++++++----- .../DefaultHandlerExceptionResolverTests.java | 1 + 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerExceptionResolver.java index 9ae1cb4238e..50997f55bd3 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerExceptionResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ package org.springframework.web.servlet.handler; import java.util.Set; + import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -128,13 +129,15 @@ public abstract class AbstractHandlerExceptionResolver implements HandlerExcepti Object handler, Exception ex) { if (shouldApplyTo(request, handler)) { - // Log exception, both at debug log level and at warn level, if desired. if (this.logger.isDebugEnabled()) { this.logger.debug("Resolving exception from handler [" + handler + "]: " + ex); } - logException(ex, request); prepareResponse(ex, response); - return doResolveException(request, response, handler, ex); + ModelAndView result = doResolveException(request, response, handler, ex); + if (result != null) { + logException(ex, request); + } + return result; } else { return null; @@ -194,7 +197,7 @@ public abstract class AbstractHandlerExceptionResolver implements HandlerExcepti * @return the log message to use */ protected String buildLogMessage(Exception ex, HttpServletRequest request) { - return "Handler execution resulted in exception: " + ex; + return "Resolved exception caused by Handler execution: " + ex; } /** diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolverTests.java index d615b3a2ee2..f5bc12712ae 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.Locale; +import org.junit.Before; import org.junit.Test; import org.springframework.beans.TypeMismatchException; @@ -49,6 +50,11 @@ public class ResponseStatusExceptionResolverTests { private final MockHttpServletResponse response = new MockHttpServletResponse(); + @Before + public void setup() { + exceptionResolver.setWarnLogCategory(exceptionResolver.getClass().getName()); + } + @Test public void statusCode() { StatusCodeException ex = new StatusCodeException(); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java index e41f7dd6295..f633f73b016 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java @@ -80,6 +80,7 @@ public class ExceptionHandlerExceptionResolverTests { @Before public void setup() throws Exception { this.resolver = new ExceptionHandlerExceptionResolver(); + this.resolver.setWarnLogCategory(this.resolver.getClass().getName()); this.request = new MockHttpServletRequest("GET", "/"); this.response = new MockHttpServletResponse(); } @@ -399,11 +400,13 @@ public class ExceptionHandlerExceptionResolverTests { @Configuration static class MyConfig { - @Bean public TestExceptionResolver testExceptionResolver() { + @Bean + public TestExceptionResolver testExceptionResolver() { return new TestExceptionResolver(); } - @Bean public AnotherTestExceptionResolver anotherTestExceptionResolver() { + @Bean + public AnotherTestExceptionResolver anotherTestExceptionResolver() { return new AnotherTestExceptionResolver(); } } @@ -445,15 +448,18 @@ public class ExceptionHandlerExceptionResolverTests { @Configuration static class MyControllerAdviceConfig { - @Bean public NotCalledTestExceptionResolver notCalledTestExceptionResolver() { + @Bean + public NotCalledTestExceptionResolver notCalledTestExceptionResolver() { return new NotCalledTestExceptionResolver(); } - @Bean public BasePackageTestExceptionResolver basePackageTestExceptionResolver() { + @Bean + public BasePackageTestExceptionResolver basePackageTestExceptionResolver() { return new BasePackageTestExceptionResolver(); } - @Bean public DefaultTestExceptionResolver defaultTestExceptionResolver() { + @Bean + public DefaultTestExceptionResolver defaultTestExceptionResolver() { return new DefaultTestExceptionResolver(); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java index 21bf3834bf1..40ee1ecb022 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java @@ -61,6 +61,7 @@ public class DefaultHandlerExceptionResolverTests { @Before public void setUp() { exceptionResolver = new DefaultHandlerExceptionResolver(); + exceptionResolver.setWarnLogCategory(exceptionResolver.getClass().getName()); request = new MockHttpServletRequest(); response = new MockHttpServletResponse(); request.setMethod("GET");