Guava: Throwables.propagate and InterruptedException

Question

what is the best practice for handling InterruptedExceptions when using Throwables.propagate(e) in Guava?

I love using throw Throwables.propagate(e), especially in methods that throw no checked exceptions and where exception handling is the responsibility of the caller. But it doesn't do what I'd expect with InterruptedException.

I don't want to lose the fact that the thread was interrupted, so I end up writing things like:

public void run() {
    Callable c = ...;
    try {
        c.call();
    } catch (InterruptedException e) {
        Thread.currentThread().interrupt();
        throw Throwables.propagate(e);
    } catch (Exception e) {
        throw Throwables.propagate(e);
    }
}

Is there a way to do this in Guava? Is there a (backwards compatible?!) way to use something like Throwables.propagate() that sets the thread as interrupted, if it is wrapping and propagating an InterruptedException?

I would raise this as a feature request within Guava – artbristol Oct 10 '12 at 12:15 — artbristol, Oct 10 '12 at 12:15

score 8 · Answer 1 · edited Jul 28 '17 at 07:43

Conveniently, we discussed this internally a while back. I'll just copy and paste:

My hard-line opinion on Throwables.propagate(e) is that it's basically throw new RuntimeException(e) and that people usually shouldn't do it, just as they usually shouldn't write throw new RuntimeException(e). (And if they're going to write it, they might as well write it directly so that it's clear what's going on.)

My hard-line opinion on catch (Exception e) -- usually how people get themselves into this mess -- is that they usually shouldn't do that, either. (Obviously there are cases in which catch (Exception e) is obviously the right thing to do (basically any top-level, operation-scope catch block), but those are... obvious.)

My hard-line opinion on InterruptedException is that having InterruptedException implement Exception at all is broken in exactly this way: It requires special handling that other exceptions don't.

My hard-line opinion on converting InterruptedException to a RuntimeException is "don't." (This, like much of the other stuff I've said above, is controversial.)

So on the one hand, I'm not sure there's anything we can do to salvage propagate(). On the other hand, maybe it's nice to make the method a little less bad.

Then again, consider this caller, which catches ExecutionException e:

throw Throwables.propagate(e.getCause());

It would be wrong to interrupt the consumer thread, just as it would be wrong to throw e.getCause() directly, because the interruption was intended for the computing thread, not the consumer thread.

I'm inclined to leave propagate() alone. (As you can probably guess, I'm personally inclined to deprecate it, but that's a bigger discussion.)

+1 interesting points. I'm amazed that people write `throw Throwables.propagate(e.getCause());`! The stack trace would look like everything happened in a single thread! — artbristol, Oct 10 '12 at 17:20
Thanks, interesting points. +1 to InterruptedException implementing Exception being wrong, but Java is where it is. I think converting InterruptedException to a RuntimeException is sometimes essential: if my method signature is fixed by an external interface (so I can't throw InterruptedException), and my operation was interrupted (so I can't fulfil my contract), I must throw an exception. What other sensible option is there other than setting the thread as interrupted and throwing a RuntimeException? — Aled Sage, Oct 10 '12 at 22:45
For writing `throw new RuntimeException(e)` instead, it's annoying noise to have an exception wrapping the exception (especially when it's not adding anything useful, such as an additional message about the current context). Wherever that noise can be avoided (e.g. it's already an unchecked exception) then great. There's a school of thought that APIs should use unchecked exceptions wherever possible. Even if one doesn't buy that argument, one still has to implement other people's interfaces written in that way. So please don't deprecate `Throwables.propagate(e)`! — Aled Sage, Oct 10 '12 at 22:46
I think it is far better to `throw Throwables.propagate(e);` than to add the exception to the method signature when you deal with an API that throws a checked exception that you don't want to handle specifically. I think it is a good idiomatic way of saying "Nothing specific here, treat it as a normal execution problem". — Pierre-Henri, Oct 11 '12 at 11:12

Guava: Throwables.propagate and InterruptedException

1 Answers1