What is the best way to handle an ExecutionExcepti

Question

I have a method that performs some task with a timeout. I use the ExecutorServer.submit() to get a Future object, and then I call future.get() with a timeout. This is working fine, but my question is the best way to handle checked exceptions that can be thrown by my task. The following code works, and preserves the checked exceptions, but it seems extremely clumsy and prone to break if the list of checked exceptions in the method signature changes.

Any suggestions on how to fix this? I need to target Java 5, but I'd also be curious to know if there are good solutions in newer versions of Java.

public static byte[] doSomethingWithTimeout( int timeout ) throws ProcessExecutionException, InterruptedException, IOException, TimeoutException {

    Callable<byte[]> callable = new Callable<byte[]>() {
        public byte[] call() throws IOException, InterruptedException, ProcessExecutionException {
            //Do some work that could throw one of these exceptions
            return null;
        }
    };

    try {
        ExecutorService service = Executors.newSingleThreadExecutor();
        try {
            Future<byte[]> future = service.submit( callable );
            return future.get( timeout, TimeUnit.MILLISECONDS );
        } finally {
            service.shutdown();
        }
    } catch( Throwable t ) { //Exception handling of nested exceptions is painfully clumsy in Java
        if( t instanceof ExecutionException ) {
            t = t.getCause();
        }
        if( t instanceof ProcessExecutionException ) {
            throw (ProcessExecutionException)t;
        } else if( t instanceof InterruptedException ) {
            throw (InterruptedException)t;
        } else if( t instanceof IOException ) {
            throw (IOException)t;
        } else if( t instanceof TimeoutException ) {
            throw (TimeoutException)t;
        } else if( t instanceof Error ) {
            throw (Error)t;
        } else if( t instanceof RuntimeException) {
            throw (RuntimeException)t;
        } else {
            throw new RuntimeException( t );
        }
    }
}

=== UPDATE ===

Many people posted responses that recommended either 1) re-throwing as a general exception, or 2) re-throw as an unchecked exception. I don't want to do either of these, because these exception types (ProcessExecutionException, InterruptedException, IOException, TimeoutException) are important - they will each be handled differently by the calling processed. If I were not needing a timeout feature, then I would want my method to throw these 4 specific exception types (well, except for TimeoutException). I don't think that adding a timeout feature should change my method signature to throw a generic Exception type.

Answer 1

Here goes my answer. Let's suppose this code

public class Test {

    public static class Task implements Callable<Void>{

        @Override
        public Void call() throws Exception {
            throw new IOException();
        }

    }

    public static class TaskExecutor {

        private ExecutorService executor;

        public TaskExecutor() {
            this.executor = Executors.newSingleThreadExecutor();
        }

        public void executeTask(Task task) throws IOException, Throwable {
            try {
                this.executor.submit(task).get();
            } catch (ExecutionException e) {
                throw e.getCause();
            }

        }

    }



    public static void main(String[] args) {
        try {
            new TaskExecutor().executeTask(new Task());
        } catch (IOException e) {
            System.out.println("IOException");
        } catch (Throwable e) {
            System.out.println("Throwable");
        }
    }


}

IOException will be printed. I think it is an acceptable solution with the downside of throwing and catching Throwable forcefully and that the final catch can be reduced to

} catch (Throwable e) { ... }

Also, another chance is doing it in the following way

public class Test {

public static class Task implements Callable<Void>{

    private Future<Void> myFuture;

    public void execute(ExecutorService executorService) {
        this.myFuture = executorService.submit(this);
    }

    public void get() throws IOException, InterruptedException, Throwable {
        if (this.myFuture != null) {
            try {
                this.myFuture.get();
            } catch (InterruptedException e) {
                throw e;
            } catch (ExecutionException e) {
                throw e.getCause();
            }
        } else {
            throw new IllegalStateException("The task hasn't been executed yet");
        }
    }

    @Override
    public Void call() throws Exception {
        throw new IOException();
    }

}

public static void main(String[] args) {
    try {
        Task task = new Task();
        task.execute(Executors.newSingleThreadExecutor());
        task.get();
    } catch (IOException e) {
        System.out.println("IOException");
    } catch (Throwable e) {
        System.out.println("Throwable");
    }
}

}

Answer 2

I wouldn't say I recommend this, but here is a way you can do it. It is type-safe and whoever comes to modify it after you will probably be unhappy with it.

public class ConsumerClass {

    public static byte[] doSomethingWithTimeout(int timeout)
            throws ProcessExecutionException, InterruptedException, IOException, TimeoutException {
        MyCallable callable = new MyCallable();
        ExecutorService service = Executors.newSingleThreadExecutor();
        try {
            Future<byte[]> future = service.submit(callable);
            return future.get(timeout, TimeUnit.MILLISECONDS);
        } catch (ExecutionException e) {
            throw callable.rethrow(e);
        } finally {
            service.shutdown();
        }
    }

}

// Need to subclass this new callable type to provide the Exception classes.
// This is where users of your API have to pay the price for type-safety.
public class MyCallable extends CallableWithExceptions<byte[], ProcessExecutionException, IOException> {

    public MyCallable() {
        super(ProcessExecutionException.class, IOException.class);
    }

    @Override
    public byte[] call() throws ProcessExecutionException, IOException {
        //Do some work that could throw one of these exceptions
        return null;
    }

}

// This is the generic implementation. You will need to do some more work
// if you want it to support a number of exception types other than two.
public abstract class CallableWithExceptions<V, E1 extends Exception, E2 extends Exception>
        implements Callable<V> {

    private Class<E1> e1;
    private Class<E2> e2;

    public CallableWithExceptions(Class<E1> e1, Class<E2> e2) {
        this.e1 = e1;
        this.e2 = e2;
    }

    public abstract V call() throws E1, E2;

    // This method always throws, but calling code can throw the result
    // from this method to avoid compiler errors.
    public RuntimeException rethrow(ExecutionException ee) throws E1, E2 {
        Throwable t = ee.getCause();

        if (e1.isInstance(t)) {
            throw e1.cast(t);
        } else if (e2.isInstance(t)) {
            throw e2.cast(t);
        } else if (t instanceof Error ) {
            throw (Error) t;
        } else if (t instanceof RuntimeException) {
            throw (RuntimeException) t;
        } else {
            throw new RuntimeException(t);
        }
    }

}

Answer 3

Here's what I do in this situation. This accomplishes the following:

• Re-throws checked exceptions without wrapping them
• Glues together the stack traces

Code:

public <V> V waitForThingToComplete(Future<V> future) {
    boolean interrupted = false;
    try {
        while (true) {
            try {
                return future.get();
            } catch (InterruptedException e) {
                interrupted = true;
            }
        }
    } catch (ExecutionException e) {
        final Throwable cause = e.getCause();
        this.prependCurrentStackTrace(cause);
        throw this.<RuntimeException>maskException(cause);
    } catch (CancellationException e) {
        throw new RuntimeException("operation was canceled", e);
    } finally {
        if (interrupted)
            Thread.currentThread().interrupt();
    }
}

// Prepend stack frames from the current thread onto exception trace
private void prependCurrentStackTrace(Throwable t) {
    final StackTraceElement[] innerFrames = t.getStackTrace();
    final StackTraceElement[] outerFrames = new Throwable().getStackTrace();
    final StackTraceElement[] frames = new StackTraceElement[innerFrames.length + outerFrames.length];
    System.arraycopy(innerFrames, 0, frames, 0, innerFrames.length);
    frames[innerFrames.length] = new StackTraceElement(this.getClass().getName(),
      "<placeholder>", "Changed Threads", -1);
    for (int i = 1; i < outerFrames.length; i++)
        frames[innerFrames.length + i] = outerFrames[i];
    t.setStackTrace(frames);
}

// Checked exception masker
@SuppressWarnings("unchecked")
private <T extends Throwable> T maskException(Throwable t) throws T {
    throw (T)t;
}

Seems to work.

Answer 4

I've found one way to solve the issue. If it's ExecutionException you can get original one by calling exception.getCause() Then you need to wrap your exception in some kind of Runtime Exception or (what is the best way for me) use @SneakyThrows annotation from project lombok (https://projectlombok.org/). I give a small piece of code example. In addition you can add a few instanceof checks before throwing an exception to be sure this is the one you're expecting.

@SneakyThrows
public <T> T submitAndGet(Callable<T> task) {
    try {
        return executor.submit(task).get(5, TimeUnit.SECONDS);
    } catch (InterruptedException | ExecutionException | TimeoutException e) {
        throw e.getCause();
    }
}

Answer 5

I'm afraid there's no answer to your problem. Basically, you are launching a task in a different thread than the one you are in, and want to use the ExecutorService pattern to catch all the exceptions that task can throw, plus the bonus of interrupting that task after a certain amount of time. Your approach is the right one : you couldnt do that with a bare Runnable.

And this exception, that you have no information about, you want to throw it again, with a certain type : ProcessExecutionException, InterruptedException or IOException. If it's another type, you want to rethrow it as a RuntimeException (which is btw not the best solution, since you dont cover all the cases).

So you have an impendance mismatch there : a Throwable on one hand, and a known exception type on the other. The only solution you have to solve it is to do what you've done : check the type, and throw it again with a cast. It can be written differently, but will look the same in the end...

Answer 6

Here is another way to do it, though I'm not convinced that this is less clumsy or less prone to break than to do it with an instanceof check as in your question:

public static byte[] doSomethingWithTimeout(int timeout)
        throws ProcessExecutionException, InterruptedException, IOException, TimeoutException {
    ....
    try {
        ....
        return future.get(1000, TimeUnit.MILLISECONDS);
        .....
    } catch (ExecutionException e) {

        try {
            throw e.getCause();
        } catch (IOException ioe) {
            throw ioe;
        } catch (InterruptedException ie) {
            throw ie;
        } catch (ProcessExecutionException pee) {
            throw pee;
        } catch (Throwable t) {
            //Unhandled exception from Callable endups here
        }

    } catch (TimeoutException e) {
        throw e;
    } catch.....
}