CODEWITHSUNDEEP
scala
Rory
Rory

Reputation: 838

Scala map Future[IOResult] to Future[Unit]

I have a function that must return a Future[Unit].

In my function I'm writing a file which returns a Future[IOResult]. Both the Future and the IOResult can have a failure status.

I'd like to check for success and failure of both the Future and the IOResult in my function, but just return a Failure[Unit] from this function, is this possible?

The code below is reporting errors:

discarded non-Unit value
[error]           Future.successful(Unit)

discarded non-Unit value
[error]           Future.failed(e)

Here is my function:

def create(filePath: String, fileStream: Source[ByteString, Any]): Future[Unit] = {

    val writeResultFuture: Future[IOResult] = fileStream.runWith(FileIO.toPath(filePath))

    writeResultFuture map { writeResult =>
      writeResult.status match {
        case Success(_) =>
          logger.info(s"Successfully uploaded: ${fileInfo.fileName} to: $filePath")
          Future.successful(Unit)
        case Failure(e) =>
          logger.error(s"Failed to upload: ${fileInfo.fileName} to: $filePath", e)
          Future.failed(e)
      }
    } recover {
      case e =>
        logger.error(s"Failed to upload: ${fileInfo.fileName} to: $filePath", e)
        Future.failed(e)
    }
  }

Upvotes: 1

Views: 1905

Answers (3)

Dima
Dima

Reputation: 40510

Three things.

  1. Unit is not a value of type Unit (it's a reference to an object scala.Unit defined in scala library). You want () to denote a value of Unit.

  2. The function you give to recover is supposed to return the actual result, not a Future (recoverWith works with futures).

  3. The function you give to map returns the actual result too. You'd want flatMap if you actually needed to return a Future

So, something like this should work:

writeResultFuture.map { writeResult =>
    writeResult.status match {
        case Success(_) =>
          logger.info(s"Successfully uploaded: ${fileInfo.fileName} to: $filePath")
        case Failure(e) =>
          logger.error(s"Failed to upload: ${fileInfo.fileName} to: $filePath", e)
          throw e
    }
  }.recover { case e =>
        logger.error(s"Failed to upload: ${fileInfo.fileName} to: $filePath", e)
        throw e
  }

Better yet, use onFailure instead of recover - that way you don't need to rethrow. Just do .onFailure { e => logger.error(...) } Also, note that you are logging errors twice this way (once inside map, and then again in recover/onFailure) ... Consider removing the recover part alltogether.

  writeResultFuture.map(_.status).map { 
    case Success(_) => logger.info(s"Successfully uploaded: ${fileInfo.fileName} to: $filePath")
    case Failure(e) => logger.error(s"Failed to upload: ${fileInfo.fileName} to: $filePath", e)
        throw e
  }

Upvotes: 4

Viktor Klang
Viktor Klang

Reputation: 26597

You are returning Future's so your end result will be of type Future[Future[…]], in your case we do not need to nest at all, so we can change the code to:

DISCLAIMER: I only typed the code in here, it has not been typechecked, YMMV. No warranties. etc etc

fileStream.runWith(FileIO.toPath(filePath)) transform {
  case Failure(e) => Failure(e)
  case Success(i: IOResult) => i.status
} andThen {
    case Success(_) =>
      logger.info(s"Successfully uploaded: ${fileInfo.fileName} to: $filePath")
    case Failure(e) =>
      logger.error(s"Failed to upload: ${fileInfo.fileName} to: $filePath", e)
  }
} map { _ => () }

Upvotes: 0

Tim
Tim

Reputation: 27421

This might work:

  writeResultFuture flatMap { writeResult =>
    writeResult.status match {
      case Success(_) =>
        logger.info(s"Successfully uploaded: ${fileInfo.fileName} to: $filePath")
        Future.successful()
      case Failure(e) =>
        Future.failed(e)
    }
  } recover {
    case e =>
      logger.error(s"Failed to upload: ${fileInfo.fileName} to: $filePath", e)
  }

Using flatMap will flatten the nested Future returned by your match statement, and recover now returns Unit so that result is Future[Unit]. recover will catch all errors that are generated so you don't need to print anything in the inner Failure case.

Upvotes: 0

Related Questions