Hi Rickard,

I'm really enjoying reading the code you've written so far.  I finally 
have some time to look it over, and the themes of what you've 
written--dynamic configuration through JMX; chains of well-defined 
interceptors implementing the EJB services--are expressed through 
the whole project.  Really nice.

I'm looking at the code that exists for exception handling from 
business methods.  I know it's a work in progress, but I have a few 
questions anyway. Sorry for any questions arising out of the "not 
implemented" case.  :-)

You have exception handling code in both TxInterceptor and 
EntityInstanceInterceptor (with plans, I assume, to add similar 
code to Stateful/StatelessInstanceInterceptors).

Question 1:  In the TxInterceptor, your code rethrows 
ServerExceptions if RemoteException, RuntimeException, or 
Errors are thrown.  If this is meant for delivery to the caller, 
shouldn't you throw TransactionRolledBackException if the method 
is executing in an existing transaction?

Question 2:  Shouldn't the exception handling code to roll back the 
transaction exist for the supports and mandatory cases as well?  
(Depending on our handling of the "unspecified transaction" case, 
we might require it for NotSupported and Never as well.  In any 
case, if the TxInterceptor is translating the exception for delivery to 
the client, it is required for every case.)

Question 3: The XXXInstanceInterceptor class will get the 
ServerException from the TxInterceptor for those cases where the 
exception is translated, and rethrow this to the caller.  However, 
the XXXInstanceInterceptor also handles RuntimeException and 
Error.  When would this get translated to a RemoteException for 
the caller?  (I notice that in the container interceptor--at least for 
stateless session beans--still handles the case of Error rather than 
indicating an "internal server error":

            // Invoke and handle exceptions
            try
            {
               return m.invoke(StatelessSessionContainer.this, new 
Object[] { method, args, ctx });
            } catch (InvocationTargetException e)
            {
               Throwable ex = e.getTargetException();
               if (ex instanceof Exception)
                  throw (Exception)ex;
               else
                  throw (Error)ex;
            } 

Question 4:  I see where you are ejecting the bean instance from 
the container, in EntityInstanceInterceptor at least.  But when I 
follow the calls, it just comes down to removing the instance from a 
Map.  Any resources (such as open database connections) that 
this instance has open will stay open until the bean is actually 
garbage collected.  We should be able to free the resources 
obtained from a factory in the bean's namespace.  We can't 
depend on an EJB being parsimonious with their system 
exceptions.

Question 5: Where is the logging done for system exceptions?  I 
may have just missed it.

Finally, I'm curious why you didn't implement an 
ExceptionInterceptor rather than try to place this functionality in 
other tangentially related interceptors such as the one for 
transactions.  It seems like it would be more consistent with the 
rest of the architecture to have its own interceptor; certainly the 
information necessary to implement it would be available.  It would 
need to go before the TxInterceptor in the chain, so that it could 
mark the transaction for rollback if necessary.



Reply via email to