playframework/playframework — pull request #1750 by richdougherty (2013-10-17 03:19:39+00:00)
Title: Promise.timeout doesn't handle exceptions correctly

In Play 2.2, the `Promise.timeout` method takes in a `message` parameter by name:

```scala
def timeout[A](message: => A, duration: Long, unit: TimeUnit = TimeUnit.MILLISECONDS)(implicit ec: ExecutionContext): Future[A] = {
val p = SPromise[A]()
import play.api.Play.current
Akka.system.scheduler.scheduleOnce(FiniteDuration(duration, unit)) {
p.success(message)
}
p.future
}
```

The `message` parameter is resolved inside of the `scheduleOnce` call, which means it happens on another thread. If resolving that parameter throws an exception, there is no way for the client to catch the exception and the `Future` returned by the `timeout` method will never redeem.

This is a regression from Play 2.0.x, where the [`timeout` method](https://github.com/playframework/playframework/blob/2.0.3/framework/src/play/src/main/scala/play/api/libs/concurrent/Promise.scala#L235) was using `play.api.libs.concurrent.Promise` and its `redeem` method:

```scala
def redeem(body: => A): Unit = {
val result = scala.util.control.Exception.allCatch[A].either(body)
atomic { implicit txn =>
if (redeemed().isDefined) sys.error("already redeemed")
redeemed() = result.fold(Thrown(_), Redeemed(_))
}
actions.single.swap(List()).foreach(invoke(this, _))
}
```

Note that the `body` parameter is passed by name, all exceptions are caught when resolving it, and the Promise will set itself to failed status if any exception was thrown.

In Play 2.2.x, Play is using `scala.concurrent.Promise` and its `success` method:

```scala
def success(value: T): this.type = complete(Success(value))
```

Here, the `value` parameter is based by value, so if resolving the `message` used in the `timeout` method threw an Exception, it would leave the client code in a broken state.