Uploaded image for project: 'Spring Framework'
  1. Spring Framework
  2. SPR-13596

Inappropriate warn logging in AbstractHandlerExceptionResolver (e.g. for 404 status)

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Complete
    • Affects Version/s: 4.2.1, 4.2.2
    • Fix Version/s: 4.2.3
    • Component/s: Web
    • Labels:
      None
    • Last commented by a User:
      true

      Description

      In the AbstractHandlerExceptionResolver the log level for all responses handled by the resolveException method is set to WARN, in bold below. The problem is that not all responses handled by the method are responses that should be treated at the WARN log level, as normal responses to the end user can be handled by this method.

      The log level should be reduced to DEBUG.

      	@Override
      	public ModelAndView resolveException(HttpServletRequest request, HttpServletResponse response,
      			Object handler, Exception ex) {
       
      		if (shouldApplyTo(request, handler)) {
      			// Log exception at debug log level
      			if (this.logger.isDebugEnabled()) {
      				this.logger.debug("Resolving exception from handler [" + handler + "]: " + ex);
      			}
      			prepareResponse(ex, response);
      			ModelAndView mav = doResolveException(request, response, handler, ex);
      			if (mav != null) {
      				// Log exception message at warn log level
      				logException(ex, request);
      			}
      			return mav;
      		}
      		else {
      			return null;
      		}
      	}
      

        Issue Links

          Activity

          Hide
          juergen.hoeller Juergen Hoeller added a comment -

          We're only really logging resolved error views at warn level... which doesn't really qualify as a normal user response. REST-like translations to a certain response status etc will never return a ModelAndView there to begin with.

          If you're unhappy with the warn-level logging for error views, consider setting your HandlerExceptionResolver's warnLogCategory property to null or overriding the logException method in a custom subclass.

          Assigning this to Rossen Stoyanchev for further review. We may of course fine-tune this in 4.3 if there turns out to be some common scenario where warn-level logging is irritating.

          Juergen

          Show
          juergen.hoeller Juergen Hoeller added a comment - We're only really logging resolved error views at warn level... which doesn't really qualify as a normal user response. REST-like translations to a certain response status etc will never return a ModelAndView there to begin with. If you're unhappy with the warn-level logging for error views, consider setting your HandlerExceptionResolver 's warnLogCategory property to null or overriding the logException method in a custom subclass. Assigning this to Rossen Stoyanchev for further review. We may of course fine-tune this in 4.3 if there turns out to be some common scenario where warn-level logging is irritating. Juergen
          Hide
          rstoya05-aop Rossen Stoyanchev added a comment -

          Indeed the logging occurs when the resolver returns a ModelAndView which indicates the exception was resolved by the handler and an (error) view is about to be displayed. Can you clarify what you mean by "normal responses" in this context? And why switch to DEBUG altogether. Shouldn't that be for "normal" only which leads back to the question about how you would identify a normal response within an exception resolver.

          Show
          rstoya05-aop Rossen Stoyanchev added a comment - Indeed the logging occurs when the resolver returns a ModelAndView which indicates the exception was resolved by the handler and an (error) view is about to be displayed. Can you clarify what you mean by "normal responses" in this context? And why switch to DEBUG altogether. Shouldn't that be for "normal" only which leads back to the question about how you would identify a normal response within an exception resolver.
          Hide
          jkylberg Jakob Kylberg added a comment -

          What I meant with a normal response is e.g. if an exception is thrown by a component in an application and that exception signals that some data hasn't been found and that a 404 should be returned to the client of the application. That exception will be caught by an exception handler and mapped to a 404 response. This flow will pass through the AbstractHandlerExceptionResolver and lead to a WARN log message that in this case IMO is not appropriate as even though it's not a 200 OK flow, it's not something that warrants a WARN log message.

          Show
          jkylberg Jakob Kylberg added a comment - What I meant with a normal response is e.g. if an exception is thrown by a component in an application and that exception signals that some data hasn't been found and that a 404 should be returned to the client of the application. That exception will be caught by an exception handler and mapped to a 404 response. This flow will pass through the AbstractHandlerExceptionResolver and lead to a WARN log message that in this case IMO is not appropriate as even though it's not a 200 OK flow, it's not something that warrants a WARN log message.
          Hide
          juergen.hoeller Juergen Hoeller added a comment -

          It turns out that this default warn logging is rather recent: introduced through SPR-13100 for 4.2. We should fine-tune this to only apply to certain kinds of responses then, not firing in case of a plain 404 or the like.

          Juergen

          Show
          juergen.hoeller Juergen Hoeller added a comment - It turns out that this default warn logging is rather recent: introduced through SPR-13100 for 4.2. We should fine-tune this to only apply to certain kinds of responses then, not firing in case of a plain 404 or the like. Juergen
          Hide
          sdeleuze Sébastien Deleuze added a comment -

          I resolved this issue by reverting most SPR-13100 changes since SPR-13267 (logging conversion exceptions at DefaultHandlerExceptionResolver level) was a better fix.

          Show
          sdeleuze Sébastien Deleuze added a comment - I resolved this issue by reverting most SPR-13100 changes since SPR-13267 (logging conversion exceptions at DefaultHandlerExceptionResolver level) was a better fix.

            People

            • Assignee:
              sdeleuze Sébastien Deleuze
              Reporter:
              jkylberg Jakob Kylberg
              Last updater:
              Stéphane Nicoll
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                2 years, 16 weeks, 5 days ago