Hello,
I have a custom authenticator valve which failed with an exception that never got logged by tomcat. After a little bit of digging in tomcat’s source it turned out that there is no valve responsible for logging exceptions thrown by valves later in the chain. The closest error handling mechanism to what I’m looking for, is StandardHostValve’s try-catch around the valve chain, which unfortunately logs the exceptions only if someone has called setError() on the response. To be more clear – StandardHostValve either sets the RequestDispatcher.ERROR_EXCEPTION or logs the exception. That logic was introduced with the fix for https://bz.apache.org/bugzilla/show_bug.cgi?id=54123 with (git) commit id: *cf4fae533cb303c97031b68ec652d6624207df21 * And was later modified a bit with the fix for https://bz.apache.org/bugzilla/show_bug.cgi?id=57252 with (git) commit id: *9807122abe9b95e1766d992314e661d9f9ed3634 * What do you think about always logging the caught throwable (the rest of the logic remains unchanged) ? diff --git a/java/org/apache/catalina/core/StandardHostValve.java b/java/org/apache/catalina/core/StandardHostValve.java index 48683f5..17f2dc5 100644 --- a/java/org/apache/catalina/core/StandardHostValve.java +++ b/java/org/apache/catalina/core/StandardHostValve.java @@ -176,13 +176,12 @@ final class StandardHostValve extends ValveBase { } } catch (Throwable t) { ExceptionUtils.handleThrowable(t); + container.getLogger().error("Exception Processing " + + request.getRequestURI(), t); + // If a new error occurred while trying to report a previous - // error simply log the new error and allow the original error - // to be reported. - if (response.isErrorReportRequired()) { - container.getLogger().error("Exception Processing " + - request.getRequestURI(), t); - } else { + // error allow the original error to be reported. + if (!response.isErrorReportRequired()) { request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, t); throwable(request, response, t); } Best regards, Svetlin
