It seems that ServletException was written before getCause() was added to Throwable (jdk 1.4), and thus has its own getRootCause(). So generic error handling is unnecessarily harder, stack traces incomplete, etc. when a ServletException is thrown; e.g. in the ApplicationFilterChain code. I think ServletException should be changed, but the behavior could be changed even if thats not possible. For example, when throwing a ServletException with a root cause exception, also call the initCause() method. It returns a reference to itself, so you could even do throw (ServletException)new ServletException(msg, e).initCause(e); (or something similar if that doesn't compile).
ServletException is defined by the servlet spec and therefore cannot be changed by the Tomcat team.
Firstly, ServletException could be fixed without a change to its interface, and it would not violate the Servlet spec. I don't know if the Tomcat team can do that or not, sounds like maybe not. Tomcat can and should be fixed independent of ServletException being fixed, and I stated my opinion of how in the bug description.
Only the spec team can change the interface or implementation of any javax.servlet.* classes.
Servlet v2.4 section SRV.1.2 ends with: J2SE 1.3 is the minimum version of the underlying Java platform with which servlet containers must be built. This minimum prevents using initCause(), which is not available in J2SE 1.3. This will be fixable in Servlet v2.5. Since Tomcat uses the JSR sponsored reference implementation, which I think has to stick with the mimimum, I'm marking this bug INVALID.
From the 5.5.9 release notes: Tomcat 5.5 is designed to run on J2SE 5.0 and later, and requires configuration to run on J2SE 1.4. Make sure to read the "RUNNING.txt" file in this directory if you are using J2SE 1.4. I take that to mean Tomcat won't run under java 1.3. I strongly doubt it is compiled with jdk1.3 either. Just to be clear, because I think the point is being missed, the fix I proposed is for Tomcat to invoke initCause(), until ServletException itself is fixed. I won't reopen the bug again, but in my opinion it is not invalid and should be fixed.
Tomcat 5.5.9 requires 1.4 or later, but the servlet-api.jar it uses is expected to be able to run with 1.3, per the spec. Thus, isn't not feasible to fix servlet-api.jar itself. Tomcat could call initCause() as you suggest, but if someone wrote a webapp that depended on getCause() not returning null, the webapp would likely not be portable. This is a case where "unnecessarily harder" is preferred to help ensure the portability of webapps. Since I would guess that you likely don't care about portability, you could customize your Tomcat to fix this. Simpler may be just to write a utility class with a static method that accepts a ServletException and returns the cause from getRootCause() or getCause(), which ever is not null. Updating the component, since this is where the primary issue lies.
*** Bug 36252 has been marked as a duplicate of this bug. ***
Guys, you don't need initCause() to fix this problem, nor do you need to change the interface or specification. All you need to do is implement a method public Throwable getCause() { return getRootCause(); } there you go. This code will run on J2EE 1.3 just fine and this is something that the Tomcat team can do independantly of the specification. I also don't understand what "portability" has anything to do with this (maybe you meant backwards compatibility?). The way I see it, if someone is coding a webapp on pre-1.3, he can't possibly invoke getCause() because it does not exist. If he codes his webapp post-1.3, he expects getCause()'s contract to be met and we are in fact trying to fix ServletException so it meets its contract. There is no loss of "portability" or backwards compatibility here. Can I reopen this issue?
(In reply to comment #8) > in fact trying to fix ServletException so it meets its contract. There is no > loss of "portability" or backwards compatibility here. Can I reopen this issue? No.
Remy, what is your reasoning for not applying this patch?
I understand the portability issue to be as follows: - This bug is fixed in Tomcat - I write code that depends on the fix - I port my app to another servlet container where the bug is not fixed - My app is slightly broken You could make this argument for any bug that exists in more than one servlet container. More people have encountered this bug and wasted time tracking it down than will ever have the hypothetical portability issue.
Right! I totally agree. Besides which, I have already reported this issue to Sun so they will be fixing it (eventually). I don't see any benefit in making everyone wait a couple of years for this...
This is not a bug - there is no violation of the Servlet spec here. It is an enhancement request. To reiterate a point the seems to have been missed, only the servlet spec team can change the interface or implementation of ServletException. We could, as suggested, change the Tomcat code to call initCause(). There are 72 or so calls that would need to be added. However, this would break application portability. The whole point of having a specification is portability. If we break it for this, why not for something else? If we do it for this, why shouldn't another vendor do it for something else. Such a change would be the beginning of a very nasty slippery slope.
Just for the record, I *did* file an enhancement request (see #36252) and it was closed as a duplicate of this issue... So I guess there is nothing for us to do but sit on our hands for the next couple of years...
Joshua Bloch recommended making the same change to ServletException as Gili just did, back in 2000, in anticipation of j2se1.4. Too bad it was ignored, probably because Joshua Bloch doesn't know anything about Java. http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4395719 Fixing this bug will not violate the spec. Calling getCause() is undefined behavior. It would be more useful if it did the right thing instead of the wrong thing. Furthermore, just because it doesn't violate the servlet spec doesn't mean its not a bug. That is a ridiculous position to take. According to the java spec, if I am using 1.4 or later, getCause() does the following: - Returns the cause of this throwable or null if the cause is nonexistent or unknown. - By returning null, ServletException is implying that the cause is nonexistent or unknown, which is not true. ServletException doesn't adhere to the java spec, and that is definitely a bug. Here is another snippet from the javadoc for Throwable.getCause(): - While it is typically unnecessary to override this method, a subclass can override it to return a cause set by some other means. This is appropriate for a "legacy chained throwable" that predates the addition of chained exceptions to Throwable. -
I agree, Tomcat's ServletException has a bug on JDK 1.4 and above, and adding getCause() would fix it without breaking any specification, portability, or backward compatibility. The suggested work-around to call both getRootCause() and getCause() and use whichever is not null will not work for printStackTrace(), which is often used for not only printing the cause chain but also for capturing it, e.g., in a StringPrintWriter.
I take it back. That fix would break backwards compatibility by causing an infinite loop in anything that extends ServletException and overrides getRootCause() to return getCause(). It's unfortunate that this wasn't fixed in the servlet spec for JDK 1.4, because now there may be work-arounds that any fix would break.
David, There is a perfectly valid workaround for this problem (that is both backwards and forwards-compatible) at: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4395719 If you agree with this workaround, I'd like to reopen this issue.
(In reply to comment #17) > I take it back. That fix would break backwards compatibility by causing an > infinite loop in anything that extends ServletException and overrides > getRootCause() to return getCause(). > You mean an infinite loop like in this bug?: http://issues.apache.org/bugzilla/show_bug.cgi?id=39088 Tomcat seems to be full of poor exception handling code, most of it centered around the exception chaining issue. If the developers would admit there are problems, fix them, and move on, Tomcat's quality would benefit, the developers who use it would benefit, and the world would be a better place. <rant> Instead we are stuck with this -"Works as designed" -"That may be a problem but only affects a small number of use cases" _bullshit_ mentality from the powers that be, and i have encountered it on every single bug I've submitted or encountered and commented on. Furthermore I am ready and willing to submit patches, and have done so, but they just go to the bit bucket in the sky, with no comments, criticism. Just crickets chirping. @see http://issues.apache.org/bugzilla/show_bug.cgi?id=39088 @see http://issues.apache.org/bugzilla/show_bug.cgi?id=33453 </rant>
Gili, that looks like a better solution, but maybe it should wait for a spec update because it would break existing code that calls initCause()? I've voted for that Sun bug. But, I'm not a Tomcat committer. Jonathan, that's a good example of the repercussions of this exception chaining discrepancy. The infinite loop I mentioned is a little different from the one you posted, because it's actually recursive, so harder to detect in a workaround.
David, we've been waiting for a specification update for years now. Notice the BugParade issue was filed in 2000 (!!). I don't understand what you meant in your last post, about the latest patch breaking applications that invoke initCause(). Our constructor would invoke initCause(), then user code can do what it wants with the resulting exception. If they choose to invoke initCause() on it, it'll simply override the exception we set. I'm sorry but I don't understand what problems could occur...? Also, I wanted to say I totally agree with Jonathan. More often than not, the Tomcat developers I've dealt gave (totally invalid) excuses for why they cannot be bothered to fix bugs. Maybe that's why Glassfish came along. Anyway, I look forward to some point in the future when this situation changes for the better.
(In reply to comment #21) > Also, I wanted to say I totally agree with Jonathan. More often than not, the > Tomcat developers I've dealt gave (totally invalid) excuses for why they cannot > be bothered to fix bugs. Maybe that's why Glassfish came along. Anyway, I look > forward to some point in the future when this situation changes for the better. I would be extremely glad if you were switching to Glassfish :) When do you plan to do it ? Also, let us know if you convince them to fix this issue.
If initCause() is called more than once, it throws IllegalStateException. So, anywhere already calling it on a ServletException would break. The infinite loop risk could be avoided by not calling getRootCause(): public Throwable getCause() { return rootCause; } For consistency with initCause(), maybe it should only default to rootCause? If null causes are ignored, it could be done with just: public Throwable getCause() { return super.getCause() == null ? rootCause : super.getCause(); } But, to allow initCause(null) to have some effect, ServletException would need to keep its own cause and override initCause() too. The first would be the same as in Throwable: private Throwable cause = this; public synchronized Throwable initCause(Throwable cause) { if (this.cause != this) throw new IllegalStateException("Can't overwrite cause"); if (cause == this) throw new IllegalArgumentException("Self-causation not permitted"); this.cause = cause; return this; } The last would default to rootCause instead of null: public Throwable getCause() { return (cause == this ? rootCause : cause); } This seems pretty compatible to me, with both backwards and specs. Can anyone think of any problems? I don't see anything resolving this in the Servlet 2.5 RM 5 spec.
Of course, allowing initCause() an effect would depend on JDK 1.4 or newer.
(In reply to comment #24) > Of course, allowing initCause() an effect would depend on JDK 1.4 or newer. For calling super.getCause(), I mean. My last suggestion is more compatible and doesn't introduce any JDK 1.4 dependency.
Tomcat 5.5 already depends on JDK 1.5 for the most part so this shouldn't be a problem. Remy, since when do you claim ownership of the Tomcat project? Last time I checked, it was open-source. Please check your ego at the door.
(In reply to comment #20) > Jonathan, that's a good example of the repercussions of this exception chaining > discrepancy. The infinite loop I mentioned is a little different from the one > you posted, because it's actually recursive, so harder to detect in a workaround. At least with a recursive infinite loop you will eventually blow up the stack and break out of it...
(In reply to comment #26) > Remy, since when do you claim ownership of the Tomcat project? Last time I > checked, it was open-source. Please check your ego at the door. The principle of meritocracy governing Apache (http://www.apache.org/foundation/how-it-works.html#meritocracy) has been officially replaced with the Remitocracy. Or I should say the Rempricktatorship.
(In reply to comment #22) > > I would be extremely glad if you were switching to Glassfish :) When do you plan > to do it ? Also, let us know if you convince them to fix this issue. > OK. I am letting you know. I am a little late though. The issue was fixed in Glassfish on August 22, 2005, more than 6 months before Remy made the above snide remark. https://glassfish.dev.java.net/source/browse/glassfish/servlet-api/src/jakarta-servletapi-5/jsr154/src/share/javax/servlet/ServletException.java And it probably was either Gili or me that convinced them to fix it. I had posted a comment to the related Java bug, and 4 days later it was fixed in Glassfish and replied to: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4395719. Gili also mentioned opening an issue with Sun. I also haven't seen the hypothetical portability issue materialize in the year and a half that Glassfish has been fixed and Tomcat hasn't.