Mutaz
Mutaz

Reputation: 547

Need help handling IllegalArgumentException in reactivemongo

The below code tries to get document by ID using Reactivemongo. However, I don't know how to deal with the IllegalArgumentException thrown when the ID is wrong! Tried the below code but the compiler is not happy with case _ => Future.successful(None), it says: found scala.concurrent.Future[None.type] required Option[SomeModel] . Also tried case _ => None with no success.

def getById(id: String)(implicit ec: ExecutionContext): Future[Option[SomeModel]]={
    this.get( BSONDocument("_id" ->  BSONObjectID(id)) ).map {
      res => Future.successful(res)
    }.recover {
      case _ => Future.successful(None)
    }
  }

def get(query: BSONDocument)(implicit ec: ExecutionContext): Future[Option[SomeModel]]= {
    collection.find(query).one[SomeModel](ReadPreference.Primary)
  }

Upvotes: 0

Views: 339

Answers (2)

MrRontgen
MrRontgen

Reputation: 132

As I understood your question,

...I don't know how to deal with the IllegalArgumentException thrown when the ID is wrong!

I think, the better solution is

def getById(id: String)(implicit ec: ExecutionContext): Future[Option[SomeModel]]={

    //Try to parse bson id from string. This method return Try[BSONObjectId] and we can simple `match` them
    BSONObjectId.parse(id) match {

       // valid bson id 
      case Success(bsonId) => this.get( BSONDocument("_id" -> bsonId) )

      //We catch IllegalArgumentException and just return None
      case Failure(ex) => Future[Option[SomeModel]](None)
    }
}

In your code, Scala try to parse BSONObjectId from string before call get method, and if string id is invalid BSON throwing exception in current thread (not in the Future result of method get). That's why recover {case _ => Future.successful(None)} will not execute. Methods recover and recoverWith executes only if Future store some exception. For example, this code will work too:

def getById(id: String)(implicit ec: ExecutionContext): Future[Option[SomeModel]]={

    //create Future, that will be store exception (if id is invalid) or valid BSON id.
    //method flatMap because this.get return Future type.
    Future(BSONObjectId(id)).flatMap{ bsonId =>  

        //this executes only if string id is valid bson.
        this.get( BSONDocument("_id" ->  bsonId) )
    }.recover{

        //this will be execute only if string id is invalid bson. 
        // the best practice to catch non-fatal Throwables via class scala.util.control.NonFatal 
        case NonFatal(e) =>  None
    }
}

But this variant is complicated (create one more Future, flatMap them, recover with NonFatal control). I would prefer the first variant with parse method (it's mush more easy without some extra futures and controls).

Upvotes: 1

Peter Neyens
Peter Neyens

Reputation: 9820

You are confusing recover and recoverWith.

Both functions expect a PartialFunction which accept a Throwable and both functions return a Future[U], but

  • recover's PartialFunction should return a U
  • while recoverWith's should return a Future[U].

In your case, you can use recover :

get(BSONDocument("_id" ->  BSONObjectID(id)))
  .recover { case _ => None }
  // you don't need map(res => Future.successful(res)

Update: You could edit get to return a failed Future instead of throwing an IllegalArgumentException. A possible way would be to use Try and its recover :

import scala.util.Try

def get(query: BSONDocument)(implicit ec: ExecutionContext): Future[Option[SomeModel]] = 
  Try(collection.find(query).one[SomeModel](ReadPreference.Primary))
    .recover{ case t => Future.failed(t) }.get

Update:

It worked when I did

def getById(id: String)(implicit ec: ExecutionContext): Future[Option[SomeModel]]={
      Try(this.get(BSONDocument("_id" -> BSONObjectID(id)))).recover{ case t => Future.failed(t) }.get
  }

def get(query: BSONDocument)(implicit ec: ExecutionContext): Future[Option[SomeModel]]={
      collection.find(query).one[SomeModel](ReadPreference.Primary)
  }

Upvotes: 1

Related Questions