Velocity
  1. Velocity
  2. VELOCITY-644

Wrong template name in Exception Message

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6-beta1
    • Fix Version/s: 1.6
    • Component/s: Engine
    • Labels:
      None

      Description

      If a reference within a macro causes an Exception, and the macro is defined in a different file, then the Exception message thrown by Velocity reports the name of the calling template file, and not the file that defines the macro. However, the column number and line number are correct for the macro file.

      1. fix644.patch
        27 kB
        Byron Foster

        Activity

        Hide
        Nathan Bubna added a comment -

        Ok, the fix and extraneous changes are in. I'd rather have them separate, but it's not worth being picky.

        Updated 1.6 test build to come shortly....

        Show
        Nathan Bubna added a comment - Ok, the fix and extraneous changes are in. I'd rather have them separate, but it's not worth being picky. Updated 1.6 test build to come shortly....
        Hide
        Byron Foster added a comment -

        I actually snuck in a small change. When in strict mode and a reference method was called on a null value, like $NULL.foo, then ASTReference would throw a MethodInvocationException. There were two problems with this. The first was that really the MethodInvocationException is intended for errors that occur while calling the method, like an exception being thrown form within the method, and as such the wording of the error message was not appropriate for $NULL.foo, and was misleading. The second is that this would cause a redundant error message to be logged. I think too much info is just as bad as too little So... I changed the case of $NULL.foo to throw a VelocityException, and I Modified StrictReferenceTestCase to handle this change.

        Show
        Byron Foster added a comment - I actually snuck in a small change. When in strict mode and a reference method was called on a null value, like $NULL.foo, then ASTReference would throw a MethodInvocationException. There were two problems with this. The first was that really the MethodInvocationException is intended for errors that occur while calling the method, like an exception being thrown form within the method, and as such the wording of the error message was not appropriate for $NULL.foo, and was misleading. The second is that this would cause a redundant error message to be logged. I think too much info is just as bad as too little So... I changed the case of $NULL.foo to throw a VelocityException, and I Modified StrictReferenceTestCase to handle this change.
        Hide
        Will Glass-Husain added a comment -

        That looks pretty good to me. Did you include the change to StrictReferenceTestCase by accident?

        Show
        Will Glass-Husain added a comment - That looks pretty good to me. Did you include the change to StrictReferenceTestCase by accident?
        Hide
        Byron Foster added a comment -

        Attached is a fix for this bug along with a test case. This fix looks bigger then it really is. I added a template member to the SimpleNode class along with a get property to compliment getLine an getColumn. This member is initialized in the SimpleNode Constructor and set from the Parser. I think it is more reliable to associate the template name to the node, as is currently done with the line number and column, then it is to get it from the context. Before this fix the context template was used to indicate the template file the error occurred in, but this really is not the intended use of this file name, but rather Its purpose is to track scope.

        Show
        Byron Foster added a comment - Attached is a fix for this bug along with a test case. This fix looks bigger then it really is. I added a template member to the SimpleNode class along with a get property to compliment getLine an getColumn. This member is initialized in the SimpleNode Constructor and set from the Parser. I think it is more reliable to associate the template name to the node, as is currently done with the line number and column, then it is to get it from the context. Before this fix the context template was used to indicate the template file the error occurred in, but this really is not the intended use of this file name, but rather Its purpose is to track scope.

          People

          • Assignee:
            Unassigned
            Reporter:
            Byron Foster
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development