[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm location is revealed

["Nathan Bubna (JIRA)" <de...@velocity.apache.org>]

    [ https://issues.apache.org/jira/browse/VELOCITY-656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12655283#action_12655283 ] 

Nathan Bubna commented on VELOCITY-656:
---------------------------------------

It's common Java practice that toString() should never return null or throw an exception.  It is pretty rare that any software is prepared to handle either possibility.   Velocity, however, has always been fine with returning null in toString() and never complained about such behavior.  The Alternator class from VelocityTools even relies on this to alternate over sets including null values.  I personally have used this often in my own development.  The change to #if to test for toString() == null is to ensure that #if works with this paradigm.

I'm not inclined to rebuild 1.6.1 again for this, and i'm hesitant about it in general.  To me, an exception during a toString() call is a big no-no on the user's part.  The stack trace they get should point them to the toString() call, which i think ought to be the thing they focus on fixing.  The template/line/col of the guilty VTL reference might be helpful to log, but their problem is in Java, not their totally valid VTL.  Wrapping the root exception in a VelocityException feels like it draws attention to the wrong code location.

> Wrap ref evaluate exception so vm location is revealed
> ------------------------------------------------------
>
>                 Key: VELOCITY-656
>                 URL: https://issues.apache.org/jira/browse/VELOCITY-656
>             Project: Velocity
>          Issue Type: Improvement
>    Affects Versions: 1.6.1
>            Reporter: Byron Foster
>         Attachments: 656.patch
>
>
> The following VTL calls the toString() method on the object in $foo:
> #if($foo)#end  
> If the toString() method throws an exception, the exception is thrown all the way out of velocity without any indication of where the vtl reference occurred.  This patch wraps the exception with a VelocityException with a message that indicates the location of the offending reference.
> However, I wonder why toString() is called at all...  the code tests for toString() == null and returns false if so, and true otherwise in the above example vtl.  Why is this test necessary, and why isn't just a non-null object enough for testing?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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