Currently my error handling is not working as I want to, this is what I am trying to do:

1. UserApi.insert fails, return its error and don't continue
2. WorkApi.insert fails, return its error after calling UserApi.delete
3. WorkApi.assign fails, return its error after calling WorkApi.delete and UserApi.delete

So in summary, UserApi.insert is called, if it is successfull, continue to #2. If WorkApi.insert is successfull, continue. And so on, if the current step fails, you have to reverse the previous one.

Also it is important to return the most relevant error for the Api call that failed.

If all calls were successful, I want to return the first calls value which is a User.

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

val u1 = User("u1")
val w1 = Work("w1")

val resp = for {
  insertResp <- UserApi.insert(u1)
  workInsertResp <- WorkApi.insert(w1)
  workAssignResp <- WorkApi.assign(w1)
} yield insertResp
println("ending...")

resp onComplete {
  case Success(r) => println(r)
  case Failure(t) => println(t)
}

case class User(name: String)
case class Work(name: String)

case class MyError(name: String)

object UserApi {
  def insert(user: User): Future[Either[MyError, User]] =
    if (user.name == "u1") Future(Right(user))
    else Future(Left(MyError("UserApi.insert")))
  def delete(user: User): Future[Either[MyError, String]] =
    Future(Right("UserApi.delete"))
}

object WorkApi {
  def insert(work: Work): Future[Either[MyError, Work]] =
    if (work.name == "w1") Future(Right(work))
    else Future(Left(MyError("WorkApi.insert")))

  def delete(work: Work): Future[Either[MyError, Work]] = Future(Right(work))

  def assign(work: Work): Future[Either[MyError, Work]] =
    if (work.name == "w1") Future(Right(work))
    else Future(Left(MyError("WorkApi.assign")))
}

Currently I am not sure how to bubble the correct error up.

Note: I am using scala 2.13.x, and I am not using other frameworks just plain Scala.

https://scastie.scala-lang.org/OV4Ax58qQ1S3R3fFUikSbw

CodePudding user response：

First of all, I would recommend against mixing Future and Either this way. Future has its own way of representing failures, and wrapping an Either in a Future means you will need to handle both the failed Future case and the Left case of Either, which can lead to some confusing code.

In the code provided in the question, there's no asynchronous execution, so using Future is redundant, and you could use Either types directly. However, I assume you want to replace these methods with ones that make actual (asynchronous) API calls, in which case you'll want to use Future without Either. Future requires that failure values extend Throwable, so this would require a change to MyError:

case class MyError(name: String) extends Exception(name)

Second, it's not a good practice to use Future.apply for non-blocking construction as in Future(Right(user)) or Future(Left(MyError("UserApi.insert"))). It isn't obvious, but this actually causes Right(user) to be scheduled as a task on the implicit execution context, rather than being computed synchronously on the current thread. It's better to use Future.successful or Future.failed to create a completed Future when the result is trivial.

With these changes, the new method implementations are:

object UserApi {
  def insert(user: User): Future[User] =
    if (user.name == "u1") Future.successful(user)
    else Future.failed(MyError("UserApi.insert"))

  def delete(user: User): Future[String] =
    Future.successful("UserApi.delete")
}

object WorkApi {
  def insert(work: Work): Future[Work] =
    if (work.name == "w1") Future.successful(work)
    else Future.failed(MyError("WorkApi.insert"))

  def delete(work: Work): Future[Work] =
    Future.successful(work)

  def assign(work: Work): Future[Work] =
    if (work.name == "w1") Future.successful(work)
    else Future.failed(MyError("WorkApi.assign"))
}

CodePudding user response：

I believe this does what you've described.

import scala.concurrent.Future
import scala.concurrent.ExecutionContext.Implicits.global

val resp: Future[Either[MyError,User]] =
  UserApi.insert(u1).flatMap{_.fold(
    {err => Future.successful(Left(err))}
    ,usr => WorkApi.insert(w1).flatMap{_.fold(
      {err => UserApi.delete(u1); Future.successful(Left(err))}
      , _  => WorkApi.assign(w1).map{_.fold(
        {err => WorkApi.delete(w1); UserApi.delete(u1); Left(err)}
        , _  => Right(usr)
      )}
    )}
  )}

. . . //and the rest of your code

testing:

import scala.concurrent.duration.DurationInt

concurrent.Await.result(resp, 9999.millis)
//res0: Either[MyError,User] = Right(User(u1))

As you can see, your current code design is not well suited for the task you've laid out.