improvement: Forward DAP errors properly to the client

Previously, we would not show the errors coming from debug discovery and no errors would be forwarded to the client.

Now, we do those two things:
- we no longer hide the discovery errors, but in case no folder contained valid run configuration, we try to show the most relevant error
- instead of throwing a random exception we use LSP build-in one, which is sent back to the client properly and will be displayed

Closes scalameta/metals-vscode#1438

metals/src/main/scala/scala/meta/internal/metals/Messages.scala

@@ -585,31 +585,6 @@ object Messages {
       show = true,
     )
 
-  object DebugClassNotFound {
-
-    def invalidTargetClass(cls: String, target: String): MessageParams = {
-      new MessageParams(
-        MessageType.Error,
-        s"Class '$cls' not found in build target '$target'.",
-      )
-    }
-
-    def invalidTarget(target: String): MessageParams = {
-      new MessageParams(
-        MessageType.Error,
-        s"Target '$target' not found.",
-      )
-    }
-
-    def invalidClass(cls: String): MessageParams = {
-      new MessageParams(
-        MessageType.Error,
-        s"Class '$cls' not found.",
-      )
-    }
-
-  }
-
   object MissingScalafmtConf {
     def tryAgain(isAgain: Boolean): String =
       if (isAgain) ", try formatting again"

metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala

@@ -58,6 +58,8 @@ import com.google.gson.JsonObject
 import fansi.ErrorMode
 import io.undertow.server.HttpServerExchange
 import org.eclipse.lsp4j.TextDocumentIdentifier
+import org.eclipse.lsp4j.jsonrpc.ResponseErrorException
+import org.eclipse.lsp4j.jsonrpc.messages
 import org.eclipse.{lsp4j => l}
 
 /**
@@ -203,6 +205,17 @@ object MetalsEnrichments
     def asJavaObject: CompletableFuture[Object] =
       future.asJava.asInstanceOf[CompletableFuture[Object]]
 
+    def liftToLspError(implicit ec: ExecutionContext): Future[A] =
+      future.recoverWith { case NonFatal(e) =>
+        val newException = new ResponseErrorException(
+          new messages.ResponseError(
+            messages.ResponseErrorCode.InvalidRequest,
+            e.getMessage(),
+            null,
+          )
+        )
+        Future.failed(newException)
+      }
     def asJavaUnit(implicit ec: ExecutionContext): CompletableFuture[Unit] =
       future.ignoreValue.asJava
 

metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala

@@ -1904,11 +1904,10 @@ class MetalsLspService(
   def analyzeStackTrace(content: String): Option[ExecuteCommandParams] =
     stacktraceAnalyzer.analyzeCommand(content)
 
-  def debugDiscovery(params: DebugDiscoveryParams): CompletableFuture[Object] =
+  def debugDiscovery(params: DebugDiscoveryParams): Future[DebugSession] =
     debugProvider
       .debugDiscovery(params)
       .flatMap(debugProvider.asSession)
-      .asJavaObject
 
   def findBuildTargetByDisplayName(target: String): Option[b.BuildTarget] =
     buildTargets.findByDisplayName(target)

metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala

@@ -23,8 +23,6 @@ import scala.meta.internal.metals.WindowStateDidChangeParams
 import scala.meta.internal.metals.clients.language.ConfiguredLanguageClient
 import scala.meta.internal.metals.clients.language.MetalsLanguageClient
 import scala.meta.internal.metals.config.StatusBarState
-import scala.meta.internal.metals.debug.BuildTargetNotFoundException
-import scala.meta.internal.metals.debug.BuildTargetUndefinedException
 import scala.meta.internal.metals.debug.DebugProvider
 import scala.meta.internal.metals.doctor.DoctorVisibilityDidChangeParams
 import scala.meta.internal.metals.doctor.HeadDoctor
@@ -93,6 +91,7 @@ import org.eclipse.lsp4j.TextDocumentPositionParams
 import org.eclipse.lsp4j.TextEdit
 import org.eclipse.lsp4j.WorkspaceEdit
 import org.eclipse.lsp4j.WorkspaceSymbolParams
+import org.eclipse.lsp4j.jsonrpc.ResponseErrorException
 import org.eclipse.lsp4j.jsonrpc.messages
 
 class WorkspaceLspService(
@@ -639,10 +638,20 @@ class WorkspaceLspService(
       folderServices.foreach(_.pause())
     }
 
-  private def displayNotFound(objectName: String, id: String) =
-    languageClient.showMessage(
-      Messages.errorMessageParams(s"$objectName not found: $id")
-    )
+  private def failedRequest(
+      message: String
+  ): Future[Object] = {
+    Future
+      .failed(
+        new ResponseErrorException(
+          new messages.ResponseError(
+            messages.ResponseErrorCode.InvalidParams,
+            message,
+            null,
+          )
+        )
+      )
+  }
 
   private def onFirstSatifying[T, R](mapTo: MetalsLspService => Future[T])(
       satisfies: T => Boolean,
@@ -842,21 +851,16 @@ class WorkspaceLspService(
         ) match {
           case Some(service) => service.startDebugProvider(params).asJavaObject
           case None =>
-            languageClient.showMessage(
-              Messages.errorMessageParams(
-                s"Could not find folder for build targets: ${targets.mkString(",")}"
-              )
-            )
-            Future.failed(DebugProvider.WorkspaceErrorsException).asJavaObject
+            failedRequest(
+              s"Could not find folder for build targets: ${targets.mkString(",")}"
+            ).asJavaObject
         }
       case ServerCommands.StartMainClass(params) if params.mainClass != null =>
         DebugProvider
           .getResultFromSearches(
             folderServices.map(_.mainClassSearch(params))
-          ) {
-            displayNotFound("Main class", params.mainClass)
-            Future.failed(new ju.NoSuchElementException(params.mainClass))
-          }
+          )
+          .liftToLspError
           .asJavaObject
 
       case ServerCommands.StartTestSuite(params)
@@ -867,20 +871,15 @@ class WorkspaceLspService(
           _.isDefined,
           (service, someTarget) =>
             service.startTestSuite(someTarget.get, params),
-          () => {
-            displayNotFound("Build target", params.target.toString())
-            Future.failed(BuildTargetNotFoundException(params.target.getUri))
-          },
+          () => failedRequest(s"Could not find '${params.target}' build target"),
         ).asJavaObject
       case ServerCommands.ResolveAndStartTestSuite(params)
           if params.testClass != null =>
         DebugProvider
           .getResultFromSearches(
             folderServices.map(_.testClassSearch(params))
-          ) {
-            displayNotFound("Test class", params.testClass)
-            Future.failed(new ju.NoSuchElementException(params.testClass))
-          }
+          )
+          .liftToLspError
           .asJavaObject
       case ServerCommands.StartAttach(params) if params.hostName != null =>
         onFirstSatifying(service =>
@@ -891,13 +890,16 @@ class WorkspaceLspService(
           _.isDefined,
           (service, someTarget) =>
             service.createDebugSession(someTarget.get.getId()),
-          () => {
-            displayNotFound("Build target", params.buildTarget)
-            Future.failed(BuildTargetUndefinedException())
-          },
+          () =>
+            failedRequest(
+              s"Could not find '${params.buildTarget}' build target"
+            ),
         ).asJavaObject
       case ServerCommands.DiscoverAndRun(params) =>
-        getServiceFor(params.path).debugDiscovery(params)
+        getServiceFor(params.path)
+          .debugDiscovery(params)
+          .liftToLspError
+          .asJavaObject
       case ServerCommands.AnalyzeStacktrace(content) =>
         Future {
           // Getting the service for focused document and first one otherwise

metals/src/main/scala/scala/meta/internal/metals/debug/BuildTargetClassesFinder.scala

@@ -39,8 +39,9 @@ class BuildTargetClassesFinder(
         // We check whether there is a main in dependencies that is not reported via BSP
         case ClassNotFoundInBuildTargetException(className, target) =>
           revertToDependencies(className, Some(target))
-        case _: ClassNotFoundException =>
+        case _: NoClassFoundException =>
           revertToDependencies(className, buildTarget = None)
+        case _ => Nil
       }
       found match {
         case Nil => Failure(ex)
@@ -109,7 +110,7 @@ class BuildTargetClassesFinder(
           }
           .reverse
       if (classes.nonEmpty) Success(classes)
-      else Failure(new ClassNotFoundException(className))
+      else Failure(new NoClassFoundException(className))
     } { targetName =>
       buildTargets
         .findByDisplayName(targetName)
@@ -148,15 +149,19 @@ class BuildTargetClassesFinder(
 case class BuildTargetNotFoundException(buildTargetName: String)
     extends Exception(s"Build target not found: $buildTargetName")
 
-case class BuildTargetUndefinedException()
-    extends Exception("Debugger configuration is missing 'buildTarget' param.")
-
 case class ClassNotFoundInBuildTargetException(
     className: String,
     buildTarget: b.BuildTarget,
 ) extends Exception(
       s"Class '$className' not found in build target '${buildTarget.getDisplayName()}'"
     )
+
+case class NoClassFoundException(
+    className: String
+) extends Exception(
+      s"Class '$className' not found in any build target"
+    )
+
 case class BuildTargetNotFoundForPathException(path: AbsolutePath)
     extends Exception(
       s"No build target could be found for the path: ${path.toString()}"

metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala

@@ -688,40 +688,6 @@ class DebugProvider(
       languageClient.metalsStatus(
         Messages.DebugErrorsPresent(clientConfig.icons())
       )
-    case t: ClassNotFoundException =>
-      languageClient.showMessage(
-        Messages.DebugClassNotFound.invalidClass(t.getMessage())
-      )
-    case ClassNotFoundInBuildTargetException(cls, buildTarget) =>
-      languageClient.showMessage(
-        Messages.DebugClassNotFound
-          .invalidTargetClass(cls, buildTarget.getDisplayName())
-      )
-    case BuildTargetNotFoundException(target) =>
-      languageClient.showMessage(
-        Messages.DebugClassNotFound
-          .invalidTarget(target)
-      )
-    case e: BuildTargetNotFoundForPathException =>
-      languageClient.showMessage(
-        Messages.errorMessageParams(e.getMessage())
-      )
-    case e: BuildTargetContainsNoMainException =>
-      languageClient.showMessage(
-        Messages.errorMessageParams(e.getMessage())
-      )
-    case e: BuildTargetUndefinedException =>
-      languageClient.showMessage(
-        Messages.errorMessageParams(e.getMessage())
-      )
-    case e: NoTestsFoundException =>
-      languageClient.showMessage(
-        Messages.errorMessageParams(e.getMessage())
-      )
-    case e: RunType.UnknownRunTypeException =>
-      languageClient.showMessage(
-        Messages.errorMessageParams(e.getMessage())
-      )
     case e @ SemanticDbNotFoundException =>
       languageClient.metalsStatus(
         MetalsStatusParams(
@@ -730,13 +696,7 @@ class DebugProvider(
           command = ClientCommands.RunDoctor.id,
         )
       )
-    case e @ DotEnvFileParser.InvalidEnvFileException(_) =>
-      languageClient.showMessage(Messages.errorMessageParams(e.getMessage()))
-
-    case e @ NoRunOptionException =>
-      languageClient.showMessage(
-        Messages.errorMessageParams(e.getMessage())
-      )
+    case _ =>
   }
 
   private def parseSessionName(
@@ -829,7 +789,7 @@ class DebugProvider(
           _ <- buildTargetClasses.rebuildIndex(target)
           result <- Future(f())
         } yield result
-      case Failure(_: ClassNotFoundException) =>
+      case Failure(_: NoClassFoundException) =>
         val allTargetIds = buildTargets.allBuildTargetIds
         for {
           _ <- compilations.compileTargets(allTargetIds)
@@ -867,6 +827,26 @@ class DebugProvider(
 
 object DebugProvider {
 
+  private def exceptionOrder(t: Throwable): Int = t match {
+    /* Validation errors, should shown first as they are relevant for each build target.
+     */
+    case DotEnvFileParser.InvalidEnvFileException(_) => 0
+    case NoRunOptionException => 1
+    case _: RunType.UnknownRunTypeException => 2
+    /* Target found, but class not, means that we managed to find the proper workspace folder
+     */
+    case _: ClassNotFoundInBuildTargetException => 3
+    case _: BuildTargetContainsNoMainException => 4
+    case _: NoTestsFoundException => 5
+    /* These exception will show up and it's hard to pinpoint which folder we should be in
+     * since we failed to fin the exception anywhere. Probably just showing the error is enough.
+     */
+    case _: BuildTargetNotFoundForPathException => 6
+    case _: NoClassFoundException => 7
+    case _: BuildTargetNotFoundException => 8
+    case _ => 9
+  }
+
   /**
    * Given an occurence and a text document return the symbol of a main method
    * that could be defined using the Scala 3 @main annotation.
@@ -958,16 +938,16 @@ object DebugProvider {
     protected def dapSessionParams(res: A): Future[DebugSessionParams]
     def createDapSession(args: A): Future[DebugSession] =
       dapSessionParams(args).flatMap(debugProvider.asSession(_))
-    def searchResult: Future[(Option[A], ClassSearch[A])] = {
+    def searchResult: Future[(Try[A], ClassSearch[A])] = {
       Future {
         val resolved = search()
         searchPromise.trySuccess(resolved)
       }
-      searchPromise.future.map(e => (e.toOption, this))
+      searchPromise.future.map(e => (e, this))
     }
-    def retrySearchResult: Future[(Option[A], ClassSearch[A])] =
+    def retrySearchResult: Future[(Try[A], ClassSearch[A])] =
       searchPromise.future
-        .flatMap(debugProvider.retryAfterRebuild(_, search).map(_.toOption))
+        .flatMap(debugProvider.retryAfterRebuild(_, search))
         .map((_, this))
   }
 
@@ -1000,27 +980,32 @@ object DebugProvider {
       debugProvider.buildTestClassParams(res, params)
   }
 
-  def getResultFromSearches[A](searches: List[ClassSearch[A]])(
-      default: => Future[DebugSession]
+  def getResultFromSearches[A](
+      searches: List[ClassSearch[A]]
   )(implicit ec: ExecutionContext): Future[DebugSession] =
     Future
       .sequence(searches.map(_.searchResult))
-      .getFirstOrElse(
-        Future
-          .sequence(searches.map(_.retrySearchResult))
-          .getFirstOrElse(default)
-      )
+      .getFirstOrError
+      .recoverWith { case _ =>
+        Future.sequence(searches.map(_.retrySearchResult)).getFirstOrError
+      }
 
   private implicit class FindFirstDebugSession[A](
-      from: Future[List[(Option[A], ClassSearch[A])]]
+      from: Future[List[(Try[A], ClassSearch[A])]]
   ) {
-    def getFirstOrElse(
-        default: => Future[DebugSession]
-    )(implicit ec: ExecutionContext): Future[DebugSession] =
-      from.flatMap {
-        _.collectFirst { case (Some(dapSession), search) =>
-          search.createDapSession(dapSession)
-        }.getOrElse(default)
+    def getFirstOrError(implicit ec: ExecutionContext): Future[DebugSession] =
+      from.flatMap { all =>
+        def mostRelevantError() = all
+          .collect { case (Failure(error), _) =>
+            error
+          }
+          .sortBy(DebugProvider.exceptionOrder)
+          .head
+        all
+          .collectFirst { case (Success(dapSession), search) =>
+            search.createDapSession(dapSession)
+          }
+          .getOrElse(Future.failed(mostRelevantError()))
       }
   }
 }