Add ExecutionList.setUncaughtExceptionHandler

[GoogleCodeExporter]

There is currently no mechanism in place to handle exceptions that are thrown 
in FutureCallbacks. This patch provides a "setUncaughtExceptionHandler" to 
ExecutionList, allowing these exceptions to be caught and handled cleanly.

https://codereview.appspot.com/6458061/

Original issue reported on code.google.com by simon.m.stewart on 14 Mar 2013 at 4:26

[GoogleCodeExporter]

"a better model may be having an Executor wrapper that handles uncaught 
exceptions rather than putting it into ExecutionList directly."

"+1 to having the Executor handle this issue."

Original comment by kurt.kluever on 14 Mar 2013 at 10:57

[GoogleCodeExporter]

What's your use-case? Are you using ExecutionList directly? Are you calling it 
via ListenableFutureTask?

Original comment by n...@google.com on 15 Mar 2013 at 7:22

[GoogleCodeExporter]

On Android, I believe that the original motivation was a FutureCallback that 
was throwing an exception in its onFailure() method. Normally, we would expect 
this sort of thing to crash the app so we could discover and fix it. Instead, 
the exception was logged and the app was just left in a bad state. We 
introduced this diff so that we would invoke 
ExecutionList.setUncaughtExceptionHandler() at startup so that it was passed a 
handler that was guaranteed to crash the app.

(I assumed this particular FutureCallback could only receive a specific type of 
Exception in our code, so I categorically did the cast without doing an 
instanceof check. I didn't account for the fact onFailure() could also get a 
CancellationException, so I got a ClassCastException at runtime in onFailure().)

I agree that making this a property of the Executor may be the cleaner 
solution, though (1) Executor is an interface that you/we don't control, and 
(2) it may require us to specify the UncaughtExceptionHandler in every place 
that we create an Executor. I'm sure we could rearchitect around that, though.

Original comment by bolinf...@gmail.com on 18 Mar 2013 at 11:36

[GoogleCodeExporter]

Though thinking about this more: what is the justification for ExecutionList 
swallowing this exception in the first place?

Original comment by bolinf...@gmail.com on 19 Mar 2013 at 7:48

[GoogleCodeExporter]

I suppose the comment in the code answers that question, but I'm not completely 
sure that I buy the answer.

Original comment by bolinf...@gmail.com on 19 Mar 2013 at 7:49

[GoogleCodeExporter]

Arguably our error was in making sameThreadExecutor() propagate exceptions. 
That interpretation suggests a workaround: Supply another sameThreadExecutor() 
that accepts an UncaughtExceptionHandler. (Our sameThreadExecutor() is more 
complex than it probably needs to be, mostly thanks to its support for the 
ExecutorService interface, including shutdown, so you could whip one up 
yourself.)

Original comment by cpov...@google.com on 19 Mar 2013 at 8:13

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:12

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:18

• Changed state: Migrated

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08

• Added labels: Restrict-AddIssueComment-Commit