Hey

Dan OConnor wrote:
> 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.

Thanks :-) 

> 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?

Quite correct :-) Will fix. Uhm.. I mean.. "NYI"..

> 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.)

Same as above.

> 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?  

ServerException is a subclass of RemoteException. Am I missing something
here? (or have you?)

> (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;
>             }

Good point. This should be translated into a ServerException.

> 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.

Correct. Any good ideas on how to do this hookup between the container
and the resource managers?

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

LogInterceptor.

> 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.

The basic reason is that exceptions are such a fundamental piece of a
call that it is hard to extract it out into a sepatate interceptor. For
example, the information that a tx was present so that a
TransactionRolledBackException should be thrown is only available for
the TxInterceptor so it is natural that it flags this. And similar for
the other cases. However, we do have to go through the interceptors very
carefully and see if the proper handling is done in all places. This is
one of the trickier parts of handling a method call, and will take some
serious thinking to get right. I'm happy to see that you *have* done
some thinking about this, and am on the right track. Please file these
things in BugZilla so we know they won't be forgotten in this frenzy of
postings.

regards,
  Rickard

-- 
Rickard �berg

@home: +46 13 177937
Email: [EMAIL PROTECTED]
http://www.telkel.com
http://www.jboss.org
http://www.dreambean.com

Reply via email to