Catch and handle Errors raised by PromiseListeners in SettablePromiseImpl

When a settable promise has been resolved we notify all listeners and
then wake up any waiting threads by counting down on the _awaitLatch.
However, if one of the listeners raised an Error (not Exception) it
would skip subsequent listeners and count down on the _awaitLatch. This
change ensures that Errors are caught and do not abnormally terminal
notifications during SettablePromiseImpl's resolution.

Author: cpettitt-linkedin

src/com/linkedin/parseq/promise/SettablePromiseImpl.java

@@ -145,11 +145,17 @@ private void doFinish(T value, Throwable error) throws PromiseResolvedException
 
   private void notifyListener(final PromiseListener<T> listener)
   {
+    // We intentionally catch Throwable around the listener invocation because
+    // it will cause the notifier loop and subsequent count down in doFinish to
+    // be skipped, which will certainly lead to bad behavior. It could be argued
+    // that the catch should not apply for use of notifyListener from
+    // addListener, but it seems better to err on the side of consistency and
+    // least surprise.
     try
     {
       listener.onResolved(this);
     }
-    catch (Exception e)
+    catch (Throwable e)
     {
       LOGGER.warn("An exception was thrown by listener: " + listener.getClass(), e);
     }

test/com/linkedin/parseq/promise/TestSettablePromise.java

@@ -323,4 +323,45 @@ public void onResolved(Promise<String> resolvedPromise)
     latch.await();
     assertEquals("done!", resultRef.get());
   }
+
+  @Test
+  public void testListenerThrowsException() throws InterruptedException
+  {
+    // This test ensures that we properly resolve the promise even when
+    // one of its listeners throws an Error.
+    final SettablePromise<String> promise = Promises.settable();
+    promise.addListener(new PromiseListener<String>()
+    {
+      @Override
+      public void onResolved(Promise<String> promise)
+      {
+        throw new RuntimeException();
+      }
+    });
+
+    promise.done("Done!");
+    assertTrue(promise.await(5, TimeUnit.SECONDS));
+    assertEquals("Done!", promise.get());
+  }
+
+
+  @Test
+  public void testListenerThrowsError() throws InterruptedException
+  {
+    // This test ensures that we properly resolve the promise even when
+    // one of its listeners throws an Error.
+    final SettablePromise<String> promise = Promises.settable();
+    promise.addListener(new PromiseListener<String>()
+    {
+      @Override
+      public void onResolved(Promise<String> promise)
+      {
+        throw new Error();
+      }
+    });
+
+    promise.done("Done!");
+    assertTrue(promise.await(5, TimeUnit.SECONDS));
+    assertEquals("Done!", promise.get());
+  }
 }