Skip to content

Commit

Permalink
improvement: Send error back to client to show
Browse files Browse the repository at this point in the history
Previously, it would be very difficult to show a proper error message to the user. Especially in VS Code a generic message was show, which was very confusing for users.

Now, we send the error message in the response so that the client can handle it as they want.

I wonder if we should keep displaying the message, but not sure if it's not better to just change the clients here to show the error.
  • Loading branch information
tgodzik committed Nov 17, 2023
1 parent a07ab91 commit 9338a6e
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
package scala.meta.internal.metals

final case class DebugSession(name: String, uri: String)
import javax.annotation.Nullable

final case class DebugSession(
@Nullable name: String,
@Nullable uri: String,
@Nullable error: String,
)

object DebugSession {

def success(name: String, uri: String): DebugSession = {
DebugSession(name, uri, null)
}
def failure(message: String): DebugSession = {
DebugSession(null, null, message)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1908,6 +1908,7 @@ class MetalsLspService(
debugProvider
.debugDiscovery(params)
.flatMap(debugProvider.asSession)
.recover(t => DebugSession.failure(t.getMessage()))
.asJavaObject

def findBuildTargetByDisplayName(target: String): Option[b.BuildTarget] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -639,11 +637,6 @@ class WorkspaceLspService(
folderServices.foreach(_.pause())
}

private def displayNotFound(objectName: String, id: String) =
languageClient.showMessage(
Messages.errorMessageParams(s"$objectName not found: $id")
)

private def onFirstSatifying[T, R](mapTo: MetalsLspService => Future[T])(
satisfies: T => Boolean,
exec: (MetalsLspService, T) => Future[R],
Expand Down Expand Up @@ -854,8 +847,9 @@ class WorkspaceLspService(
.getResultFromSearches(
folderServices.map(_.mainClassSearch(params))
) {
displayNotFound("Main class", params.mainClass)
Future.failed(new ju.NoSuchElementException(params.mainClass))
Future.successful(
DebugSession.failure(s"Main class not found: ${params.mainClass}")
)
}
.asJavaObject

Expand All @@ -868,8 +862,11 @@ class WorkspaceLspService(
(service, someTarget) =>
service.startTestSuite(someTarget.get, params),
() => {
displayNotFound("Build target", params.target.toString())
Future.failed(BuildTargetNotFoundException(params.target.getUri))
Future.successful(
DebugSession.failure(
s"Build target not found: ${params.target.toString()}"
)
)
},
).asJavaObject
case ServerCommands.ResolveAndStartTestSuite(params)
Expand All @@ -878,8 +875,9 @@ class WorkspaceLspService(
.getResultFromSearches(
folderServices.map(_.testClassSearch(params))
) {
displayNotFound("Test class", params.testClass)
Future.failed(new ju.NoSuchElementException(params.testClass))
Future.successful(
DebugSession.failure(s"Test class: ${params.testClass}")
)
}
.asJavaObject
case ServerCommands.StartAttach(params) if params.hostName != null =>
Expand All @@ -892,8 +890,9 @@ class WorkspaceLspService(
(service, someTarget) =>
service.createDebugSession(someTarget.get.getId()),
() => {
displayNotFound("Build target", params.buildTarget)
Future.failed(BuildTargetUndefinedException())
Future.successful(
DebugSession.failure(s"Build target: ${params.buildTarget}")
)
},
).asJavaObject
case ServerCommands.DiscoverAndRun(params) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ class DebugProvider(
)
} yield {
statusBar.addMessage("Started debug server!")
DebugSession(server.sessionName, server.uri.toString)
DebugSession.success(server.sessionName, server.uri.toString)
}
}

Expand Down
4 changes: 2 additions & 2 deletions tests/unit/src/main/scala/tests/TestingServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ final case class TestingServer(
params.setDataKind(kind)
params.setData(parameter.toJson)
executeCommandUnsafe(ServerCommands.StartDebugAdapter.id, Seq(params))
.collect { case DebugSession(_, uri) =>
.collect { case DebugSession(_, uri, _) =>
scribe.info(s"Starting debug session for $uri")
TestDebugger(URI.create(uri), stoppageHandler)
}
Expand Down Expand Up @@ -708,7 +708,7 @@ final case class TestingServer(
): Future[TestDebugger] = {
assertSystemExit(params)
executeCommandUnsafe(ServerCommands.StartDebugAdapter.id, Seq(params))
.collect { case DebugSession(_, uri) =>
.collect { case DebugSession(_, uri, _) =>
TestDebugger(URI.create(uri), stoppageHandler)
}
}
Expand Down

0 comments on commit 9338a6e

Please sign in to comment.