Why throwing an EJBException is a "recommended" practice?

Question

I keep getting this "suggestion" from many fellow developers over and over again. In my experience I've found that EJBExceptions are well-suited for "end of the world" from the bean instance perspective (like when something is so wrong that the bean instance cannot recover by itself). If an instance can recover, I think it's better to throw an application exception.

Here is the pattern that I meet over and over again:

private SomeResource resource;
ejbCreate:
    resource = allocateResource(...);

omMessage:
    try {
    ...
    } catch (JMSException e) {
        throw new EJBException(e);
    }

ejbRemove:
    freeResource(resource);

IMHO this is a antipattern that causes resource leaks.

EDIT: Specifically, the EJB specification says that if a bean throws a runtime exception (and EJBException is a runtime exception) from the business method, then the bean is discarded without calling ejbRemove on it.

Is this a relevant example against throwing an EJBException? What are the relevant cases when EJBException should be thrown?

Answer 1

Whether or not is leaks resources depends on how those resources are administered. The discarded bean gets garbage collected giving up its reference to the resource which gets garbage collected in turn when no more references point to it. If the resource is also kept in a collection with a path to root, it will leak unless the container has a pool manager that can detect idle resources and recycle it.

I agree with you that the bean should handle any exceptions it can by itself and let the result be known to the caller. For me the anti pattern in the situation you describe is using exceptions to pass object state. Exceptions should be exceptions, not used as return values.

Answer 2

The throwing of EJBException is recommended by the EJB spec (14.2.2 in the EJB 3) in the cases where the EJB cannot recover from an exception it encounters. The spec also says that the EJB can reasonably allow (unchecked) System Exceptions to propogate to the container.

I agree with your reading of the spec than in such cases life-cycle callback methods will not be invoked by the containedr, and hence your concern that any resource-tidy up that would normally happen in the ejbRemove() callback will not happen and so there's a danger of resource leakage.

My experience is that very many tricky problems arise because of a lack of defensive coding. "Situation X cannot occur in a well behaved system, and if it does then the whole system is broken, so I'll not code for that eventuality." Then we get some "interesting" alignment of the stars and operator errors and the "can't happen" happens several times in quick succession and unanticipated side-effects kick in leading to really difficult to diagnose problems.

Hence I would say:

1. Do everything you can to leave the Bean instance in a state where the next invocation of a business method might be able to work. This might mean catching exceptions and closing resoruces that are in error. In this case you may then chose to tell the callers, "sorry guv, that request didn't work, but maybe if you retry later ..." I do that by throwing a TransientException checked excption.
2. If you have no important hhousekeeping in ejbRemove then you can allow SystemExceptions to propogate - but I'm not sure that this is friendly. You depend upon a library and it throws a NullPointerException. Is is actually more robust to catch and rethrow TransientException?
3. If you do have important housekeeping, then I think you do need to catch all exceptions and at least attempt the cleanup. You may then choose to rethrow EjbException or a system exception so that the instance is destroyed, but at least you've tried to do the housekeeping.

Answer 3

The container can still commit the current transaction even though the EJB method has thrown an application exception. If your EJB method can throw an application exception then you should consider using EJBContext.setRollbackOnly() like this:

public void method() throws ApplicationException {
    try {
    ...
    } catch (ApplicationException e) {
        _context.setRollbackOnly();
        throw e;
    }
}

Throwing an EJBException or a system (unchecked) exception means that the container will automatically rollback the transaction. See: http://java.sun.com/j2ee/tutorial/1_3-fcs/doc/Transaction3.html

If your system's EJBs do not routinely trap and handle application exceptions then code that throws them can result in a partially complete operation committing its changes.

This can lead to hard-to-find bugs because the "user myth" of an EJB method with CMP is that it maps to a transaction, which is atomic. The partially complete business logic is atomically committed to the database. This is extremely confusing when it happens in practice. Joel's article on leaky abstractions: http://www.joelonsoftware.com/articles/LeakyAbstractions.html explains why.

If your system has EJBs that do not handle application exceptions correctly the logical answer is to fix them so that they do. Until the problem is fixed, your team might have a rational reason for not wanting application exceptions thrown from EJBs.