You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Sean Legassick <se...@informage.net> on 2001/02/27 09:20:11 UTC
[PATCH] Exceptions in ASTMethod
Actually the patch below is only a little bit of what I want :-)
With the increasing move towards the "pull" model in Turbine, methods
invoked from templates are likely to be doing more and more real work,
so when they throw exceptions it's really important to be able to find
out what happened. Right now the exception just gets eaten, and the
reference evaluates to null. The patch below helps a bit by logging the
exception to the velocity log.
However what I'd really like is for Velocity to propogate the exception
right up to the application back out through Template.merge. That way
the application (in my case Turbine) can use its own mechanisms for
notifying the user/developer of what happened. Of course you might want
to wrap the exception in a velocity class (probably one living in
o.a.v.exception).
I realise this involves changing the signature of SimpleNode and
probably some other methods back up the call stack to merge, but it
would add huge value for me. Non of the other SimpleNode subclasses
need to change the execute signature, subclassed methods can have a
narrower throws definitions than the superclass.
Anyway the patch to add logging:
Index: ASTMethod.java
===================================================================
RCS file: /home/cvspublic/jakarta-velocity/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java,v
retrieving revision 1.10
diff -u -r1.10 ASTMethod.java
--- ASTMethod.java 2001/01/03 05:26:27 1.10
+++ ASTMethod.java 2001/02/27 08:12:00
@@ -248,6 +248,7 @@
}
catch (Exception e)
{
+ Runtime.error("ASTMethod.execute() : invocation exception : " + e );
return null;
}
}
--
Sean Legassick
sean@informage.net
Hombre soy, nada humano me puede ser ajeno
Re: [PATCH] Exceptions in ASTMethod (revised)
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
This is the right idea. Well figure out something to throw at you :)
geir
Sean Legassick wrote:
>
> On Tue, Feb 27, 2001 at 08:20:11AM +0000, Sean Legassick wrote:
> > Actually the patch below is only a little bit of what I want :-)
>
> Actually it was a tiny if not non-existent bit of what I want :-)
>
> The log only says 'InvocationTargetException' which isn't much help -
> the patches below fix that and dig out the wrapped exception. I added a
> patch for PropertyExecutor because this is just as relevant there.
>
> BTW as regards adding proper exception propogation I'm willing to code
> up some patches myself if the principle and the type of exception to
> throw is agreed...
>
> Sean
>
> Index: ASTMethod.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-velocity/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java,v
> retrieving revision 1.10
> diff -u -r1.10 ASTMethod.java
> --- ASTMethod.java 2001/01/03 05:26:27 1.10
> +++ ASTMethod.java 2001/02/27 09:01:22
> @@ -74,6 +74,7 @@
> package org.apache.velocity.runtime.parser.node;
>
> import java.lang.reflect.Method;
> +import java.lang.reflect.InvocationTargetException;
>
> import java.io.*;
>
> @@ -246,6 +247,12 @@
>
> return obj;
> }
> + catch (InvocationTargetException e)
> + {
> + Runtime.error("ASTMethod.execute() : invocation exception : "
> + + e.getTargetException() );
> + return null;
> + }
> catch (Exception e)
> {
> return null;
>
> Index: PropertyExecutor.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-velocity/src/java/org/apache/velocity/runtime/parser/node/PropertyExecutor.java,v
> retrieving revision 1.4
> diff -u -r1.4 PropertyExecutor.java
> --- PropertyExecutor.java 2001/02/01 17:28:15 1.4
> +++ PropertyExecutor.java 2001/02/27 09:03:13
> @@ -55,8 +55,10 @@
> package org.apache.velocity.runtime.parser.node;
>
> import java.lang.reflect.Method;
> +import java.lang.reflect.InvocationTargetException;
>
> import org.apache.velocity.context.InternalContextAdapter;
> +import org.apache.velocity.runtime.Runtime;
>
> /**
> * Returned the value of object property when executed.
> @@ -126,6 +128,12 @@
> return null;
>
> return method.invoke(o, null);
> + }
> + catch (InvocationTargetException e)
> + {
> + Runtime.error("PropertyExecutor: invocation exception "
> + + e.getTargetException());
> + return null;
> }
> catch (Exception e)
> {
> --
> Sean Legassick
> sean@informage.net
> Hombre soy, nada humano me puede ser ajeno
>
>
--
Geir Magnusson Jr. geirm@optonline.com
Developing for the web? See http://jakarta.apache.org/velocity/
Re: [PATCH] Exceptions in ASTMethod (revised)
Posted by Daniel Rall <dl...@collab.net>.
"Geir Magnusson Jr." <ge...@optonline.net> writes:
> Sean Legassick wrote:
> >
> > On Tue, Feb 27, 2001 at 08:00:31AM -0500, Geir Magnusson Jr. wrote:
> >
> > > Just to summarize so it's clear - you are not looking for invocation
> > > exceptions to be thrown (because those are a natural part of the
> > > introspection flail-o-matic in some places, but any exceptions *your*
> > > methods throw to propogate, right?
> >
> > Yep, absolutely (although given enough prodding of an invocation
> > exception one can get at the heart of the problem anyway, but it
> > probably makes sense to strip that layer away - like you say its just
> > part of the introspection mechanism gubbins)...
>
> I was thinking about this and looking through the code.
>
> I think its a good idea to propagate exceptions from method invocations
> out through Template.merge().
I agree, +1.
--
Daniel Rall <dl...@collab.net>
Re: concatenation eccentricity
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
John McNally wrote:
>
> I had something similar to:
>
> #set ( $a = "$b${c}foo.java" )
> #set ( $d = "$b${c}.java" )
>
> if $b=boy and $c=cat
> this gives $a=boycatfoo.java and $d=boy${c}.java
>
> changing to
>
> #set ( $d = "${b}${c}.java" )
> it works => $d = boycat.java
>
> I have no problem with requiring the braces but not sure why it works
> for $a but not $d.
>
> John McNally
Should be fixed now, so give it a whirl. Thanks again.
geir
--
Geir Magnusson Jr. geirm@optonline.com
Developing for the web? See http://jakarta.apache.org/velocity/
Re: concatenation eccentricity
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
John McNally wrote:
>
> I had something similar to:
>
> #set ( $a = "$b${c}foo.java" )
> #set ( $d = "$b${c}.java" )
>
> if $b=boy and $c=cat
> this gives $a=boycatfoo.java and $d=boy${c}.java
>
> changing to
>
> #set ( $d = "${b}${c}.java" )
> it works => $d = boycat.java
>
> I have no problem with requiring the braces but not sure why it works
> for $a but not $d.
>
> John McNally
That's wierd, and appears to be a bug. :) Thanks.
geir
--
Geir Magnusson Jr. geirm@optonline.com
Developing for the web? See http://jakarta.apache.org/velocity/
concatenation eccentricity
Posted by John McNally <jm...@collab.net>.
I had something similar to:
#set ( $a = "$b${c}foo.java" )
#set ( $d = "$b${c}.java" )
if $b=boy and $c=cat
this gives $a=boycatfoo.java and $d=boy${c}.java
changing to
#set ( $d = "${b}${c}.java" )
it works => $d = boycat.java
I have no problem with requiring the braces but not sure why it works
for $a but not $d.
John McNally
RE: [PATCH] Exceptions in ASTMethod (revised)
Posted by Paulo Gaspar <pa...@krankikom.de>.
I agree with the idea. But what about the InvocationTargetException?
Having a wrapping exception for Exceptions that are thrown from invoked
user methods helps finding out where the problem is - that way the user
is sure that it is his fault and not something in the library.
BTW, I would suggest using a method declared as:
java.lang.Throwable getRootCause()
to get the original exception from InvocationTargetException. This way
one follows the ServletException "convention" with the additional
advantage that Tomcat (at least 3.3, but I believe it is common code)
will print the Original Exception and its call stack. (Besides,
"getRootCause()" sounds more explicit to me than "getTargetException()",
but that might just be personal preference.)
This happens because Tomcat tries to get an original exception calling
a few possible methods "by name". Follows the source for the Tomcat
method that does the trick, since it is an interesting piece of code.
It comes from package org.apache.tomcat.util.log, class Logger:
private static void printThrowable(PrintWriter w, Throwable t, String
rootcause, int depth ) {
if (t != null) {
// XXX XXX XXX Something seems wrong - DOS, permissions. Need to
// check.
t.printStackTrace(w);
// Find chained exception using few general patterns
Class tC=t.getClass();
Method mA[]= tC.getMethods();
Method nextThrowableMethod=null;
for( int i=0; i< mA.length ; i++ ) {
if( "getRootCause".equals( mA[i].getName() )
|| "getNextException".equals( mA[i].getName() )
|| "getException".equals( mA[i].getName() )) {
// check param types
Class params[]=mA[i].getParameterTypes();
if( params==null || params.length==0 ) {
nextThrowableMethod=mA[i];
break;
}
}
}
if( nextThrowableMethod != null ) {
try {
Throwable nextT=(Throwable)nextThrowableMethod.invoke( t ,
emptyObjectArray );
if( nextT != null ) {
w.println(rootcause);
if( depth > 0 ) {
printThrowable(w, nextT, rootcause, depth-1);
}
}
} catch( Exception ex ) {
// ignore
}
}
}
}
Have fun,
Paulo Gaspar
> -----Original Message-----
> From: gmj@mta2.srv.hcvlny.cv.net [mailto:gmj@mta2.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
>
> Sean Legassick wrote:
> >
> > On Tue, Feb 27, 2001 at 08:00:31AM -0500, Geir Magnusson Jr. wrote:
> >
> > > Just to summarize so it's clear - you are not looking for invocation
> > > exceptions to be thrown (because those are a natural part of the
> > > introspection flail-o-matic in some places, but any exceptions *your*
> > > methods throw to propogate, right?
> >
> > Yep, absolutely (although given enough prodding of an invocation
> > exception one can get at the heart of the problem anyway, but it
> > probably makes sense to strip that layer away - like you say its just
> > part of the introspection mechanism gubbins)...
>
> I was thinking about this and looking through the code.
>
> I think its a good idea to propagate exceptions from method invocations
> out through Template.merge().
>
> I can enumerate why if anyone needs details - the short answer is that I
> think that you need to know during runtime, in the Controller layer,
> when something goes wrong. That way you can do something intelligent,
> like switch templates, email the admin, panic, whatever :)
>
> This will turn out to be very important in the 'Pull Model' - there is
> no other way that an application can gracefully recover from an error -
> the template shouldn't have to deal with it. (Users calling saying the
> site is hosed doesn't cut it either... :)
>
> It will take a little care and work (so I am not doing it tonight - too
> fogged in - and wanted to solicit opinions...)
>
> To summarize :
>
> public void merge( Context context, Writer writer)
> throws ResourceNotFoundException, ParseErrorException, Exception
>
> where Exception can also be exceptions thrown from in-context method
> invocations.
>
> We can make a new exception (one of the o.a.v.e set), but I don't really
> see any point.
>
> The public API won't change at all, so no code should notice (other than
> they have a chance to deal with problems.
>
> geir
>
> --
> Geir Magnusson Jr. geirm@optonline.com
> Developing for the web? See http://jakarta.apache.org/velocity/
>
Re: [PATCH] Exceptions in ASTMethod (revised)
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Sean Legassick wrote:
>
> On Tue, Feb 27, 2001 at 08:00:31AM -0500, Geir Magnusson Jr. wrote:
>
> > Just to summarize so it's clear - you are not looking for invocation
> > exceptions to be thrown (because those are a natural part of the
> > introspection flail-o-matic in some places, but any exceptions *your*
> > methods throw to propogate, right?
>
> Yep, absolutely (although given enough prodding of an invocation
> exception one can get at the heart of the problem anyway, but it
> probably makes sense to strip that layer away - like you say its just
> part of the introspection mechanism gubbins)...
I was thinking about this and looking through the code.
I think its a good idea to propagate exceptions from method invocations
out through Template.merge().
I can enumerate why if anyone needs details - the short answer is that I
think that you need to know during runtime, in the Controller layer,
when something goes wrong. That way you can do something intelligent,
like switch templates, email the admin, panic, whatever :)
This will turn out to be very important in the 'Pull Model' - there is
no other way that an application can gracefully recover from an error -
the template shouldn't have to deal with it. (Users calling saying the
site is hosed doesn't cut it either... :)
It will take a little care and work (so I am not doing it tonight - too
fogged in - and wanted to solicit opinions...)
To summarize :
public void merge( Context context, Writer writer)
throws ResourceNotFoundException, ParseErrorException, Exception
where Exception can also be exceptions thrown from in-context method
invocations.
We can make a new exception (one of the o.a.v.e set), but I don't really
see any point.
The public API won't change at all, so no code should notice (other than
they have a chance to deal with problems.
geir
--
Geir Magnusson Jr. geirm@optonline.com
Developing for the web? See http://jakarta.apache.org/velocity/
Re: [PATCH] Exceptions in ASTMethod (revised)
Posted by Sean Legassick <se...@informage.net>.
On Tue, Feb 27, 2001 at 08:00:31AM -0500, Geir Magnusson Jr. wrote:
> Sean Legassick wrote:
> >
> > On Tue, Feb 27, 2001 at 08:20:11AM +0000, Sean Legassick wrote:
> > > Actually the patch below is only a little bit of what I want :-)
> >
> > Actually it was a tiny if not non-existent bit of what I want :-)
> >
> > The log only says 'InvocationTargetException' which isn't much help -
> > the patches below fix that and dig out the wrapped exception. I added a
> > patch for PropertyExecutor because this is just as relevant there.
> >
> > BTW as regards adding proper exception propogation I'm willing to code
> > up some patches myself if the principle and the type of exception to
> > throw is agreed...
> >
>
> Just to summarize so it's clear - you are not looking for invocation
> exceptions to be thrown (because those are a natural part of the
> introspection flail-o-matic in some places, but any exceptions *your*
> methods throw to propogate, right?
Yep, absolutely (although given enough prodding of an invocation
exception one can get at the heart of the problem anyway, but it
probably makes sense to strip that layer away - like you say its just
part of the introspection mechanism gubbins)...
--
Sean Legassick
sean@informage.net
Czlowiekiem jestem i nic co ludzkie nie jest mi obce
Re: [PATCH] Exceptions in ASTMethod (revised)
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Sean Legassick wrote:
>
> On Tue, Feb 27, 2001 at 08:20:11AM +0000, Sean Legassick wrote:
> > Actually the patch below is only a little bit of what I want :-)
>
> Actually it was a tiny if not non-existent bit of what I want :-)
>
> The log only says 'InvocationTargetException' which isn't much help -
> the patches below fix that and dig out the wrapped exception. I added a
> patch for PropertyExecutor because this is just as relevant there.
>
> BTW as regards adding proper exception propogation I'm willing to code
> up some patches myself if the principle and the type of exception to
> throw is agreed...
>
Just to summarize so it's clear - you are not looking for invocation
exceptions to be thrown (because those are a natural part of the
introspection flail-o-matic in some places, but any exceptions *your*
methods throw to propogate, right?
geir
> Sean
>
> Index: ASTMethod.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-velocity/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java,v
> retrieving revision 1.10
> diff -u -r1.10 ASTMethod.java
> --- ASTMethod.java 2001/01/03 05:26:27 1.10
> +++ ASTMethod.java 2001/02/27 09:01:22
> @@ -74,6 +74,7 @@
> package org.apache.velocity.runtime.parser.node;
>
> import java.lang.reflect.Method;
> +import java.lang.reflect.InvocationTargetException;
>
> import java.io.*;
>
> @@ -246,6 +247,12 @@
>
> return obj;
> }
> + catch (InvocationTargetException e)
> + {
> + Runtime.error("ASTMethod.execute() : invocation exception : "
> + + e.getTargetException() );
> + return null;
> + }
> catch (Exception e)
> {
> return null;
>
> Index: PropertyExecutor.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-velocity/src/java/org/apache/velocity/runtime/parser/node/PropertyExecutor.java,v
> retrieving revision 1.4
> diff -u -r1.4 PropertyExecutor.java
> --- PropertyExecutor.java 2001/02/01 17:28:15 1.4
> +++ PropertyExecutor.java 2001/02/27 09:03:13
> @@ -55,8 +55,10 @@
> package org.apache.velocity.runtime.parser.node;
>
> import java.lang.reflect.Method;
> +import java.lang.reflect.InvocationTargetException;
>
> import org.apache.velocity.context.InternalContextAdapter;
> +import org.apache.velocity.runtime.Runtime;
>
> /**
> * Returned the value of object property when executed.
> @@ -126,6 +128,12 @@
> return null;
>
> return method.invoke(o, null);
> + }
> + catch (InvocationTargetException e)
> + {
> + Runtime.error("PropertyExecutor: invocation exception "
> + + e.getTargetException());
> + return null;
> }
> catch (Exception e)
> {
> --
> Sean Legassick
> sean@informage.net
> Hombre soy, nada humano me puede ser ajeno
>
>
--
Geir Magnusson Jr. geirm@optonline.com
Developing for the web? See http://jakarta.apache.org/velocity/
Re: [PATCH] Exceptions in ASTMethod (revised)
Posted by Sean Legassick <se...@informage.net>.
On Tue, Feb 27, 2001 at 08:20:11AM +0000, Sean Legassick wrote:
> Actually the patch below is only a little bit of what I want :-)
Actually it was a tiny if not non-existent bit of what I want :-)
The log only says 'InvocationTargetException' which isn't much help -
the patches below fix that and dig out the wrapped exception. I added a
patch for PropertyExecutor because this is just as relevant there.
BTW as regards adding proper exception propogation I'm willing to code
up some patches myself if the principle and the type of exception to
throw is agreed...
Sean
Index: ASTMethod.java
===================================================================
RCS file: /home/cvspublic/jakarta-velocity/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java,v
retrieving revision 1.10
diff -u -r1.10 ASTMethod.java
--- ASTMethod.java 2001/01/03 05:26:27 1.10
+++ ASTMethod.java 2001/02/27 09:01:22
@@ -74,6 +74,7 @@
package org.apache.velocity.runtime.parser.node;
import java.lang.reflect.Method;
+import java.lang.reflect.InvocationTargetException;
import java.io.*;
@@ -246,6 +247,12 @@
return obj;
}
+ catch (InvocationTargetException e)
+ {
+ Runtime.error("ASTMethod.execute() : invocation exception : "
+ + e.getTargetException() );
+ return null;
+ }
catch (Exception e)
{
return null;
Index: PropertyExecutor.java
===================================================================
RCS file: /home/cvspublic/jakarta-velocity/src/java/org/apache/velocity/runtime/parser/node/PropertyExecutor.java,v
retrieving revision 1.4
diff -u -r1.4 PropertyExecutor.java
--- PropertyExecutor.java 2001/02/01 17:28:15 1.4
+++ PropertyExecutor.java 2001/02/27 09:03:13
@@ -55,8 +55,10 @@
package org.apache.velocity.runtime.parser.node;
import java.lang.reflect.Method;
+import java.lang.reflect.InvocationTargetException;
import org.apache.velocity.context.InternalContextAdapter;
+import org.apache.velocity.runtime.Runtime;
/**
* Returned the value of object property when executed.
@@ -126,6 +128,12 @@
return null;
return method.invoke(o, null);
+ }
+ catch (InvocationTargetException e)
+ {
+ Runtime.error("PropertyExecutor: invocation exception "
+ + e.getTargetException());
+ return null;
}
catch (Exception e)
{
--
Sean Legassick
sean@informage.net
Hombre soy, nada humano me puede ser ajeno