tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1087467 - in /tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool: PoolProperties.java TrapException.java
Date Sat, 02 Apr 2011 18:45:17 GMT
2011/4/1  <fhanik@apache.org>:
> Author: fhanik
> Date: Thu Mar 31 22:28:54 2011
> New Revision: 1087467
>
> URL: http://svn.apache.org/viewvc?rev=1087467&view=rev
> Log:
> Implement exception traps as suggested by Eiji Takahashi
> http://markmail.org/message/c7hrhky4jtgcto76
>
> Added:
>    tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/TrapException.java
> Modified:
>    tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java
>
> Modified: tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java?rev=1087467&r1=1087466&r2=1087467&view=diff
>


In the new TrapException class:

> +    @Override
> +    public Object invoke(Object proxy, Method method, Object[] args) throws Throwable
{
> +        try {
> +            return super.invoke(proxy, method, args);
> +        }catch (Throwable t) {
> +            Throwable exception = t;
> +            if (t instanceof InvocationTargetException) {
> +                InvocationTargetException it = (InvocationTargetException)t;

1) The above class cast is not necessary. You can call
Throwable.getCause() directly.

> +                exception = it.getCause()!=null?it.getCause():it;
> +            }

2) I think you should not wrap Errors. Just
s/catch(Throwable)/catch(Exception)/.
At least, do not catch ones not caught by org.apache.tomcat.util.ExceptionUtils.

3) isDeclaredException(Method, Class) below compares equality of
classes which does not take care of inheritance.

> +    public boolean isDeclaredException(Method m, Class<?> clazz) {
> +        for (Class<?> cl : m.getExceptionTypes()) {
> +            if (cl.equals(clazz)) return true;
> +        }
> +        return false;
> +    }

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