Scala: compose results of futures with exception handling

Question

I am new to Future in Scala and I haven't found the solution to my problem yet. I am trying to achieve the following (overall description: trying to get the list of guests for a list of hotels, querying each hotel separately):

Make n calls to another API, with a time out for each call
Combine all the results (converting a list of lists into a list that contains all the elements)
If an individual call fails, log the error and return an empty list (essentially in this case it's better if I get partial results rather than no results at all)
Ideally, if an individual call fails, retry x times after waiting for a certain period of time and eventually fail and handle the error as if there was no retry

Here is my code. HotelReservation represents the external API I would call.

import com.typesafe.scalalogging.slf4j.Logging

import scala.concurrent._, ExecutionContext.Implicits.global
import scala.util.{Failure, Success}

case class Guest(name: String, country: String)

trait HotelReservation extends Logging {

  def getGuests(id: Int): Future[List[Guest]] = Future {
    logger.debug(s"getting guests for id $id")
    id match {
      case 1 => List(new Guest("John", "canada"))
      case 2 => List(new Guest("Harry", "uk"), new Guest("Peter", "canada"))
      case 3 => {
        Thread.sleep(4000)
        List(new Guest("Harry", "austriala"))
      }
      case _ => throw new IllegalArgumentException("unknown hotel id")
    }
  }
}

object HotelReservationImpl extends HotelReservation

HotelSystem makes the calls.

import com.typesafe.scalalogging.slf4j.Logging

import scala.util.control.NonFatal
import scala.util.{Failure, Success}
import scala.concurrent._, duration._, ExecutionContext.Implicits.global

class HotelSystem(hres: HotelReservation) {

  def pollGuests(hotelIds: List[Int]): Future[List[Guest]] = {

    Future.sequence(
  hotelIds.map { id => future {
    try {
      Await.result(hres.getGuests(id), 3 seconds)
    } catch {
      case _: Exception =>
        Console.println(s"failed for id $id")
        List.empty[Guest]
    }

  }
  }
).map(_.fold(List())(_ ++ _)) /*recover { case NonFatal(e) =>
  Console.println(s"failed:", e)
  List.empty[Guest]
}*/
  }
}

And the test.

object HotelSystemTest extends App {

  Console.println("*** hotel test start ***")

  val hres = HotelReservationImpl

  val hotel = new HotelSystem(hres)
  val result = hotel.pollGuests(List(1, 2, 3, 6))

  result onSuccess {
    case r => Console.println(s"success: $r")
  }

  val timeout = 5000
  Console.println(s"waiting for $timeout ms")
  Thread.sleep(timeout)
  Console.println("*** test end ***")
}

1 and 2 are working. So is 3 but I think I read somewhere on SO that having a try catch around the call to the future is not a good idea and it's better to use recover. However, in this case, if I use recover, if there is an individual failure, the whole call fails and return an empty list. Any ideas about how to improve this?

Matias Saarinen · Accepted Answer · 2015-03-30T16:45:01.850

5

There is actually two things that you could have done differently: leave the try-catch out and don't use Await with Futures.

Here is a better way to implement the pollGuests:

Future.sequence(
   hotelIds.map { hotelId =>
      hres.getGuests(hotelId).recover {
         case e: Exception => List.empty[Guest]
      }
   }
).map(_.flatten)

The first point here is that you don't have to use Futures inside the pollGuests() since the getGuests() already gives you a Future. You just create a new Future with the recover() so that the possible failure is already handled when you return the Future

The second point is that you shouldn't use Await. It makes your code block until the Future is ready which could for example freeze your whole UI thread. I assume that you were using the Await to be able to use try-catch but thanks to the recover() you don't need that anymore.

edited Mar 30 '15 at 16:45

answered Mar 29 '15 at 18:18

Matias Saarinen

606
4
10

I think I understand what you changed but it's giving me several compile errors (type mismatch). – Emmanuel Ballerini Mar 30 '15 at 12:43
Oh, you're right. Now it's fixed. It was true that you had to create Future of a List of Lists before you could flatten it. – Matias Saarinen Mar 30 '15 at 16:46
Yeah. It works now. Something I forgot to mention and which was part of the Await is the timeout. How would you handle a timeout in this case? – Emmanuel Ballerini Mar 30 '15 at 17:52
The timeout is not actually a simple thing to do. You can use Await but that is not really a good solution since the blocking. [This answer](http://stackoverflow.com/questions/16304471/scala-futures-built-in-timeout) provides you a better solution which however is quite complicated. I would say that how you should implement your timeout depends on the use case. I would start with: do you really need a timeout. – Matias Saarinen Mar 30 '15 at 20:47

Scala: compose results of futures with exception handling

1 Answers1