improvement: have Dismiss. remember the bsp error and don't show it…

… again

metals/src/main/scala/scala/meta/internal/builds/BSPErrorHandler.scala

@@ -9,6 +9,7 @@ import scala.util.control.NonFatal
 
 import scala.meta.internal.bsp.BspSession
 import scala.meta.internal.metals.ClientCommands
+import scala.meta.internal.metals.ConcurrentHashSet
 import scala.meta.internal.metals.Directories
 import scala.meta.internal.metals.MetalsEnrichments._
 import scala.meta.internal.metals.clients.language.MetalsLanguageClient
@@ -23,27 +24,32 @@ class BspErrorHandler(
     currentSession: () => Option[BspSession],
 )(implicit context: ExecutionContext) {
 
-  private def logsPath = workspaceFolder.resolve(Directories.log)
+  protected def logsPath: AbsolutePath =
+    workspaceFolder.resolve(Directories.log)
   private val lastError = new AtomicReference[String]("")
+  private val dismissedErrors = ConcurrentHashSet.empty[String]
 
-  def onError(message: String): Unit = {
-    if (shouldShowBspError) {
+  def onError(message: String): Future[Unit] = {
+    if (shouldShowBspError && !dismissedErrors.contains(message)) {
       val previousError = lastError.getAndSet(message)
       if (message != previousError) {
         showError(message)
-      }
+      } else Future.successful(())
     } else {
-      scribe.error(message)
+      logError(message)
+      Future.successful(())
     }
   }
 
-  private def shouldShowBspError = currentSession().exists(session =>
+  def shouldShowBspError: Boolean = currentSession().exists(session =>
     session.main.isBloop || session.main.isScalaCLI
   )
 
+  protected def logError(message: String): Unit = scribe.error(message)
+
   private def showError(message: String): Future[Unit] = {
-    val bspError = s"${BspErrorHandler.errorInBsp}: $message"
-    scribe.error(bspError)
+    val bspError = s"${BspErrorHandler.errorInBsp} $message"
+    logError(bspError)
     val params = BspErrorHandler.makeShowMessage(message)
     languageClient.showMessageRequest(params).asScala.flatMap {
       case BspErrorHandler.goToLogs =>
@@ -54,6 +60,8 @@ class BspErrorHandler(
         Future.successful(gotoLogs(errorMsgStartLine))
       case BspErrorHandler.restartBuildServer =>
         restartBspServer().ignoreValue
+      case BspErrorHandler.dismiss =>
+        Future.successful(dismissedErrors.add(message)).ignoreValue
       case _ => Future.successful(())
     }
   }
@@ -94,15 +102,12 @@ object BspErrorHandler {
     val (msg, actions) =
       if (message.length() <= MESSAGE_MAX_LENGTH) {
         (
-          s"""|$errorHeader
-              |$message""".stripMargin,
+          makeShortMessage(message),
           List(restartBuildServer, dismiss),
         )
       } else {
         (
-          s"""|$errorHeader
-              |${message.take(MESSAGE_MAX_LENGTH)}...
-              |$gotoLogsToSeeFull""".stripMargin,
+          makeLongMessage(message),
           List(goToLogs, restartBuildServer, dismiss),
         )
       }
@@ -113,13 +118,23 @@ object BspErrorHandler {
     params
   }
 
-  private val errorHeader = "Encountered an error in the build server:"
-  private val goToLogs = new l.MessageActionItem("Go to logs.")
-  private val restartBuildServer =
+  def makeShortMessage(message: String): String =
+    s"""|$errorHeader
+        |$message""".stripMargin
+
+  def makeLongMessage(message: String): String =
+    s"""|${makeShortMessage(s"${message.take(MESSAGE_MAX_LENGTH)}...")}
+        |$gotoLogsToSeeFull""".stripMargin
+
+  val goToLogs = new l.MessageActionItem("Go to logs.")
+  val dismiss = new l.MessageActionItem("Dismiss.")
+  val restartBuildServer =
     new l.MessageActionItem("Restart build server.")
-  private val dismiss = new l.MessageActionItem("Dismiss.")
+
+  val errorHeader = "Encountered an error in the build server:"
   private val gotoLogsToSeeFull = "Go to logs to see the full error"
-  private val errorInBsp = "Build server error:"
+
+  val errorInBsp = "Build server error:"
 
   private val MESSAGE_MAX_LENGTH = 150
 

tests/unit/src/main/scala/tests/TestingClient.scala

@@ -12,6 +12,7 @@ import scala.concurrent.Future
 import scala.concurrent.Promise
 
 import scala.meta.inputs.Input
+import scala.meta.internal.builds.BspErrorHandler
 import scala.meta.internal.builds.BuildTool
 import scala.meta.internal.builds.BuildTools
 import scala.meta.internal.decorations.PublishDecorationsParams
@@ -89,6 +90,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers)
   var buildServerNotResponding =
     ServerLivenessMonitor.ServerNotResponding.dismiss
   var regenerateAndRestartScalaCliBuildSever = FileOutOfScalaCliBspScope.ignore
+  var bspError = BspErrorHandler.dismiss
 
   val resources = new ResourceOperations(buffers)
   val diagnostics: TrieMap[AbsolutePath, Seq[Diagnostic]] =
@@ -371,6 +373,10 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers)
           regenerateAndRestartScalaCliBuildSever
         } else if (params.getMessage() == choicesMessage) {
           params.getActions().asScala.head
+        } else if (
+          params.getMessage().startsWith(BspErrorHandler.errorHeader)
+        ) {
+          bspError
         } else {
           throw new IllegalArgumentException(params.toString)
         }

tests/unit/src/test/scala/tests/BspErrorHandlerSuite.scala

@@ -0,0 +1,106 @@
+package tests
+
+import java.nio.file.Paths
+import java.util.concurrent.Executors
+
+import scala.concurrent.ExecutionContext
+import scala.concurrent.Future
+
+import scala.meta.internal.builds.BspErrorHandler
+import scala.meta.internal.metals.Buffers
+import scala.meta.internal.metals.Directories
+import scala.meta.internal.metals.MetalsEnrichments._
+import scala.meta.io.AbsolutePath
+
+import com.google.gson.JsonObject
+
+class BspErrorHandlerSuite extends BaseSuite {
+  implicit val ec: ExecutionContext =
+    ExecutionContext.fromExecutorService(Executors.newCachedThreadPool())
+
+  val workspace: AbsolutePath = AbsolutePath(Paths.get("."))
+    .resolve("target/bsp-error-suite/")
+    .createDirectories()
+  val client = new TestingClient(workspace, Buffers())
+  val errorHandler = new TestBspErrorHandler(client, workspace)
+  val exampleError1 = "an error"
+  val exampleError2 = "a different error"
+  val longError: String =
+    """|This is a long error.
+       |Such a long error will not fully fit in the
+       |message box to be show to the user.
+       |It will get trimmed to maximum 150 characters.
+       |The full message will be available in the metals logs.
+       |""".stripMargin
+
+  test("handle-bsp-error") {
+    FileLayout.fromString(
+      s"""|/.metals/metals.log
+          |
+          |""".stripMargin,
+      workspace,
+    )
+
+    client.bspError = BspErrorHandler.restartBuildServer
+
+    for {
+      _ <- errorHandler.onError(exampleError1)
+      _ <- errorHandler.onError(exampleError1)
+      _ <- errorHandler.onError(exampleError2)
+      _ = client.bspError = BspErrorHandler.dismiss
+      _ <- errorHandler.onError(exampleError1)
+      _ = client.bspError = BspErrorHandler.goToLogs
+      _ <- errorHandler.onError(longError)
+      _ <- errorHandler.onError(exampleError1)
+    } yield {
+      assertNoDiff(
+        client.workspaceMessageRequests,
+        s"""|${BspErrorHandler.makeShortMessage(exampleError1)}
+            |${BspErrorHandler.makeShortMessage(exampleError2)}
+            |${BspErrorHandler.makeShortMessage(exampleError1)}
+            |${BspErrorHandler.makeLongMessage(longError)}
+            |""".stripMargin,
+      )
+      assert(client.clientCommands.asScala.nonEmpty)
+      assertNoDiff(
+        client.clientCommands.asScala.head.getCommand(),
+        "metals-goto-location",
+      )
+      val params = client.clientCommands.asScala.head
+        .getArguments()
+        .asScala
+        .head
+        .asInstanceOf[JsonObject]
+      assertEquals(
+        params.get("uri").getAsString(),
+        workspace.resolve(Directories.log).toURI.toString(),
+      )
+      assertEquals(
+        params
+          .getAsJsonObject("range")
+          .getAsJsonObject("start")
+          .get("line")
+          .getAsInt(),
+        4,
+      )
+      workspace.resolve(Directories.log).delete()
+    }
+  }
+
+}
+
+class TestBspErrorHandler(
+    val languageClient: TestingClient,
+    workspaceFolder: AbsolutePath,
+)(implicit context: ExecutionContext)
+    extends BspErrorHandler(
+      languageClient,
+      workspaceFolder,
+      () => Future.successful(true),
+      () => None,
+    ) {
+  override def shouldShowBspError: Boolean = true
+
+  override def logError(message: String): Unit =
+    logsPath.appendText(s"$message\n")
+}