CODEWITHSUNDEEP
scalaconcurrency
Emmanuel Ballerini
Emmanuel Ballerini

Reputation: 684

Scala: compose results of futures with exception handling

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):

  1. Make n calls to another API, with a time out for each call
  2. Combine all the results (converting a list of lists into a list that contains all the elements)
  3. 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)
  4. 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?

Upvotes: 2

Views: 701

Answers (1)

Matias Saarinen
Matias Saarinen

Reputation: 606

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.

Upvotes: 5

Related Questions