tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Remove use of StandardWrapper.getRootCause()
Date Fri, 05 Apr 2013 22:25:03 GMT
Hi!

I want to deprecate and remove the method
org.apache.catalina.core.StandardWrapper.getRootCause(ServletException).

This method has somewhat long history, first using some known APIs for
specific exception classes, and later updated to use
Throwable.getCause() introduced with Java 1.4.  As it is currently
implemented, it returns the deepest throwable in getCause() chain of a
ServletException.

This method is used for two purposes,

a)  Get exception that will be written to a log file.
b)  For "if (!(rootCause instanceof ClientAbortException))" check. A
ClientAbortException is not logged.

For example, (StandardWrapperValve # invoke(..)):

        } catch (ServletException e) {
            Throwable rootCause = StandardWrapper.getRootCause(e);
            if (!(rootCause instanceof ClientAbortException)) {
                container.getLogger().error(sm.getString(
                        "standardWrapper.serviceExceptionRoot",
                        wrapper.getName(), context.getName(), e.getMessage()),
                        rootCause);
            }
            throwable = e;
            exception(request, response, e);
        }

Regarding a), as I mentioned in comment 3 of bug 54802, I am not
satisfied with it, because

https://issues.apache.org/bugzilla/show_bug.cgi?id=54802#c3

1. The current practice is to provide the "cause" exception whenever
possible. In the last years there was a number of changes in Tomcat
where the "cause" exception is now provided where previously if was
ignored.  So the "root" exception may be not very meaningful,  and
upper level exceptions may provide important contextual information to
it.

2. The ServletException itself might be not a plain wrapper, but some
meaningful child class of it.
E.g. in case of bug 54802 it is a JasperException. It contains
important information.

3. ErrorReportValve prints top-level exception and up to 10 first
"cause" exceptions.  If only the "root" exception is being logged, an
administrator misses a lot of information that has been shown to a
user.


I would rather pass the original ServletException to the logger, so
that its stacktrace including all its causes were logged.


Regarding b) I think that it actually does not work at all.

A new ClientAbortException is created in two places in Tomcat 8,
- OutputBuffer.doFlush(boolean)
- OutputBuffer.realWriteBytes(..).

In both cases it has another exception (an IOException) as its cause,
and thus it is not the "root" exception. Thus the check in b) never succeeds.


I intend to remove usages of "StandardWrapper.getRootCause()" in
Tomcat 7 and 8, passing the original ServletException to a logger.

Regarding ClientAbortException, I think it would be better to add a
new helper method that will go up the getCause() chain, looking for an
instance of it. E.g. "StandardWrapper.isClientAbortException(ex)". It
needs the same recursion protection as the current getRootCause()
method (a limit of 20 loops).

Any comments?

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message