Skip to Content
Course content

231: Code Review Checklist for Production Scala Services

Click on the "Edit" button in the top corner of the screen to edit your slide content.

A few years ago, I was reviewing a PR for a high-throughput payment gateway. The developer had implemented a new retry mechanism using Future.sequence on a list of API calls. It looked elegant—very "functional"—and passed all the unit tests. But about an hour after it hit production, the service started flatlining. We weren't seeing CPU spikes or memory leaks; we were seeing thread pool exhaustion. Because they had launched thousands of Futures into the default global ExecutionContext without any throttling, the service spent more time context-switching than actually processing payments. The health check failed, the load balancer pulled the node, and we had a cascading failure across the cluster.

That's the thing about Scala: it gives you incredibly powerful tools to express complex logic concisely, but those same tools can hide production landmines if you aren't looking for them during code review. When you're reviewing Scala code for a production service, you can't just check if the logic is "correct." You have to check if the code is "stable" under load.

Taming the Concurrency Chaos

The first thing I look for in any PR is how Futures are being handled. If I see Await.result or Await.ready anywhere in the business logic, it's an immediate request for change. Blocking a thread in a non-blocking system is a cardinal sin; it’s the fastest way to kill your throughput. If the developer truly needs to block, they should be using a dedicated, separate ExecutionContext specifically tuned for blocking I/O, and they should wrap the call in a blocking { ... } block to hint to the ForkJoinPool that it needs to spawn a replacement thread.

I also keep an eye out for "Future leaks." This happens when a Future is created but its result is never handled, or worse, when a recursive function creates Futures without a proper termination condition or limiting mechanism. Always ask: "What happens if this external service takes 30 seconds to respond instead of 30 milliseconds?" If the answer is "we spawn 10,000 more Futures," the code isn't production-ready.

Eliminating the 'Throw' Mentality

In a production Scala service, throw new Exception() is a smell. Exceptions are for truly exceptional, unrecoverable failures—like the disk being full or the JVM running out of memory. For business-level errors (e.g., UserNotFound or InsufficientFunds), I expect to see Either[Error, Success] or a custom ADT.

When reviewing, look for try-catch blocks. If you see a giant try-catch wrapping a whole method, the developer is treating Scala like Java. Encourage them to wrap the risky call in a Try(...) and then map that into an Either. This forces the next developer in the chain to actually handle the error case rather than hoping the exception is caught somewhere up the stack. I've seen far too many production outages caused by an uncaught exception killing a critical background actor or stream processor.

Spotting Performance Leaks in the Type System

Finally, we need to talk about the "hidden" costs. Scala's collections are beautiful, but using the wrong one in a hot loop can be disastrous. If I see a List being accessed by index (list(i)) inside a loop, that's an O(n) operation that turns a linear process into a quadratic one. In those cases, I'll suggest a Vector or an Array.

I also scrutinize implicit conversions and lazy val usage in singletons. A lazy val is thread-safe by default, but it achieves this via a synchronized block under the hood. If you have a high-contention piece of code hitting a lazy val in a shared object, you might be introducing a bottleneck you didn't even know existed. I usually suggest replacing these with a simple val if the initialization cost is low, or a more explicit caching strategy if it's high.




📋 Practical Task

Refactoring the Leaky Payment Gateway Client

You have been assigned to review a snippet of code intended for the PaymentService. The current implementation is prone to thread exhaustion and fragile error handling. Your task is to refactor the processPayments method to meet production standards.


import scala.concurrent._
import scala.concurrent.duration._
import scala.concurrent.ExecutionContext.Implicits.global

case class Payment(id: String, amount: Double)
case class PaymentResult(id: String, success: Boolean)

object PaymentGateway {
  def callApi(p: Payment): Future[PaymentResult] = {
    // Simulates an API call
    Future { 
      Thread.sleep(100) 
      if (p.amount > 1000) throw new RuntimeException("Limit exceeded")
      PaymentResult(p.id, true)
    }
  }
}

class PaymentService {
  def processPayments(payments: List[Payment]): List[PaymentResult] = {
    // PROBLEM: This blocks the main thread and doesn't handle errors safely
    payments.map { p =>
      val f = PaymentGateway.callApi(p)
      Await.result(f, 2.seconds)
    }
  }
}

Requirements for your fix:

  • Remove Await.result. The method should now return a Future[List[Either[Throwable, PaymentResult]]] (or a similar non-blocking structure).
  • Replace the throw new RuntimeException logic in the gateway simulation (or handle it in the service) using Try or Either so the entire batch doesn't fail if one payment exceeds the limit.
  • Ensure that the transformation from a list of payments to a list of results happens asynchronously.
Rating
0 0

There are no comments for now.

to be the first to leave a comment.