[2.x] feat: Cache failed compilation to avoid repeated failures (#8490)

When a compilation fails with CompileFailed, the failure is now cached so that subsequent builds with the same inputs don't re-run the failed compilation. This significantly improves the experience when using BSP clients like Metals that may trigger many compilations in a row. The implementation: - Adds CachedCompileFailure, CachedProblem, and CachedPosition types to serialize compilation failures - Modifies ActionCache.cache to catch CompileFailed exceptions and store them in the cache with exitCode=1 - On cache lookup, checks for cached failures first and re-throws the cached exception if found - Fixes DiskActionCacheStore.put to preserve exitCode from request - Adds unit test to verify cached failure behavior Fixes #7662
2026-01-12 16:03:05 -05:00 · 2026-01-12 16:03:05 -05:00 · fe6125d8d1
parent bc32efbea6
commit fe6125d8d1
6 changed files with 339 additions and 49 deletions
--- a/util-cache/src/main/contraband-scala/sbt/util/GenericFailure.scala
+++ b/util-cache/src/main/contraband-scala/sbt/util/GenericFailure.scala
@ -0,0 +1,55 @@
+/**
+ * This code is generated using [[https://www.scala-sbt.org/contraband]].
+ */
+
+// DO NOT EDIT MANUALLY
+package sbt.util
+/**
+ * A GenericFailure represents a cached task failure.
+ * This allows caching failures so that repeated builds don't re-run failed tasks.
+ * The kind field indicates the type of failure (e.g., "CompileFailed").
+ * 
+ * Fixes https://github.com/sbt/sbt/issues/7662
+ */
+final class GenericFailure private (
+  val kind: Option[String],
+  val message: Option[String],
+  val problems: Vector[xsbti.Problem]) extends Serializable {
+  
+  private def this() = this(None, None, Vector())
+  
+  override def equals(o: Any): Boolean = this.eq(o.asInstanceOf[AnyRef]) || (o match {
+    case x: GenericFailure => (this.kind == x.kind) && (this.message == x.message) && (this.problems == x.problems)
+    case _ => false
+  })
+  override def hashCode: Int = {
+    37 * (37 * (37 * (37 * (17 + "sbt.util.GenericFailure".##) + kind.##) + message.##) + problems.##)
+  }
+  override def toString: String = {
+    "GenericFailure(" + kind + ", " + message + ", " + problems + ")"
+  }
+  private def copy(kind: Option[String] = kind, message: Option[String] = message, problems: Vector[xsbti.Problem] = problems): GenericFailure = {
+    new GenericFailure(kind, message, problems)
+  }
+  def withKind(kind: Option[String]): GenericFailure = {
+    copy(kind = kind)
+  }
+  def withKind(kind: String): GenericFailure = {
+    copy(kind = Option(kind))
+  }
+  def withMessage(message: Option[String]): GenericFailure = {
+    copy(message = message)
+  }
+  def withMessage(message: String): GenericFailure = {
+    copy(message = Option(message))
+  }
+  def withProblems(problems: Vector[xsbti.Problem]): GenericFailure = {
+    copy(problems = problems)
+  }
+}
+object GenericFailure {
+  
+  def apply(): GenericFailure = new GenericFailure()
+  def apply(kind: Option[String], message: Option[String], problems: Vector[xsbti.Problem]): GenericFailure = new GenericFailure(kind, message, problems)
+  def apply(kind: String, message: String, problems: Vector[xsbti.Problem]): GenericFailure = new GenericFailure(Option(kind), Option(message), problems)
+}
--- a/util-cache/src/main/contraband/actionresult.contra
+++ b/util-cache/src/main/contraband/actionresult.contra
@ -15,3 +15,14 @@ type ActionResult {
  contents: [java.nio.ByteBuffer] @since("0.4.0")
  isExecutable: [Boolean] @since("0.5.0")
 }
+
+## A GenericFailure represents a cached task failure.
+## This allows caching failures so that repeated builds don't re-run failed tasks.
+## The kind field indicates the type of failure (e.g., "CompileFailed").
+##
+## Fixes https://github.com/sbt/sbt/issues/7662
+type GenericFailure @generateCodec(false) {
+  kind: String @since("0.1.0")
+  message: String @since("0.1.0")
+  problems: [xsbti.Problem] @since("0.1.0")
+}
--- a/util-cache/src/main/scala/sbt/util/ActionCache.scala
+++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala
@ -24,11 +24,13 @@ import sjsonnew.{ HashWriter, JsonFormat }
 import sjsonnew.support.murmurhash.Hasher
 import sjsonnew.support.scalajson.unsafe.{ CompactPrinter, Converter, Parser }
 import scala.quoted.{ Expr, FromExpr, ToExpr, Quotes }
-import xsbti.{ FileConverter, HashedVirtualFileRef, VirtualFile, VirtualFileRef }
+import xsbti.{ CompileFailed, FileConverter, HashedVirtualFileRef, VirtualFile, VirtualFileRef }

 object ActionCache:
  private[sbt] val dirZipExt = ".sbtdir.zip"
  private[sbt] val manifestFileName = "sbtdir_manifest.json"
+  private[sbt] val failureFileName = "failure.json"
+  private[sbt] val failureExitCode = 1

  /**
   * This is a key function that drives remote caching.
@ -54,11 +56,28 @@ object ActionCache:
      action: I => InternalActionResult[O],
  ): O =
    import config.*
+
+    def cacheFailure(e: CompileFailed): Nothing =
+      // Cache the failure so subsequent builds don't re-run failed compilation
+      // This fixes https://github.com/sbt/sbt/issues/7662
+      // Use the same input digest as success, distinguished by exitCode
+      cacheEventLog.append(ActionCacheEvent.OnsiteTask)
+      val (input, valuePath) = mkInput(key, codeContentHash, extraHash)
+      val cachedFailure = CachedCompileFailure.fromException(e)
+      val json = Converter.toJsonUnsafe(cachedFailure)
+      val failureFile = StringVirtualFile1(valuePath, CompactPrinter(json))
+      store.put(
+        UpdateActionResultRequest(input, Vector(failureFile), exitCode = failureExitCode)
+      )
+      throw e
+
    def organicTask: O =
      // run action(...) and combine the newResult with outputs
      val InternalActionResult(result, outputs) =
        try action(key): @unchecked
        catch
+          case e: CompileFailed =>
+            cacheFailure(e)
          case e: Exception =>
            cacheEventLog.append(ActionCacheEvent.Error)
            throw e
@ -83,11 +102,82 @@ object ActionCache:
            result
          case Left(e) => throw e

-    get(key, codeContentHash, extraHash, tags, config) match
-      case Some(value) => value
-      case None        => organicTask
+    // Single cache lookup - use exitCode to distinguish success from failure
+    getWithFailure(key, codeContentHash, extraHash, tags, config) match
+      case Right(value) => value
+      case Left(Some(failure)) =>
+        config.cacheEventLog.append(ActionCacheEvent.Found("cached-failure"))
+        // Replay problems to the logger so users see the cached errors/warnings
+        failure.replay(config.logger)
+        throw failure.toException
+      case Left(None) => organicTask
  end cache

+  /**
+   * Retrieves the cached value or failure with a single cache lookup.
+   * Returns Right(value) for cached success, Left(Some(failure)) for cached failure,
+   * or Left(None) for cache miss.
+   */
+  private def getWithFailure[I: HashWriter, O: JsonFormat](
+      key: I,
+      codeContentHash: Digest,
+      extraHash: Digest,
+      tags: List[CacheLevelTag],
+      config: BuildWideCacheConfiguration,
+  ): Either[Option[CachedCompileFailure], O] =
+    import config.store
+    def valueFromStr(str: String, origin: Option[String]): O =
+      config.cacheEventLog.append(ActionCacheEvent.Found(origin.getOrElse("unknown")))
+      val json = Parser.parseUnsafe(str)
+      Converter.fromJsonUnsafe[O](json)
+
+    def failureFromStr(str: String): CachedCompileFailure =
+      val json = Parser.parseUnsafe(str)
+      Converter.fromJsonUnsafe[CachedCompileFailure](json)
+
+    // Optimization: Check if we can read directly from symlinked value file
+    val (input, valuePath) = mkInput(key, codeContentHash, extraHash)
+    val resolvedValuePath = config.fileConverter.toPath(VirtualFileRef.of(valuePath))
+
+    def readFromSymlink(): Option[Either[Option[CachedCompileFailure], O]] =
+      if java.nio.file.Files.isSymbolicLink(resolvedValuePath) && java.nio.file.Files
+          .exists(resolvedValuePath)
+      then
+        Exception.nonFatalCatch
+          .opt(IO.read(resolvedValuePath.toFile(), StandardCharsets.UTF_8))
+          .flatMap: str =>
+            // We still need to sync output files for side effects and check exitCode
+            findActionResult(key, codeContentHash, extraHash, config) match
+              case Right(result) =>
+                store.syncBlobs(result.outputFiles, config.outputDirectory)
+                if result.exitCode.contains(failureExitCode) then
+                  Some(Left(Some(failureFromStr(str))))
+                else Some(Right(valueFromStr(str, Some("symlink"))))
+              case Left(_) => None
+      else None
+
+    readFromSymlink() match
+      case Some(result) => result
+      case None =>
+        findActionResult(key, codeContentHash, extraHash, config) match
+          case Right(result) =>
+            // Check exitCode to determine if this is a cached failure
+            val isFailure = result.exitCode.contains(failureExitCode)
+            result.contents.headOption match
+              case Some(head) =>
+                store.syncBlobs(result.outputFiles, config.outputDirectory)
+                val str = String(head.array(), StandardCharsets.UTF_8)
+                if isFailure then Left(Some(failureFromStr(str)))
+                else Right(valueFromStr(str, result.origin))
+              case _ =>
+                val paths = store.syncBlobs(result.outputFiles, config.outputDirectory)
+                if paths.isEmpty then Left(None)
+                else
+                  val str = IO.read(paths.head.toFile())
+                  if isFailure then Left(Some(failureFromStr(str)))
+                  else Right(valueFromStr(str, result.origin))
+          case Left(_) => Left(None)
+
  /**
   * Retrieves the cached value.
   */
@ -98,47 +188,9 @@ object ActionCache:
      tags: List[CacheLevelTag],
      config: BuildWideCacheConfiguration,
  ): Option[O] =
-    import config.store
-    def valueFromStr(str: String, origin: Option[String]): O =
-      config.cacheEventLog.append(ActionCacheEvent.Found(origin.getOrElse("unknown")))
-      val json = Parser.parseUnsafe(str)
-      Converter.fromJsonUnsafe[O](json)
-
-    // Optimization: Check if we can read directly from symlinked value file
-    val (input, valuePath) = mkInput(key, codeContentHash, extraHash)
-    val resolvedValuePath = config.fileConverter.toPath(VirtualFileRef.of(valuePath))
-
-    def readFromSymlink(): Option[O] =
-      if java.nio.file.Files.isSymbolicLink(resolvedValuePath) && java.nio.file.Files
-          .exists(resolvedValuePath)
-      then
-        Exception.nonFatalCatch
-          .opt(IO.read(resolvedValuePath.toFile(), StandardCharsets.UTF_8))
-          .map: str =>
-            // We still need to sync output files for side effects
-            findActionResult(key, codeContentHash, extraHash, config) match
-              case Right(result) =>
-                store.syncBlobs(result.outputFiles, config.outputDirectory)
-              case Left(_) => // Ignore if we can't find ActionResult
-            valueFromStr(str, Some("symlink"))
-      else None
-
-    readFromSymlink() match
-      case Some(value) => Some(value)
-      case None =>
-        findActionResult(key, codeContentHash, extraHash, config) match
-          case Right(result) =>
-            // some protocol can embed values into the result
-            result.contents.headOption match
-              case Some(head) =>
-                store.syncBlobs(result.outputFiles, config.outputDirectory)
-                val str = String(head.array(), StandardCharsets.UTF_8)
-                Some(valueFromStr(str, result.origin))
-              case _ =>
-                val paths = store.syncBlobs(result.outputFiles, config.outputDirectory)
-                if paths.isEmpty then None
-                else Some(valueFromStr(IO.read(paths.head.toFile()), result.origin))
-          case Left(_) => None
+    getWithFailure(key, codeContentHash, extraHash, tags, config) match
+      case Right(value) => Some(value)
+      case Left(_)      => None

  /**
   * Checks if the ActionResult exists in the cache.
--- a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala
+++ b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala
@ -217,7 +217,7 @@ class DiskActionCacheStore(base: Path, converter: FileConverter) extends Abstrac
    try
      val acFile = acBase.toFile / request.actionDigest.toString.replace("/", "-")
      val refs = putBlobsIfNeeded(request.outputFiles).toVector
-      val v = ActionResult(refs, storeName)
+      val v = ActionResult(refs, Some(storeName), request.exitCode)
      val json = Converter.toJsonUnsafe(v)
      IO.write(acFile, CompactPrinter(json))
      Right(v)
--- a/util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala
+++ b/util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala
@ -0,0 +1,109 @@
+/*
+ * sbt
+ * Copyright 2023, Scala center
+ * Copyright 2011 - 2022, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt.util
+
+import sjsonnew.{ Builder, JsonFormat, Unbuilder, deserializationError }
+import sbt.internal.util.codec.{ ProblemFormats, SeverityFormats, PositionFormats }
+import xsbti.{ CompileFailed, Problem, Severity }
+
+/**
+ * A wrapper around GenericFailure for CompileFailed exceptions.
+ * This allows caching compilation failures so that repeated builds
+ * don't re-run failed compilations unnecessarily.
+ *
+ * Fixes https://github.com/sbt/sbt/issues/7662
+ */
+final case class CachedCompileFailure(underlying: GenericFailure):
+  def toException: CompileFailed = new CompileFailed:
+    override def arguments(): Array[String] = Array.empty
+    override def problems(): Array[Problem] = underlying.problems.toArray
+    override def getMessage(): String = underlying.message.getOrElse("")
+
+  /** Replay problems to the logger so users see the cached errors/warnings. */
+  def replay(logger: Logger): Unit =
+    underlying.problems.foreach: problem =>
+      val msg = CachedCompileFailure.formatProblem(problem)
+      problem.severity match
+        case Severity.Error => logger.error(msg)
+        case Severity.Warn  => logger.warn(msg)
+        case Severity.Info  => logger.info(msg)
+end CachedCompileFailure
+
+object CachedCompileFailure
+    extends ProblemFormats
+    with SeverityFormats
+    with PositionFormats
+    with sjsonnew.BasicJsonProtocol:
+
+  private val CompileFailedKind = "CompileFailed"
+
+  /**
+   * Format a problem for display. Uses the `rendered` field if available (Scala 3),
+   * otherwise constructs a message from position and message (Scala 2.13).
+   */
+  private[util] def formatProblem(problem: Problem): String =
+    import sbt.util.InterfaceUtil.toOption
+    toOption(problem.rendered).getOrElse:
+      val pos = problem.position
+      val file = toOption(pos.sourcePath).getOrElse("unknown")
+      val line = toOption(pos.line).map(l => s":$l").getOrElse("")
+      val pointer = toOption(pos.pointer).map(p => s":$p").getOrElse("")
+      val lineContent = Option(pos.lineContent).filter(_.nonEmpty).map(c => s"\n$c").getOrElse("")
+      val pointerLine = toOption(pos.pointerSpace).map(s => s"\n$s^").getOrElse("")
+      s"$file$line$pointer: ${problem.message}$lineContent$pointerLine"
+
+  /**
+   * Check if the problems contain enough information to be useful when replayed.
+   * For Scala 2.13, the `rendered` field is empty, so we check if position info exists.
+   */
+  def hasSufficientInfo(e: CompileFailed): Boolean =
+    import sbt.util.InterfaceUtil.toOption
+    e.problems()
+      .forall: problem =>
+        // Either has rendered text (Scala 3) or has position info (Scala 2.13)
+        toOption(problem.rendered).isDefined ||
+          (toOption(problem.position.sourcePath).isDefined && problem.message.nonEmpty)
+
+  def fromException(e: CompileFailed): CachedCompileFailure =
+    CachedCompileFailure(
+      GenericFailure(
+        kind = CompileFailedKind,
+        message = Option(e.getMessage).getOrElse(""),
+        problems = e.problems().toVector
+      )
+    )
+
+  // Custom JsonFormat for GenericFailure since we disabled automatic codec generation
+  given JsonFormat[GenericFailure] = new JsonFormat[GenericFailure]:
+    override def read[J](jsOpt: Option[J], unbuilder: Unbuilder[J]): GenericFailure =
+      jsOpt match
+        case Some(js) =>
+          unbuilder.beginObject(js)
+          val kind = unbuilder.readField[Option[String]]("kind")
+          val message = unbuilder.readField[Option[String]]("message")
+          val problems = unbuilder.readField[Vector[Problem]]("problems")
+          unbuilder.endObject()
+          GenericFailure(kind, message, problems)
+        case None =>
+          deserializationError("Expected JsObject but found None")
+
+    override def write[J](obj: GenericFailure, builder: Builder[J]): Unit =
+      builder.beginObject()
+      builder.addField("kind", obj.kind)
+      builder.addField("message", obj.message)
+      builder.addField("problems", obj.problems)
+      builder.endObject()
+
+  given JsonFormat[CachedCompileFailure] = new JsonFormat[CachedCompileFailure]:
+    private val gf = summon[JsonFormat[GenericFailure]]
+    override def read[J](jsOpt: Option[J], unbuilder: Unbuilder[J]): CachedCompileFailure =
+      CachedCompileFailure(gf.read(jsOpt, unbuilder))
+    override def write[J](obj: CachedCompileFailure, builder: Builder[J]): Unit =
+      gf.write(obj.underlying, builder)
+end CachedCompileFailure
--- a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala
+++ b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala
@ -5,12 +5,19 @@ import sbt.internal.util.StringVirtualFile1
 import sbt.io.IO
 import sbt.io.syntax.*
 import verify.BasicTestSuite
-import xsbti.VirtualFile
-import xsbti.FileConverter
-import xsbti.VirtualFileRef
+import xsbti.{
+  CompileFailed,
+  Problem,
+  Position,
+  Severity,
+  VirtualFile,
+  FileConverter,
+  VirtualFileRef
+}
 import java.nio.file.Path
 import java.nio.file.Paths
 import java.nio.file.Files
+import java.util.Optional
 import ActionCache.InternalActionResult

 object ActionCacheTest extends BasicTestSuite:
@ -83,6 +90,9 @@ object ActionCacheTest extends BasicTestSuite:
  test("Disk cache can recover gracefully from invalid JSON"):
    withDiskCache(testActionCacheInvalidJson)

+  test("Disk cache caches CompileFailed exceptions"):
+    withDiskCache(testCachedCompileFailure)
+
  def testActionCacheInvalidJson(cache: DiskActionCacheStore): Unit =
    import sjsonnew.BasicJsonProtocol.*
    var called = 0
@ -105,6 +115,59 @@ object ActionCacheTest extends BasicTestSuite:
      // check that the action has been invoked twice
      assert(called == 2)

+  def testCachedCompileFailure(cache: DiskActionCacheStore): Unit =
+    import sjsonnew.BasicJsonProtocol.*
+    var called = 0
+    val testProblem = new Problem:
+      override def category(): String = "Test"
+      override def severity(): Severity = Severity.Error
+      override def message(): String = "Test error message"
+      override def position(): Position = new Position:
+        override def line(): Optional[Integer] = Optional.of(42)
+        override def lineContent(): String = "val x = 1"
+        override def offset(): Optional[Integer] = Optional.empty()
+        override def pointer(): Optional[Integer] = Optional.empty()
+        override def pointerSpace(): Optional[String] = Optional.empty()
+        override def sourcePath(): Optional[String] = Optional.of("/test/file.scala")
+        override def sourceFile(): Optional[java.io.File] = Optional.empty()
+
+    val testException = new CompileFailed:
+      override def arguments(): Array[String] = Array.empty
+      override def problems(): Array[Problem] = Array(testProblem)
+      override def getMessage(): String = "Compilation failed"
+
+    val action: ((Int, Int)) => InternalActionResult[Int] = { (a, b) =>
+      called += 1
+      throw testException
+    }
+    IO.withTemporaryDirectory: tempDir =>
+      val config = getCacheConfig(cache, tempDir)
+
+      // First call should throw and cache the failure
+      var caught1: CompileFailed = null
+      try
+        ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action)
+        assert(false, "Expected CompileFailed to be thrown")
+      catch case e: CompileFailed => caught1 = e
+
+      assert(caught1 != null)
+      assert(called == 1)
+
+      // Second call should throw cached failure without calling action again
+      var caught2: CompileFailed = null
+      try
+        ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action)
+        assert(false, "Expected CompileFailed to be thrown")
+      catch case e: CompileFailed => caught2 = e
+
+      assert(caught2 != null)
+      // Action should NOT have been called again - failure was cached
+      assert(called == 1)
+      // Verify the cached exception has the same data
+      assert(caught2.problems().length == 1)
+      assert(caught2.problems()(0).message() == "Test error message")
+      assert(caught2.getMessage() == "Compilation failed")
+
  def withInMemoryCache(f: InMemoryActionCacheStore => Unit): Unit =
    val cache = InMemoryActionCacheStore()
    f(cache)