Bug 16775 - JSTL looses Root Cause exception when it wraps and rethrows a JspTagException
Summary: JSTL looses Root Cause exception when it wraps and rethrows a JspTagException
Status: RESOLVED FIXED
Alias: None
Product: Taglibs
Classification: Unclassified
Component: Standard Taglib (show other bugs)
Version: 1.0.2
Hardware: All other
: P3 enhancement (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-02-04 19:48 UTC by Heath Chiavettone
Modified: 2004-11-16 19:05 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Heath Chiavettone 2003-02-04 19:48:08 UTC
There are 30 places in the code where JSTL catches various exceptions and then 
throws a JspTagException().  At this point the toString() method is called on 
the original root cause exception to construct the JspTagException.  
Unfortunately, the stack trace of the root cause exception is lost at this 
point.  There have been times where the only way to figure out what when wrong 
is to see the original stack trace.  At these points, I've been forced to 
debug the application and set break points down in JSTL code at the point the 
catch occurs so that I can figure out what is the root cause of a problem.

It should be a simple thing to construct the JspTagException and pass the 
original exception as the second parameter to the JspTagException.  Our JSP 
error page is smart enough to drill down through an exception's root cause 
hierarchy and log this information.


\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\extra\spath\SPathTag.java
(104,2):	throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\core\ForEachSupport
.java(407,21):                    throw new JspTagException(ex.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\core\ForEachSupport
.java(415,21):                    throw new JspTagException(ex.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\core\ImportSupport.
java(163,6):	    throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\core\ImportSupport.
java(183,6):	    throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\core\RedirectSuppor
t.java(153,6):	    throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\core\UrlSupport.jav
a(158,3):		throw new JspTagException(ex.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\fmt\BundleSupport.j
ava(137,3):		throw new JspTagException(ioe.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\fmt\FormatDateSuppo
rt.java(199,3):		throw new JspTagException(ioe.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\fmt\FormatNumberSup
port.java(234,3):		throw new JspTagException(ioe.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\fmt\MessageSupport.
java(168,3):		throw new JspTagException(ioe.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\fmt\MessageSupport.
java(223,3):		throw new JspTagException(ioe.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\fmt\ParseDateSuppor
t.java(222,3):		throw new JspTagException(ioe.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\fmt\ParseNumberSupp
ort.java(200,3):		throw new JspTagException(ioe.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\fmt\RequestEncoding
Support.java(130,3):		throw new JspTagException(uee.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\fmt\TimeZoneSupport
.java(133,6):	    throw new JspTagException(ioe.getMessage());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\sql\DataSourceUtil.
java(165,17):                throw new JspTagException(
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\sql\DriverTag.java
(134,6):	    throw new JspTagException("Invalid driver class name: " +
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\sql\SetDataSourceTa
gSupport.java(145,17):                throw new JspTagException(
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\sql\TransactionTagS
upport.java(156,6):	    throw new JspTagException(
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\sql\TransactionTagS
upport.java(171,6):	    throw new JspTagException(
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\xml\ExprSupport.jav
a(107,6):	    throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\xml\ExprSupport.jav
a(109,6):	    throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\xml\ForEachTag.java
(92,13):            throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\xml\IfTag.java
(95,13):            throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\xml\SetTag.java
(122,6):	    throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\common\xml\WhenTag.java
(95,13):            throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\el\core\IfTag.java(102,6):
	    throw new JspTagException(ex.toString());
\jakarta-
taglibs\standard\src\org\apache\taglibs\standard\tag\el\core\WhenTag.java
(100,6):	    throw new JspTagException(ex.toString());
Comment 1 Pierre Delisle 2003-02-06 19:04:44 UTC
First, some background information:

Up until JSP 1.2, there were no JspTagException constructor 
that accepted both a message and a root exception. This has been fixed
in JSP 2.0 (but JSTL 1.0.x must run on JSP 1.2).

The JSTL spec has standardized on JspException because of this:
  Chapter 2 - Conventions
  When an action is required to throw an exception, there were two choices when
  no root cause was involved: JspException or JspTagException. The
  specification has now standardized on JspException everywhere in the spec
  (instead of JspException in some places (with root cause), and
  JspTagException in some others (no root cause)).

This being said, a closer look at all the JspTagExceptions thrown in the
standard taglib shows the following patterns:

1. No root cause involved

   There are cases where a JspTagException is thrown without any root cause
   to be propagated. This is often the case in setter methods.
   For example, in TransactionTagSupport:

        } else {
            throw new JspTagException(
                Resources.getMessage("TRANSACTION_INVALID_ISOLATION"));
        }

   There is nothing to fix here.

2. doStartTag() and doEndTag()

   These two methods of the Tag interface throw a JspException. 
   There are three subpatterns here:

   2.1: No root cause involved
     For example, in TransactionTagSupport:
            ...
            if (origIsolation == Connection.TRANSACTION_NONE) {
                throw new JspTagException(
                    Resources.getMessage("TRANSACTION_NO_SUPPORT"));
            }
     Nothing to fix here.

   2.2: IOException

     Whenever the implementation needs to do some IO (e.g. access 'out', 
     send something on the response), there is
     the risk of an IOException. For example,

     in MessageSupport:
            try {
                pageContext.getOut().print(message);
            } catch (IOException ioe) {
                throw new JspTagException(ioe.getMessage());
            }

     in RedirectSupport:
        try {
            response.sendRedirect(result);
        } catch (java.io.IOException ex) {
            throw new JspTagException(ex.toString());
        }

     We could definitely do the following instead, to expose
     the root cause:

                throw new JspException(ioe.getMessage(), ioe);

     However, please note that these rare IO exceptions
     most likely point to a system problem whose trace
     won't be of much use to the developer.

   2.3: System software exceptions (e.g. SQLException, SAXPathException)
     For example, in TransactionTagSupport:
        ...
        } catch (SQLException e) {
            throw new JspTagException(
                Resources.getMessage("ERROR_GET_CONNECTION",
                                     e.getMessage()));
        } 

     We could definitely throw the following instead:
            throw new JspException(
                Resources.getMessage("ERROR_GET_CONNECTION",
                                     e.getMessage()), e);

     However, please note that the root cause exception will simply
     point into JDBC code (or SAXPath...), which won't be of much use to the 
     developer.

3. interfaces specific to the 'standard' implementation

   ConditionalTagSupport exposes method condition() which throws
   a JspTagException.

   Agree that it might have been better to expose a JspException to
   allow a developer that subclasses ConditionalTagSupport to include
   the root cause of an exception. This will be possible with JSTL 1.1
   because JspTagException will support the constructor with both
   message and root cause exception.


As you can see from the above analysis, there does not seem to be
any situation where "custom" code is involved, and whose failure
(root cause exception) would be important to propagate up the stack.

We used to have such a situation, and this has been fixed in 
bug 15495.

OK, let's get practical now (hopefully, you're still reading :-)

For standard 1.0.3

Because we're very close to an official release of JSTL 1.0.3
we're not planning to change anything right now, unless
you feel there are specific instances where a root cause 
exception should be propagated up the stack (similar to the
issue in bug 15495). Let us know quickly though, since we're 
quite close to releasing.

For standard 1.1 (which will require JSP 2.0)

Since there's no harm in passing the exception to the new
JSP 2.0 JspTagException constructor, we'll fix all occurrences
where we throw a JspTagException and there is a root cause exception
available (even if we can't really see how it would help
most users).

Another thing we also noted is that we are not consistent in the way we
return the message associated with an exception. We use both toString() and
getMessage(). toString() seems better because it gives the name
of the Exception class in the string returned (usually helpful). 
We will do a global fix for consistency.
Comment 2 Heath Chiavettone 2003-02-07 19:54:39 UTC
Yes I read the whole thing.  And to be honest, its been a while since I 
encountered the situation where the root cause was being swallowed.  After 
reading you analysis, I may have misrepresented the issue.  Ultimately, the 
issue is that somewhere in JSTL, an exception was caught, the "message" 
or "toString" of the exception was used to construct a new exception and the 
root cause lost.  I'm not sure if your analysis covered the WhenTag's catching 
of the JspException and converting that to a JspTagException.  Or perhaps it 
the pattern shown below which was causing me grief, since the root cause parse 
exception is lost when the ElException is thrown?:

      catch (ParseException exc) {
	throw new ELException 
	  (formatParseException (pExpressionString,
				 exc));
      }
Comment 3 Heath Chiavettone 2003-02-25 02:08:02 UTC
After further review, the biggest problem is losing exception while evaluating 
expression language parameter.  It seems there are several levels of try 
catches occuring.  First an exception is thrown and caught and turned into a 
ParseException, which is caught and turned into an ELException, which is 
caught and turned into a JspException.  At the heart of things, I've lost the 
original exception (in this case an NPE in the getFamilyImageIcon() method 
which took a debugging session to find).  See the stack trace below:

javax.servlet.jsp.JspException: An error occurred while evaluating custom 
action attribute "value" with value "${design.familyImageIcon}": An error 
occurred while getting property "familyImageIcon" from an instance of class 
<someclass>
        at org.apache.taglibs.standard.lang.jstl.Evaluator.evaluate
(Evaluator.java:140)
        at 
org.apache.taglibs.standard.lang.support.ExpressionEvaluatorManager.evaluate
(ExpressionEvaluatorManager.java:112)
        at org.apache.taglibs.standard.tag.el.core.ExpressionUtil.evalNotNull
(ExpressionUtil.java:85)
        at org.apache.taglibs.standard.tag.el.core.OutTag.evaluateExpressions
(OutTag.java:137)
        at org.apache.taglibs.standard.tag.el.core.OutTag.doStartTag
(OutTag.java:95)
...
Comment 4 Pierre Delisle 2003-02-26 22:02:33 UTC
See bug 15873 for a description of this issue and how it
relates to the container being used.
Comment 5 Pierre Delisle 2003-08-04 17:48:41 UTC
Fixed in 1.1: 
Consistent use JspTagException:
   - always include original exception when available
   - use ex.toString() instead of getMessage()