You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by "Byron Foster (JIRA)" <de...@velocity.apache.org> on 2008/12/10 17:26:44 UTC
[jira] Created: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
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
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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "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=12660091#action_12660091 ]
Nathan Bubna commented on VELOCITY-656:
---------------------------------------
No, no new interfaces. I'm not even much a fan of TemplateNumber. I prefer reflection and convention to avoid hard dependencies on Velocity APIs. Version compatibility is much easier on both sides when you test for the presence of the feature directly, rather than rely on interfaces. We're moving into 2.0 discussion again, but really, i would prefer to watch for methods like getAsNumber(), getAsBoolean(), and getAsString() for such things.
In the 1.x frame, we must still allow/expect toString() to return null. Since we're looking at a 1.6.2 release, you might as well commit your try/catch exception wrapping for this into the trunk and merge it to the 1.6.x branch. My main reason for marking this as a 1.7 fix was that i didn't think it worth rebuilding 1.6.1 for this, and i didn't yet envision a 1.6.2.
> 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.6.1, 1.7
> Reporter: Byron Foster
> Fix For: 1.7
>
> 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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "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=12660479#action_12660479 ]
Nathan Bubna commented on VELOCITY-656:
---------------------------------------
Thanks! :)
> 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.6.1, 1.7
> Reporter: Byron Foster
> Fix For: 1.7
>
> 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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "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
[jira] Updated: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "Byron Foster (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELOCITY-656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Byron Foster updated VELOCITY-656:
----------------------------------
Attachment: 656.patch
> 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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "Claude Brisson (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELOCITY-656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12655536#action_12655536 ]
Claude Brisson commented on VELOCITY-656:
-----------------------------------------
You might just drop Hibernate itself... ;-)
> 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.6.1, 1.7
> Reporter: Byron Foster
> Fix For: 1.7
>
> 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
[jira] Closed: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "Byron Foster (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELOCITY-656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Byron Foster closed VELOCITY-656.
---------------------------------
Resolution: Fixed
> 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.6.1, 1.7
> Reporter: Byron Foster
> Fix For: 1.7
>
> 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
[jira] Updated: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "Nathan Bubna (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELOCITY-656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Nathan Bubna updated VELOCITY-656:
----------------------------------
Affects Version/s: 1.7
1.6
Fix Version/s: 1.7
> 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.6.1, 1.7
> Reporter: Byron Foster
> Fix For: 1.7
>
> 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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "Byron Foster (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELOCITY-656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660068#action_12660068 ]
Byron Foster commented on VELOCITY-656:
---------------------------------------
How about an interface much like TemplateNumber.java (which may be what you are referring to)
interface TemplateEvaluate {
public boolean evaluateBoolean()
}
evaluate would do an instanceOf and then test for evaluateBoolean instead of calling toString()?
Classes like Alternator could implement this interface.
> 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.6.1, 1.7
> Reporter: Byron Foster
> Fix For: 1.7
>
> 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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "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=12655313#action_12655313 ]
Nathan Bubna commented on VELOCITY-656:
---------------------------------------
If it won't bother you to leave this out of 1.6.1, that's great, i'm rather weary of rebuilding it.
Dropping Hibernate objects in the context is always scary, but i can't deny having done it myself.
I still think it's terribly wrong that toString() ever throws an exception.
My feeling was that logging this was better than wrapping it, but it's more of a -0 feeling than a -1 feeling.
Let's leave this for 1.7.
And i'll probably just let you commit it once we get your account set up (hopefully not too much longer).
> 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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "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=12656027#action_12656027 ]
Nathan Bubna commented on VELOCITY-656:
---------------------------------------
Yeah, i think we should allow/expect toString() might return null, at least for 1.x. For 2.0, i've been musing about watching for a getAsString() when going to render/eval/whatever something. Then we can again expect toString() to never return null, and ask those objects who want special treatment to implement getAsString()
> 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.6.1, 1.7
> Reporter: Byron Foster
> Fix For: 1.7
>
> 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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "Byron Foster (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELOCITY-656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12655308#action_12655308 ]
Byron Foster commented on VELOCITY-656:
---------------------------------------
This issue came up when referring to a Hibernate object wrapped with a proxy object which is a standard pattern that Hibernate uses for working with POJOs. If any methods are called on the proxy outside of the originating Hibernate session, then Hibernate throws the dreaded LazyIntializationException, this includes the toString() method. The stack trace in this case is deep within Hibernate.
I guess I have two points, the first is that I think that Velocity should always report accurately where in a template a problem has occurred. The second, as far as I know, currently any error encountered while processing a template generates some sort of velocity exception, and the location of the problem within the template file is reported in the exception message. This patch was attempting to be consistent with that trend.
My only concern with calling toString() is that on some object this may be non-trivial, and may be surprising behavior from #if($foo). But, it's probably not a big deal.
Let me know if you think another approach would be better, and I'll provide a new patch. I don't think this has a high priority which is why I marked it as an improvement, so I don't think it's necessary for 1.6.1.
> 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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "Byron Foster (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELOCITY-656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12656019#action_12656019 ]
Byron Foster commented on VELOCITY-656:
---------------------------------------
Setting aside Hibernate's suspicious claims to using POJO's, I think that actually my preferred solution is to remove the call to toString, but it sounds like that will meet resistance :)
> 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.6.1, 1.7
> Reporter: Byron Foster
> Fix For: 1.7
>
> 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
[jira] Commented: (VELOCITY-656) Wrap ref evaluate exception so vm
location is revealed
Posted by "Byron Foster (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELOCITY-656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660467#action_12660467 ]
Byron Foster commented on VELOCITY-656:
---------------------------------------
That's very Pythonic of you :). Ok committed to trunk and 1.6.2
> 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.6.1, 1.7
> Reporter: Byron Foster
> Fix For: 1.7
>
> 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