Seeking opinions on ListenableFuture exception propagation

[I-guava]

So i have started using ListenableFuture mainly due to improved code readability but I'm not quite settled on how to deal with exception propagation. (Btw, all in context of java 6 / android / many futures which ultimately can fail with more or less the same set of errors).

Regardless of ListenableFuture specifics my basic exception strategy (for my current project) is to reduce all exception to a small set of errors codes by anticipating some common fail scenarios upon which the user might be able to take some corrective action (e.g . check your wifi connection) while logging and letting the app crash for all others exceptions.

My basic usage is:

final ListenableFuture<Result> future = executor.submit(new Callable<Result>() {

@Override

public Result call() throws Exception {

Result result = doSomething();

// maybe do some extra processing;

return result;

}

});

Result doSomething() {

Result result = null;

try{

result = doSomething();

}

catch(InterestingException1 e) {

throw new MyCatchedException(e, Error.Code1);

}

catch(InterestingException2 e) {

throw new MyCatchedException(e, Error.Code2);

}

catch(Exception e) {

throw new MyCatchedException(e, Error.Bug);

}

return result;

}

Futures.addCallback(future, new FutureCallback<Result>() {

@Override

public void onSuccess(Result result) {

callSomeNonFurtureListener(result, null); 

}

@Override

public void onFailure(Throwable throwable) {

callSomeNonFurtureListener(null, MyCatchedException);

}

}, MoreExecutors. ....);

The issue is with void onFailure(Throwable throwable) where essentially I lose my exception wrapper (the same with the Runnable callback version since I would need to call getCause on ExecutionException).

So currently I'm doing something like this:

MyCatchedException e = null;

if (throwable instanceof MyCatchedException)

e = (MyCatchedException) throwable;

else

e = new MyCatchedException(throwable,  Error.Bug);

I'm using error codes because it seems less verbose to check a fixed set of error codes at the listener, specifically the errors that I'm interested about in the context of a particular Future completion task, then to have each error code be a standalone exception.

As specified in the title I'm seeking for opinions/alternatives about this.

tnx.

-Itai

[Paul Mantyla]

What you're doing seems fine.  You're preserving the original exception and providing an error code to help interpret the error.  It seems like a common practice.  Is there something about your current approach that particularly bothers you?  If you have a lot of interesting exceptions, you could move the conversion logic to a helper method:

} catch (Exception e) {

  throw convertException(e);

}

And that helper method could even use a Map to map exception types to error codes.

Paul

[I-guava]

Tnx.

What feels a bit "bumpy" with this is that when onFailure is called I need to manually recover the information about why the future failed..

Futures.addCallback(future, new FutureCallback<Result>() {

@Override

public void onSuccess(Result result) {

callSomeNonFurtureListener(result, null); 

}

@Override

public void onFailure(Throwable throwable) {

MyCatchedException  e = fromThrowable(throwable);

callSomeNonFurtureListener(null, MyCatchedException);

}

}, MoreExecutors. ....);

fromThrowable(Throwable throwable) {

if (throwable instanceof MyCatchedException)

return (MyCatchedException) throwable;

else

return new MyCatchedException(throwable,  Error.Bug);

}

}

Maybe having a FutureException contract that would implicitly do this conversion would streamline this:

Futures.addCallback(future, new FutureCallback<Result, MyCatchedException>() {

@Override

public void onSuccess(Result result) {

callSomeNonFurtureListener(result, null); 

}

@Override

public void onFailure(MyCatchedException  exception) {

callSomeNonFurtureListener(null, exception);

}

}, MoreExecutors. ....);

Just a thought...

[Chris Povirk]

I glanced at 30 Google-internal implementors of
FutureCallback.onFailure. instanceof checks appeared in only 5. That's
a sizable minority (in a small sample size) but still small enough
that I'd feel bad making the other 25 users write
"FutureCallback<Throwable>," especially with the potential for writing
"FutureCallback<Exception>" or "FutureCallback<MyException>" and
failing to take any action if an Error or RuntimeException occurs.
(That's not to say that we it's a great idea to handle an Error the
same way as an expected exception, only that it may be better to do
that than to silently swallow it.)

Incidentally, we're considering parameterizing a class on the Future
exception type just as you describe here: It's just that we'd be
parameterizing FutureFallback rather than FutureCallback.

If you'd like to make a further pitch for parameterization of
FutureCallback, let's continue this discussion on a feature request:
http://code.google.com/p/guava-libraries/issues/entry

[I-guava]

Pitched with a small twist :)

https://code.google.com/p/guava-libraries/issues/detail?id=1489