Commons Jelly
  1. Commons Jelly
  2. JELLY-235

nested exceptions are displayed incorrectly with JellyException

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.0
    • Fix Version/s: None
    • Component/s: core / taglib.core
    • Labels:
      None

      Description

      JellyException has some built-in support for nested exceptions. In particular, when it displays a stack trace, it tries to also displays the nested exception, which is great.

      What's not so great is that when JDK 1.4 added this on its own, both Jelly and JDK try to display the nested exceptions, and you ended up seeing O(n^2) stack traces for n-level nested exception.

      Imagine a situation where N JellyExceptions are nested inside each other.
      With the following current printStackTrace:

          public void printStackTrace() {
              super.printStackTrace();
              if (cause != null) {
                  System.out.println("Root cause");
                  cause.printStackTrace();
              }
          }
      

      Outer JellyException prints its stack trace first (in which JDK prints out all the nested exceptions),
      and then JellyException prints the stack trace of the inner JellyException. Then this same step is repeatedly, causing an output like this:

      JellyExpception 3
      Caused by JellyException 2
      Caused by JellyException 1
      Root cause:
      JellyException 2
      Caused by JellyException 1
      Root cause:
      JellyException 1
      

      See the attached patch for one possible fix.

      Personally, I don't mind if JellyException choose to completely stop trying to print out the nested exception on its own, and simply delegate everything to JDK. This will make the output worse on JDK 1.3 and earlier, but the # of people using those is shrinking every day.

        Issue Links

          Activity

          Hide
          Jesse Glick added a comment -

          On a related note: the logic for extracting a detail message from the cause is not very helpful. Example: in my servlet error page I see

          --%<--
          exception

          javax.servlet.ServletException: file:/[....].jelly:6:66: <j:forEach> org/apache/commons/collections/iterators/ArrayIterator
          [....]

          root cause

          org.apache.commons.jelly.JellyTagException: file:/[....].jelly:6:66: <j:forEach> org/apache/commons/collections/iterators/ArrayIterator
          org.apache.commons.jelly.impl.TagScript.handleException(TagScript.java:712)
          org.apache.commons.jelly.impl.TagScript.run(TagScript.java:282)
          [.....]

          note The full stack trace of the root cause is available in the Apache Tomcat/5.5.17 logs.
          --%<--

          Hmm. Not very informative. What is wrong with ArrayIterator? Not of the right type? Hard to guess. Need to go look at the server logs, where I see

          --%<--
          Caused by: java.lang.NoClassDefFoundError: org/apache/commons/collections/iterators/ArrayIterator
          at org.apache.commons.jelly.expression.ExpressionSupport.evaluateAsIterator(ExpressionSupport.java:106)
          at org.apache.commons.jelly.tags.core.ForEachTag.doTag(ForEachTag.java:89)
          at org.apache.commons.jelly.impl.TagScript.run(TagScript.java:262)
          --%<--

          Now that immediately tells me what is going on. The crucial info is the exception class name: NoClassDefFoundError.

          The problem in Jelly is here:

          --%<--
          public JellyException(Throwable cause, String fileName, String elementName, int columnNumber, int lineNumber)

          { this(cause.getLocalizedMessage(), cause, fileName, elementName, columnNumber, lineNumber); }

          public JellyException(String reason, Throwable cause, String fileName, String elementName, int columnNumber, int lineNumber) {
          super( (reason==null?cause.getClass().getName():reason) );
          // ....
          --%<--

          A NCDFE has as its message (and, by default, also its localizedMessage) the class name that could not be found. Since reason != null, that is used as the detail message also for the JellyException. But this is not as informative as it should be.

          I would therefore recommend changing the code to e.g.

          --%<--
          public JellyException(Throwable cause, String fileName, String elementName, int columnNumber, int lineNumber)

          { this(cause.toString(), cause, fileName, elementName, columnNumber, lineNumber); }

          public JellyException(String reason, Throwable cause, String fileName, String elementName, int columnNumber, int lineNumber) {
          super( (reason==null?cause.toString():reason) );
          // ....
          --%<--

          This would ensure that by default, the exception detail message shown in the web page includes both the detail message and class name of the original exception.

          Show
          Jesse Glick added a comment - On a related note: the logic for extracting a detail message from the cause is not very helpful. Example: in my servlet error page I see -- %< -- exception javax.servlet.ServletException: file:/[....].jelly:6:66: <j:forEach> org/apache/commons/collections/iterators/ArrayIterator [....] root cause org.apache.commons.jelly.JellyTagException: file:/[....].jelly:6:66: <j:forEach> org/apache/commons/collections/iterators/ArrayIterator org.apache.commons.jelly.impl.TagScript.handleException(TagScript.java:712) org.apache.commons.jelly.impl.TagScript.run(TagScript.java:282) [.....] note The full stack trace of the root cause is available in the Apache Tomcat/5.5.17 logs. -- %< -- Hmm. Not very informative. What is wrong with ArrayIterator? Not of the right type? Hard to guess. Need to go look at the server logs, where I see -- %< -- Caused by: java.lang.NoClassDefFoundError: org/apache/commons/collections/iterators/ArrayIterator at org.apache.commons.jelly.expression.ExpressionSupport.evaluateAsIterator(ExpressionSupport.java:106) at org.apache.commons.jelly.tags.core.ForEachTag.doTag(ForEachTag.java:89) at org.apache.commons.jelly.impl.TagScript.run(TagScript.java:262) -- %< -- Now that immediately tells me what is going on. The crucial info is the exception class name: NoClassDefFoundError. The problem in Jelly is here: -- %< -- public JellyException(Throwable cause, String fileName, String elementName, int columnNumber, int lineNumber) { this(cause.getLocalizedMessage(), cause, fileName, elementName, columnNumber, lineNumber); } public JellyException(String reason, Throwable cause, String fileName, String elementName, int columnNumber, int lineNumber) { super( (reason==null?cause.getClass().getName():reason) ); // .... -- %< -- A NCDFE has as its message (and, by default, also its localizedMessage) the class name that could not be found. Since reason != null, that is used as the detail message also for the JellyException. But this is not as informative as it should be. I would therefore recommend changing the code to e.g. -- %< -- public JellyException(Throwable cause, String fileName, String elementName, int columnNumber, int lineNumber) { this(cause.toString(), cause, fileName, elementName, columnNumber, lineNumber); } public JellyException(String reason, Throwable cause, String fileName, String elementName, int columnNumber, int lineNumber) { super( (reason==null?cause.toString():reason) ); // .... -- %< -- This would ensure that by default, the exception detail message shown in the web page includes both the detail message and class name of the original exception.

            People

            • Assignee:
              Unassigned
              Reporter:
              Kohsuke Kawaguchi
            • Votes:
              2 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Development