tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
Subject RE: Handler/ServletWrapper/Error handling
Date Sat, 09 Dec 2000 16:04:21 GMT
Hi Larry,

> Suggestion 1) In Handler and ServletWrapper some calls are made to
> contextM.handleStatus() and contextM.handleError().  I think it would
> be an improvement to add handlerStatus() and handleError() to
> Handler, which would make the call to contextM.handleXXX.  This would
> allow, ServletWrapper could hook these if needed.  For example,
> a ServletWrapper.handleError() could check for UnavailableExceptions
> and take some action if desired.

Easy to do, seems a good idea. What about adding handleInitException() and
handleServiceException() - since a ClassNotFound will have different
effects ( 404 if in init, normal error if in service - the first means the
servlet wasn't found, the second may happen if the servlet does a
Class.forName with a class that is not found ). 

There is another hook I want to add, preInit() - that can be called before 
preServletInit / init / postServletInit. It'll be used to prepare the
handler for preServletInit ( i.e. loadServlet, check if still
unavailable). That would make a very clean implementation of unavailable,
and avoid calling pre/post too often. 

( the rule should be that preInit / init / postInit are called at least
once before service() - some people expect init() to be called only once,
but that's not true since UnavailableException may determine a another
attempt. )

> Suggesion 2) Since "inclusion", as far as I can tell, is only implemented
> in RequestDispatcherImpl it could be considered a Servlet 2.2 thing.
> With Suggestion 1 above, tests for res.isIncluded() could move to
> ServletWrapper.  I'm not sure if Handler needs to be "inclusion" aware.

That would be good for better separating the layers. Great !

> Question) Mainly to provide better default output for UnavailableExceptions, I was looking
to implement an interceptor in
> facade22, say Servlet22ErrorHandler, that could provide additional error
> handling support.  Preferrably it could override some of ErrorHandler's behavior, as
opposed to replacing ErrorHandler completely. There is a
> general problem with this with regard to the order the contextInit()
> and handleError() methods would be called.  Servlet22ErrorHandler would
> want its contextInit() called after ErrorHandler's, but would want its
> handleError() called before ErrorHandler's!?  Maybe handleError() methods
> could be called in reverse order of appearance in server.xml?  Or is
> ErrorHandler only replacable?  Ideas?

ErrorHandler is the result of refactoring the error processing code and an
attempt to make it more modular. It's the first round - I expect / hope
it'll be replaced with a better version based on what we learn. 

Regarding ordering/chaining of hooks, there are few options:

1. We can move ErrorHandler after Servlet22EH ( or just replace it ).

2. We can enhance the code to do what Apache2.0 does - accept a FIRST,
BEGGINING, END, LAST options for the hooks. That's the best solution, but
it's a bit complex ( and I would leave it at the end, after we finish
everything else in core / bug fixes / etc.). 

I think it's more important to finish the core objects and freeze the core
API  - and then work on individual modules and replace them if needed  -
that can continue after 3.3 is released, since modules can be
added/replaced without need for a tomcat release. 

> If the suggestions make sense in your patches go ahead and include them.


BTW, what about a more radical aproach - just remove init/destroy from
handler, leave only service() ( eventually renamed to "invoke" :-) ?
The whole init/destroy machine is servlet-specific - none of the internal
handlers are using it and the modules have access to all the Interceptor
notifications anyway. 

The only reason for keeping the init/destroy is to enforce the
pre/postInit calling - but we can move this responsibility to the

That would make Handler really easy, clean up and simplify


View raw message