Bug in error page handling?

[Gunther Schadow <gu...@aurora.rg.iupui.edu>]

Hi,

in jakarta-tomcat v3.0

the errorPage directive does not comply to its specification.  The
spec say that the error page will catch the most general exception,
the "Throwable", when in fact it only handles regular "Exceptions"!
This is most unfortunate since you wannabe able to catch all those
Errors too, that you throw while developing.

I hunted after this bug in the source code and found:

org.apache.jasper.compiler.JspParserEventListener.generateFooter()

where the following two lines made me think:

  //writer.println("} catch (Throwable t) {");
  writer.println("} catch (Exception ex) {");

this means someone has deliberately violated the spec and did so
temporarily with the intention to solve a problem and then come
back bring everything into conformance again. However, this person
may have got distracted.

Does someone take care about it or should I hack on this? I really
didn't want to get involved in yet another meta-software development
project. Gee, I'll give it one shot.

regards
-Gunther

Re: Bug in error page handling?

[Hans Bergsten <ha...@gefionsoftware.com>]

Gunther Schadow wrote:
> 
> > I'm afraid the JSP spec says Exception, not Throwable, in the arg list for
> > PageContext.handlePageException(). I agree Throwable would be better, but
> > it requires a spec change., so we can't just change Tomcat (it's the RI,
> > remember).
> >
> > Eduardo, can you add this to the list of details to look at in JSP.next?
> 
> I guess we have a disagreement about what the specs are. Now I'm
> certainly not all up to date with everything but I take my knowledge
> from the following sources:
> 
> JavaServer Pages Specification; Version 1.1; November 30, 1999
> 
> see page 47 section 2.7.1.1 paragraph "errorPage" there it says Throwable
> very clearly!
> 
> also see the
> 
> JavaServer Pages (JSP) Technology Syntax (quick reference card) as of
> 8/99. There it says:
> 
> ...
> Implicit Objects
> ...
>   exception     java.lang.Throwable
> 
> so the intention of the authors is quite clear to me.  For some weird
> reason the javax.servlet.jsp.PageContext.handlePageExcpetion was defined
> erroneously.  Even the initial jakarta-tomcat code was correct and the
> changed to the worse, only because of the PageContext definition.
> 
> I think this is clearly an error of javax. ... PageContext.  All written
> documents tell you Throwable.  So one should ask the insiders why they
> ended up with Exception, and, if there's no good reason, one should
> just silently correct the mistake in javax.  It's backwards compatible
> anyway, at least from the user's point of view.  And how many other
> implementors of a PageContextImpl class are there?


Okay, you're right. But I'm looking at 6.1.4.2 (PageContext, Usage) on
page 120. There it says "The signature of this method is 
void handlePageException(Exception e) throws ServletException, IOException."

Since you found a direct reference to Throwable in section 2.7.1.1, and I 
also think Throwable is the right choice, I'm inclined to say that the 
6.1.4.2 section is in error (I know there was some discussion about this and 
I'm pretty sure it was decided it should be Throwable after all).

But still, since the spec is inconsistent, it would be best to get a 
clarification from Eduardo before the patch is committed.

Hans
-- 
Hans Bergsten		hans@gefionsoftware.com
Gefion Software		http://www.gefionsoftware.com

Re: Bug in error page handling?

[Gunther Schadow <gu...@aurora.rg.iupui.edu>]

> I'm afraid the JSP spec says Exception, not Throwable, in the arg list for
> PageContext.handlePageException(). I agree Throwable would be better, but
> it requires a spec change., so we can't just change Tomcat (it's the RI,
> remember).
> 
> Eduardo, can you add this to the list of details to look at in JSP.next?

I guess we have a disagreement about what the specs are. Now I'm 
certainly not all up to date with everything but I take my knowledge 
from the following sources:

JavaServer Pages Specification; Version 1.1; November 30, 1999

see page 47 section 2.7.1.1 paragraph "errorPage" there it says Throwable
very clearly!

also see the 

JavaServer Pages (JSP) Technology Syntax (quick reference card) as of
8/99. There it says:

...
Implicit Objects
...
  exception     java.lang.Throwable

so the intention of the authors is quite clear to me.  For some weird
reason the javax.servlet.jsp.PageContext.handlePageExcpetion was defined
erroneously.  Even the initial jakarta-tomcat code was correct and the
changed to the worse, only because of the PageContext definition.

I think this is clearly an error of javax. ... PageContext.  All written
documents tell you Throwable.  So one should ask the insiders why they
ended up with Exception, and, if there's no good reason, one should 
just silently correct the mistake in javax.  It's backwards compatible
anyway, at least from the user's point of view.  And how many other
implementors of a PageContextImpl class are there? 

regards
-Gunther

Re: Bug in error page handling?

[Hans Bergsten <ha...@gefionsoftware.com>]

Gunther Schadow wrote:
> 
> If I could add something, did a little more research, found that
> the below-mentioned edit had been made from revision 1.1 to 1.2
> (very early) and this is a dependence to the javax.jsp.PageContext
> class.  That means that a lot of change is needed upstream to
> make the PageContext.handlePageException(Exception ex) into a
> handlePageException(Throwable ex).  Anyway, I think it is well
> worth the effort, since you really don't want to have Errors
> unhandled or dumped to the System.err.
> 
> Can someone fix this? If I did, I'd change javax.jsp.PageContext
> but I'm not sure whether changing this interface is allowed? The
> change would be backwards compatible since an Exception is a
> Throwable. But I guess I still don't want to get involved.
> 
> -Gunther
> 
> I wrote:
> > Hi,
> >
> > in jakarta-tomcat v3.0
> >
> > the errorPage directive does not comply to its specification.  The
> > spec says that the error page will catch the most general exception,
> > the "Throwable", when in fact it only handles regular "Exceptions"!
> > This is most unfortunate since you wannabe able to catch all those
> > Errors too that you throw while developing.
> >
> > I hunted after this bug in the source code and found:
> >
> > org.apache.jasper.compiler.JspParserEventListener.generateFooter()
> >
> > where the following two lines made me think:
> >
> >   //writer.println("} catch (Throwable t) {");
> >   writer.println("} catch (Exception ex) {");
> >
> > this means someone has deliberately violated the spec and did so
> > temporarily with the intention to solve a problem and then come
> > back bring everything into conformance again. However, this person
> > may have got distracted.
> >
> > Does someone take care about it or should I hack on this? I really
> > didn't want to get involved in yet another meta-software development
> > project. Gee, I'll give it one shot.

I'm afraid the JSP spec says Exception, not Throwable, in the arg list for
PageContext.handlePageException(). I agree Throwable would be better, but
it requires a spec change., so we can't just change Tomcat (it's the RI,
remember).

Eduardo, can you add this to the list of details to look at in JSP.next?

Hans
-- 
Hans Bergsten		hans@gefionsoftware.com
Gefion Software		http://www.gefionsoftware.com

Fixed source code (was: Re: Bug in error page handling?)

[Gunther Schadow <gu...@aurora.rg.iupui.edu>]

All right I changed the sources so that they comply with the 
spec. Now the error page traps all Throwables.  Attached are
the three source files I had to change. The changes are minor
and are attached as one context diff file with three chunks.
Someone please review the implication and insert this into the
source tree.

thanks,
-Gunther

Gunther Schadow wrote:
> 
> If I could add something, did a little more research, found that
> the below-mentioned edit had been made from revision 1.1 to 1.2
> (very early) and this is a dependence to the javax.jsp.PageContext
> class.  That means that a lot of change is needed upstream to
> make the PageContext.handlePageException(Exception ex) into a
> handlePageException(Throwable ex).  Anyway, I think it is well
> worth the effort, since you really don't want to have Errors
> unhandled or dumped to the System.err.
> 
> Can someone fix this? If I did, I'd change javax.jsp.PageContext
> but I'm not sure whether changing this interface is allowed? The
> change would be backwards compatible since an Exception is a
> Throwable. But I guess I still don't want to get involved.
> 
> -Gunther
> 
> I wrote:
> > Hi,
> >
> > in jakarta-tomcat v3.0
> >
> > the errorPage directive does not comply to its specification.  The
> > spec says that the error page will catch the most general exception,
> > the "Throwable", when in fact it only handles regular "Exceptions"!
> > This is most unfortunate since you wannabe able to catch all those
> > Errors too that you throw while developing.
> >
> > I hunted after this bug in the source code and found:
> >
> > org.apache.jasper.compiler.JspParserEventListener.generateFooter()
> >
> > where the following two lines made me think:
> >
> >   //writer.println("} catch (Throwable t) {");
> >   writer.println("} catch (Exception ex) {");
> >
> > this means someone has deliberately violated the spec and did so
> > temporarily with the intention to solve a problem and then come
> > back bring everything into conformance again. However, this person
> > may have got distracted.
> >
> > Does someone take care about it or should I hack on this? I really
> > didn't want to get involved in yet another meta-software development
> > project. Gee, I'll give it one shot.
> >
> > regards
> > -Gunther
> 
>   --------------------------------------------------------------------------------
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: general-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: general-help@jakarta.apache.org

Re: Bug in error page handling?

[Gunther Schadow <gu...@aurora.rg.iupui.edu>]

If I could add something, did a little more research, found that
the below-mentioned edit had been made from revision 1.1 to 1.2
(very early) and this is a dependence to the javax.jsp.PageContext
class.  That means that a lot of change is needed upstream to
make the PageContext.handlePageException(Exception ex) into a
handlePageException(Throwable ex).  Anyway, I think it is well
worth the effort, since you really don't want to have Errors 
unhandled or dumped to the System.err.

Can someone fix this? If I did, I'd change javax.jsp.PageContext
but I'm not sure whether changing this interface is allowed? The
change would be backwards compatible since an Exception is a 
Throwable. But I guess I still don't want to get involved.

-Gunther 


I wrote:
> Hi,
> 
> in jakarta-tomcat v3.0
> 
> the errorPage directive does not comply to its specification.  The
> spec says that the error page will catch the most general exception,
> the "Throwable", when in fact it only handles regular "Exceptions"!
> This is most unfortunate since you wannabe able to catch all those
> Errors too that you throw while developing.
> 
> I hunted after this bug in the source code and found:
> 
> org.apache.jasper.compiler.JspParserEventListener.generateFooter()
> 
> where the following two lines made me think:
> 
>   //writer.println("} catch (Throwable t) {");
>   writer.println("} catch (Exception ex) {");
> 
> this means someone has deliberately violated the spec and did so
> temporarily with the intention to solve a problem and then come
> back bring everything into conformance again. However, this person
> may have got distracted.
> 
> Does someone take care about it or should I hack on this? I really
> didn't want to get involved in yet another meta-software development
> project. Gee, I'll give it one shot.
> 
> regards
> -Gunther