Merge pull request #1108 from gneuvill/series/7.3.x

Fix a safety hole in Task.onFinish
scalaz · Feb 26, 2016 · 7c15ea2 · 7c15ea2
2 parents 2ed0798 + c35652e
commit 7c15ea2
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 3 deletions.
diff --git a/concurrent/src/main/scala/scalaz/concurrent/Task.scala b/concurrent/src/main/scala/scalaz/concurrent/Task.scala
@@ -55,8 +55,10 @@ class Task[+A](val get: Future[Throwable \/ A]) {
    */
   def onFinish(f: Option[Throwable] => Task[Unit]): Task[A] =
     new Task(get flatMap {
-      case -\/(e) => f(Some(e)).get *> Future.now(-\/(e))
-      case r => f(None).get *> Future.now(r)
+      case -\/(e) =>
+        Task.Try(f(Some(e))).fold(e2 => Future.now(-\/(e2)), _.get *> Future.now(-\/(e)))
+      case r =>
+        Task.Try(f(None)).fold(e => Future.now(-\/(e)), _.get *> Future.now(r))
     })
 
   /**

diff --git a/tests/src/test/scala/scalaz/concurrent/TaskTest.scala b/tests/src/test/scala/scalaz/concurrent/TaskTest.scala
@@ -125,8 +125,13 @@ object TaskTest extends SpecLite {
     Task { Thread.sleep(10); throw FailWhale; 42 }.handleWith { case FailWhale => Task.delay(throw SadTrombone) }.unsafePerformSyncAttempt ==
       -\/(SadTrombone)
   }
+
+  "catches exceptions thrown by onFinish argument function" ! {
+    Task { Thread.sleep(10); 42 }.onFinish { _ => throw SadTrombone; Task.now(()) }.unsafePerformSyncAttemptFor(1000) ==
+      -\/(SadTrombone)
+  }  
 
-  "evalutes Monad[Task].point lazily" in {
+  "evaluates Monad[Task].point lazily" in {
     val M = implicitly[Monad[Task]]
     var x = 0
     M point { x += 1 }