EJB.next Interceptor Improvements - Method signatures

Interceptors are great, but some parts of them are odd when applied practically. For as fundamentally reflective as interceptors are, it isn't possible to pass all interceptable calls through a single method, like so:

import javax.annotation.PostConstruct;
import javax.annotation.PreDestroy;
import javax.ejb.PostActivate;
import javax.ejb.PrePassivate;
import javax.interceptor.AroundInvoke;
import javax.interceptor.AroundTimeout;
import javax.interceptor.InvocationContext;
import java.util.concurrent.TimeUnit;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.NANOSECONDS;

public class StatsInterceptor {

    @PostConstruct
    @PreDestroy
    @PrePassivate
    @PostActivate
    @AroundInvoke
    @AroundTimeout
    public Object intercept(InvocationContext context) throws Exception {
        final long start = System.nanoTime();
        try {
            return context.proceed();
        } finally {
            final long nanos = System.nanoTime() - start;
            final long millis = MILLISECONDS.convert(nanos, NANOSECONDS);
            System.out.println("invoke time "+ millis +" milleseconds");
        }
    }
}

This looks very nice, but currently is not legal. The spec currently requires different formats for callback-based interception vs business-method-based interception. This sounds like a nice safe and conservative rule, but is it benefiting users? Not really. Let's see what we have to do to our interceptor to make it compliant.

First, callback intercept methods are not allowed to return anything and must have a void return type. The logic here is that since callbacks have no return value, interceptors should know this. So fine, let's work around that and just make a new method:

    @PostConstruct
    @PreDestroy
    @PrePassivate
    @PostActivate
    @AroundInvoke
    public void callback(InvocationContext context) throws Exception {
        intercept(context);
    }

Second, callback intercept methods are not allowed to throw any exceptions. Again the rationale is that callbacks aren't allowed to throw checked exceptions, so interceptors should be aware of that as well. The trick is we can't simply remove the 'throws Exception' clause without doing any extra work. The InvocationContext.proceed() method throws java.lang.Exception but we can't, so we are forced to take responsibility for it. If we just catch and ignore the exception all hell will break loose, so instead we should at least wrap it as a RuntimeException.

    @PostConstruct
    @PreDestroy
    @PrePassivate
    @PostActivate
    @AroundTimeout
    public void callback(InvocationContext context) {
        try {
            intercept(context);
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

We've just made a mistake. Did you see it? If there are several interceptors in the chain and all of them wrap the exception like this, when an actual exception is thrown it will end up wrapped N times -- once per interceptor -- before finally reaching the container. Instead, we should at least check if the exception needs wrapping and hope that other interceptors do the same:

    @PostConstruct
    @PreDestroy
    @PrePassivate
    @PostActivate
    @AroundTimeout
    public void callback(InvocationContext context) {
        try {
            intercept(context);
        } catch (Exception e) {
            if (e instanceof RuntimeException) {
                throw (RuntimeException) e;
            } else{
                throw new RuntimeException(e);
            }
        }
    }

Now we're done. Summary: all of the above is boilerplate code you need in any well implemented callback-based interceptor.

What have we achieved? Not much.

In terms of user code, it doesn't help you any to have these restrictions. If you wanted special logic for callback-based interceptor methods you can easily put that annotation on a separate method.

In terms of container code, there's little gain as well. Ultimately, most containers use reflection to kick off the interceptor chain. The java.lang.reflect.Method.invoke() method returns Object and throws InvocationTargetException. Restricting users from returning Object and throwing Exception have no real impact on the container code. It's a callback. Ultimately all exceptions, checked or not, go into the log file and no further and return values are simply ignored.

Summary: The restriction on returning void and throwing Exception should be lifted. For backwards compatibility we can just say this former requirement is now optional and allow interceptor signatures of 'Object <method>(InvocationContext c) throws Exception' even if what is being intercepted is a callback.