Merge pull request #4378 from andreaTP/fix4370

Fix: #4370

main/src/main/scala/sbt/MainLoop.scala

@@ -148,21 +148,36 @@ object MainLoop {
     val channelName = exec.source map (_.channelName)
     StandardMain.exchange publishEventMessage
       ExecStatusEvent("Processing", channelName, exec.execId, Vector())
-    val newState = Command.process(exec.commandLine, state)
-    val doneEvent = ExecStatusEvent(
-      "Done",
-      channelName,
-      exec.execId,
-      newState.remainingCommands.toVector map (_.commandLine),
-      exitCode(newState, state),
-    )
-    if (doneEvent.execId.isDefined) { // send back a response or error
-      import sbt.protocol.codec.JsonProtocol._
-      StandardMain.exchange publishEvent doneEvent
-    } else { // send back a notification
-      StandardMain.exchange publishEventMessage doneEvent
+
+    try {
+      val newState = Command.process(exec.commandLine, state)
+      val doneEvent = ExecStatusEvent(
+        "Done",
+        channelName,
+        exec.execId,
+        newState.remainingCommands.toVector map (_.commandLine),
+        exitCode(newState, state),
+      )
+      if (doneEvent.execId.isDefined) { // send back a response or error
+        import sbt.protocol.codec.JsonProtocol._
+        StandardMain.exchange publishEvent doneEvent
+      } else { // send back a notification
+        StandardMain.exchange publishEventMessage doneEvent
+      }
+      newState
+    } catch {
+      case err: Throwable =>
+        val errorEvent = ExecStatusEvent(
+          "Error",
+          channelName,
+          exec.execId,
+          Vector(),
+          ExitCode(ErrorCodes.UnknownError),
+        )
+        import sbt.protocol.codec.JsonProtocol._
+        StandardMain.exchange publishEvent errorEvent
+        throw err
     }
-    newState
   }
 
   def logFullException(e: Throwable, log: Logger): Unit = State.logFullException(e, log)

sbt/src/server-test/events/build.sbt

@@ -0,0 +1,2 @@
+
+commands += Command.command("hello") { state => ??? }

sbt/src/test/scala/sbt/RunFromSourceMain.scala

@@ -7,23 +7,23 @@
 
 package sbt
 
-import scala.util.Try
 import sbt.util.LogExchange
 import scala.annotation.tailrec
 import buildinfo.TestBuildInfo
 import xsbti._
+import scala.sys.process.Process
 
 object RunFromSourceMain {
   private val sbtVersion = "1.1.4" // TestBuildInfo.version
   private val scalaVersion = "2.12.6"
 
-  def fork(workingDirectory: File): Try[Unit] = {
+  def fork(workingDirectory: File): Process = {
     val fo = ForkOptions()
       .withOutputStrategy(OutputStrategy.StdoutOutput)
     fork(fo, workingDirectory)
   }
 
-  def fork(fo0: ForkOptions, workingDirectory: File): Try[Unit] = {
+  def fork(fo0: ForkOptions, workingDirectory: File): Process = {
     val fo = fo0
       .withWorkingDirectory(workingDirectory)
     implicit val runner = new ForkRun(fo)
@@ -32,7 +32,7 @@ object RunFromSourceMain {
     }
     val options = Vector(workingDirectory.toString)
     val log = LogExchange.logger("RunFromSourceMain.fork", None, None)
-    Run.run("sbt.RunFromSourceMain", cp, options, log)
+    runner.fork("sbt.RunFromSourceMain", cp, options, log)
   }
 
   def main(args: Array[String]): Unit = args match {

sbt/src/test/scala/testpkg/ServerSpec.scala

@@ -16,6 +16,8 @@ import java.io.File
 import sbt.io.syntax._
 import sbt.io.IO
 import sbt.RunFromSourceMain
+import scala.concurrent.ExecutionContext
+import java.util.concurrent.ForkJoinPool
 
 class ServerSpec extends AsyncFreeSpec with Matchers {
   "server" - {
@@ -36,10 +38,22 @@ class ServerSpec extends AsyncFreeSpec with Matchers {
         s contains """"id":3"""
       })
     }
+
+    "report task failures in case of exceptions" in withTestServer("events") { p =>
+      p.writeLine(
+        """{ "jsonrpc": "2.0", "id": 11, "method": "sbt/exec", "params": { "commandLine": "hello" } }"""
+      )
+      assert(p.waitForString(10) { s =>
+        (s contains """"id":11""") && (s contains """"error":""")
+      })
+    }
   }
 }
 
 object TestServer {
+  // The test server instance will be executed in a Thread pool separated from the tests
+  implicit val ec = ExecutionContext.fromExecutor(new ForkJoinPool())
+
   private val serverTestBase: File = new File(".").getAbsoluteFile / "sbt" / "src" / "server-test"
 
   def withTestServer(testBuild: String)(f: TestServer => Future[Assertion]): Future[Assertion] = {
@@ -54,7 +68,7 @@ object TestServer {
     try {
       f(testServer)
     } finally {
-      testServer.bye()
+      try { testServer.bye() } finally {}
     }
   }
 
@@ -63,7 +77,7 @@ object TestServer {
   }
 }
 
-case class TestServer(baseDirectory: File) {
+case class TestServer(baseDirectory: File)(implicit ec: ExecutionContext) {
   import TestServer.hostLog
 
   val readBuffer = new Array[Byte](4096)
@@ -73,11 +87,11 @@ case class TestServer(baseDirectory: File) {
   private val RetByte = '\r'.toByte
 
   hostLog("fork to a new sbt instance")
-  import scala.concurrent.ExecutionContext.Implicits.global
-  Future {
-    RunFromSourceMain.fork(baseDirectory)
-    ()
-  }
+  val process =
+    Future {
+      RunFromSourceMain.fork(baseDirectory)
+    }
+
   lazy val portfile = baseDirectory / "project" / "target" / "active.json"
 
   hostLog("wait 30s until the server is ready to respond")
@@ -114,6 +128,11 @@ case class TestServer(baseDirectory: File) {
     sendJsonRpc(
       """{ "jsonrpc": "2.0", "id": 9, "method": "sbt/exec", "params": { "commandLine": "exit" } }"""
     )
+    for {
+      p <- process
+    } {
+      p.destroy()
+    }
   }
 
   def sendJsonRpc(message: String): Unit = {