Fix issues with exception handling in futures. Review by @heathermiller

4 files changed, 19 insertions, 8 deletions

diff --git a/src/library/scala/concurrent/Future.scala b/src/library/scala/concurrent/Future.scala
index 89b04e6248..04f56950f1 100644
--- a/src/library/scala/concurrent/Future.scala
+++ b/src/library/scala/concurrent/Future.scala
@@ -312,7 +312,7 @@ trait Future[+T] extends Awaitable[T] {
           if (pf.isDefinedAt(v)) p success pf(v)
           else p failure new NoSuchElementException("Future.collect partial function is not defined at: " + v)
         } catch {
-          case t: Throwable => p complete resolver(t)
+          case NonFatal(t) => p complete resolver(t)
         }
     }
 
diff --git a/src/library/scala/concurrent/impl/ExecutionContextImpl.scala b/src/library/scala/concurrent/impl/ExecutionContextImpl.scala
index c308a59297..2b929d91ab 100644
--- a/src/library/scala/concurrent/impl/ExecutionContextImpl.scala
+++ b/src/library/scala/concurrent/impl/ExecutionContextImpl.scala
@@ -93,8 +93,12 @@ private[scala] class ExecutionContextImpl(es: AnyRef) extends ExecutionContext w
   }
 
   def reportFailure(t: Throwable) = t match {
+    /*TODO: clarify that resolver should wrap Errors, in which case we do not want
+      Error to be re-thrown here */
     case e: Error => throw e // rethrow serious errors
-    case t => t.printStackTrace()
+    case t =>
+      //println(t.toString)
+      t.printStackTrace()
   }
 
 }
diff --git a/src/library/scala/concurrent/impl/Future.scala b/src/library/scala/concurrent/impl/Future.scala
index 1cc9e95463..9743c37403 100644
--- a/src/library/scala/concurrent/impl/Future.scala
+++ b/src/library/scala/concurrent/impl/Future.scala
@@ -71,14 +71,18 @@ object Future {
   
   def apply[T](body: =>T)(implicit executor: ExecutionContext): Future[T] = {
     val promise = new Promise.DefaultPromise[T]()
+
+    //TODO: shouldn't the following be:
+    //dispatchFuture(executor, () => { promise complete Right(body) })
+
     executor.execute(new Runnable {
       def run = {
         promise complete {
           try {
             Right(body)
           } catch {
-            case NonFatal(e) =>
-              executor.reportFailure(e)
+            case e =>
+              //executor.reportFailure(e)
               scala.concurrent.resolver(e)
           }
         }
diff --git a/test/files/jvm/scala-concurrent-tck.scala b/test/files/jvm/scala-concurrent-tck.scala
index f0ca438774..8fcaceb3f0 100644
--- a/test/files/jvm/scala-concurrent-tck.scala
+++ b/test/files/jvm/scala-concurrent-tck.scala
@@ -64,7 +64,7 @@ trait FutureCallbacks extends TestBase {
       }
     }
   }
-  
+
   def testOnSuccessWhenFailed(): Unit = once {
     done =>
     val f = future[Unit] {
@@ -94,7 +94,7 @@ trait FutureCallbacks extends TestBase {
         assert(x == 1)
     }
   }
-  
+
   def testOnFailureWhenSpecialThrowable(num: Int, cause: Throwable): Unit = once {
     done =>
     val f = future[Unit] {
@@ -289,6 +289,9 @@ trait FutureCombinators extends TestBase {
       }
   }
 
+  /* TODO: Test for NonFatal in collect (more of a regression test at this point).
+   */
+
   def testForeachSuccess(): Unit = once {
     done =>
       val p = promise[Int]()
@@ -473,8 +476,8 @@ trait FutureCombinators extends TestBase {
   def testFallbackToFailure(): Unit = once {
     done =>
     val cause = new Exception
-    val f = future { throw cause }
-    val g = future { sys.error("failed") }
+    val f = future { /*throw cause*/ sys.error("failed") }
+    val g = future { /*sys.error("failed")*/ throw cause }
     val h = f fallbackTo g
 
     h onSuccess {