Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Guava: Throwables.propagate and InterruptedException

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?

like image 238
Aled Sage Avatar asked Oct 10 '12 12:10

Aled Sage


1 Answers

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

like image 117
Chris Povirk Avatar answered Nov 10 '22 20:11

Chris Povirk