ExternalCommand: more information in error conditions (#5016)

* ExternalCommand: more information in error conditions

* report exit code if it's non-zero
* pass on original error (if any) rather than disregarding it
* log.warn stderr output (if any)
* add tests

* compiler warning fix

* exit code `2` on linux, `1` on mac...

* fix for mac

* error msg is different on windows

joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/utils/ExternalCommand.scala

@@ -1,28 +1,38 @@
 package io.joern.x2cpg.utils
 
+import org.slf4j.LoggerFactory
+
 import java.io.File
-import java.net.URL
 import java.nio.file.{Path, Paths}
 import java.util.concurrent.ConcurrentLinkedQueue
 import scala.sys.process.{Process, ProcessLogger}
 import scala.util.{Failure, Success, Try}
 import scala.jdk.CollectionConverters.*
+import System.lineSeparator
 
 trait ExternalCommand {
 
+  private val logger = LoggerFactory.getLogger(this.getClass)
+
   protected val IsWin: Boolean = scala.util.Properties.isWin
 
   // do not prepend any shell layer by default
   // individual frontends may override this
   protected val shellPrefix: Seq[String] = Nil
 
   protected def handleRunResult(result: Try[Int], stdOut: Seq[String], stdErr: Seq[String]): Try[Seq[String]] = {
+    if (stdErr.nonEmpty) logger.warn(s"subprocess stderr: ${stdErr.mkString(lineSeparator)}")
+
     result match {
-      case Success(0) =>
-        Success(stdOut)
-      case _ =>
+      case Success(0)     => Success(stdOut)
+      case Failure(error) => Failure(error)
+      case Success(nonZeroExitCode) =>
         val allOutput = stdOut ++ stdErr
-        Failure(new RuntimeException(allOutput.mkString(System.lineSeparator())))
+        val message =
+          s"""Process exited with code $nonZeroExitCode. Output:
+             |${allOutput.mkString(lineSeparator)}
+             |""".stripMargin
+        Failure(new RuntimeException(message))
     }
   }
 

joern-cli/frontends/x2cpg/src/test/scala/io/joern/x2cpg/utils/ExternalCommandTest.scala

@@ -4,9 +4,11 @@ import better.files.File
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.wordspec.AnyWordSpec
 
+import scala.util.Properties.isWin
 import scala.util.{Failure, Success}
 
 class ExternalCommandTest extends AnyWordSpec with Matchers {
+  def cwd = File.currentWorkingDirectory.pathAsString
 
   "ExternalCommand.run" should {
     "be able to run `ls` successfully" in {
@@ -15,6 +17,29 @@ class ExternalCommandTest extends AnyWordSpec with Matchers {
         ExternalCommand.run(cmd, sourceDir.pathAsString) should be a Symbol("success")
       }
     }
+
+    "report exit code and stdout/stderr for nonzero exit code" in {
+      ExternalCommand.run("ls /does/not/exist", cwd) match {
+        case result: Success[_] =>
+          fail(s"expected failure, but got $result")
+        case Failure(exception) =>
+          exception.getMessage should include("Process exited with code")  // exit code `2` on linux, `1` on mac...
+          exception.getMessage should include("No such file or directory") // again, different errors on mac and linux
+      }
+    }
+
+    "report error for io exception (e.g. for nonexisting command)" in {
+      ExternalCommand.run("/command/does/not/exist", cwd) match {
+        case result: Success[_] =>
+          fail(s"expected failure, but got $result")
+        case Failure(exception) =>
+          exception.getMessage should include("""Cannot run program "/command/does/not/exist"""")
+          if (isWin)
+            exception.getMessage should include("The system cannot find the file")
+          else
+            exception.getMessage should include("No such file or directory")
+      }
+    }
   }
 
 }