0

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)
  }
Mutaz
  • 547
  • 4
  • 12

2 Answers2

1

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)
  }
Mutaz
  • 547
  • 4
  • 12
Peter Neyens
  • 9,770
  • 27
  • 33
  • Thanks, compiler feels better now. But I'm not able to catch the exception. Say `val resultFuture = get(BSONDocument("_id" -> BSONObjectID(id))) .recover { case _ => None }` neither resultFuture.onFailure nor resultFuture.onSuccess is reached/matched. How can I match the recover `case _ => None` pattern? – Mutaz Nov 11 '15 at 18:32
  • Does `collection.find` throw an `IllegalArgumentException` or return a failed `Future` ? – Peter Neyens Nov 11 '15 at 18:34
  • Still throws `IllegalArgumentException` – Mutaz Nov 11 '15 at 18:35
  • I have updated my answer to let `get` return a failed `Future` instead of throwing an `IllegalArgumentException`. – Peter Neyens Nov 11 '15 at 18:41
  • Used your code but the exception is still thrown unhandled! What do you think? – Mutaz Nov 11 '15 at 18:51
  • Updated your answer with what worked! Very strange! Why that worked and yours didn't? I prefer to have it at the `get` level rather than `getById` level. Any idea? – Mutaz Nov 11 '15 at 19:11
  • I think the `IllegalArgumentException` is thrown when you create a `BSONDocument` with an invalid ID. – Peter Neyens Nov 11 '15 at 19:18
  • Oh! How I missed this :D . You are right. I simulated a mongoDB ID that doesn't exist by using a valid one and deleting one character. Seems it expects an ID with a certain length. Many thanks :) – Mutaz Nov 11 '15 at 19:24
1

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

MrRontgen
  • 132
  • 1
  • 6