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.