Set proper fields in ExecuteReply.Error and Execute.error (#1166)

Construct proper ExecuteReply.Error and Execute.Error complying with Jupyter protocol
almond-sh · Jun 29, 2023 · 4899b03 · 4899b03
1 parent e1bd5fa
commit 4899b03
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 22 deletions.
diff --git a/modules/scala/scala-interpreter/src/main/scala/almond/Execute.scala b/modules/scala/scala-interpreter/src/main/scala/almond/Execute.scala
@@ -416,15 +416,17 @@ final class Execute(
                 val cutoff = Set("$main", "evaluatorRunPrinter")
 
                 ExecuteResult.Error(
-                  (
-                    "Interrupted!" +: st
-                      .takeWhile(x => !cutoff(x.getMethodName))
-                      .map(ExecuteResult.Error.highlightFrame(
-                        _,
-                        fansi.Attr.Reset,
-                        colors0().literal()
-                      ))
-                  ).mkString(System.lineSeparator())
+                  "Interrupted!",
+                  "",
+                  List("Interrupted!") ++ st
+                    .takeWhile(x => !cutoff(x.getMethodName))
+                    .map(ExecuteResult.Error.highlightFrame(
+                      _,
+                      fansi.Attr.Reset,
+                      colors0().literal()
+                    ))
+                    .map(_.render)
+                    .toList
                 )
             }
 

diff --git a/modules/scala/scala-interpreter/src/test/scala/almond/ScalaInterpreterTests.scala b/modules/scala/scala-interpreter/src/test/scala/almond/ScalaInterpreterTests.scala
@@ -181,9 +181,11 @@ object ScalaInterpreterTests extends TestSuite {
       }
 
       test("exception") {
-        val code = """sys.error("foo")"""
+        val code = """sys.error("foo\nbar")"""
         val res  = interpreter.execute(code)
-        assert(res.asError.exists(_.message.contains("java.lang.RuntimeException: foo")))
+        assert(res.asError.exists(_.name.contains("java.lang.RuntimeException")))
+        assert(res.asError.exists(_.message.contains("foo\nbar")))
+        assert(res.asError.exists(_.stackTrace.exists(_.contains("ammonite."))))
       }
     }
 

diff --git a/modules/scala/scala-interpreter/src/test/scala/almond/ScalaKernelTests.scala b/modules/scala/scala-interpreter/src/test/scala/almond/ScalaKernelTests.scala
@@ -169,6 +169,24 @@ object ScalaKernelTests extends TestSuite {
 
       assert(messageTypes == expectedMessageTypes)
 
+      def checkError(ename: String, evalue: String, traceback: List[String]) = {
+        assert(ename == "java.lang.RuntimeException")
+        assert(evalue == "foo")
+        assert(traceback.exists(_.contains("java.lang.RuntimeException: foo")))
+        assert(traceback.exists(_.contains("ammonite.")))
+      }
+
+      val executeResultErrors = streams.executeResultErrors
+      assert(executeResultErrors.size == 1)
+      checkError(
+        executeResultErrors.head.ename,
+        executeResultErrors.head.evalue,
+        executeResultErrors.head.traceback
+      )
+
+      val executeErrors = streams.executeErrors
+      checkError(executeErrors(1)._1, executeErrors(1)._2, executeErrors(1)._3)
+
       val replies = streams.executeReplies
 
       // first code is in error, subsequent ones are cancelled because of the stop-on-error, so no results here

diff --git a/modules/shared/interpreter/src/main/scala/almond/interpreter/ExecuteResult.scala b/modules/shared/interpreter/src/main/scala/almond/interpreter/ExecuteResult.scala
@@ -75,42 +75,51 @@ object ExecuteResult {
       Some(rec(t))
     }
 
-    def showException(
+    def exceptionToStackTraceLines(
       ex: Throwable,
       error: fansi.Attrs,
       highlightError: fansi.Attrs,
       source: fansi.Attrs
-    ) = {
-
+    ): Seq[String] = {
       val cutoff = Set("$main", "evaluatorRunPrinter")
       val traces = unapplySeq(ex).get.map(exception =>
-        error(exception.toString).render + System.lineSeparator() +
+        Seq(error(exception.toString).render) ++
           exception
             .getStackTrace
             .takeWhile(x => !cutoff(x.getMethodName))
             .map(highlightFrame(_, highlightError, source))
-            .mkString(System.lineSeparator())
+            .map(_.render)
+            .toSeq
       )
-      traces.mkString(System.lineSeparator())
+      traces.flatten
     }
 
+    def showException(
+      ex: Throwable,
+      error: fansi.Attrs,
+      highlightError: fansi.Attrs,
+      source: fansi.Attrs
+    ) =
+      exceptionToStackTraceLines(ex, error, highlightError, source).mkString(System.lineSeparator())
+
     def error(
       errorColor: fansi.Attrs,
       literalColor: fansi.Attrs,
       exOpt: Option[Throwable],
       msg: String
     ) =
       ExecuteResult.Error(
-        msg + exOpt.fold("")(ex =>
-          (if (msg.isEmpty) "" else "\n") + showException(
+        exOpt.fold("")(_.getClass.getName),
+        msg + exOpt.fold("")(_.getMessage),
+        exOpt.fold(List.empty[String])(ex =>
+          exceptionToStackTraceLines(
             ex,
             errorColor,
             fansi.Attr.Reset,
             literalColor
-          )
+          ).toList
         )
       )
-
   }
 
   /** [[ExecuteResult]], if execution was aborted.

diff --git a/...preter/src/main/scala/almond/interpreter/messagehandlers/InterpreterMessageHandlers.scala b/...preter/src/main/scala/almond/interpreter/messagehandlers/InterpreterMessageHandlers.scala
@@ -87,7 +87,7 @@ final case class InterpreterMessageHandlers(
                     runAfterQueued(interpreter.cancelledSignal.set(false))
                 else
                   IO.unit
-              val error = Execute.Error("", "", List(e.message))
+              val error = Execute.Error(e.name, e.message, e.stackTrace)
               extra *>
                 message
                   .publish(Execute.errorType, error)

diff --git a/modules/shared/test-kit/src/main/scala/almond/testkit/ClientStreams.scala b/modules/shared/test-kit/src/main/scala/almond/testkit/ClientStreams.scala
@@ -231,6 +231,23 @@ final case class ClientStreams(
       .flatten
       .toList
 
+  def executeResultErrors: Seq[Execute.Error] =
+    generatedMessages
+      .iterator
+      .collect {
+        case Left((Channel.Publish, m)) if m.header.msg_type == Execute.errorType.messageType =>
+          m.decodeAs[Execute.Error] match {
+            case Left(_) => Nil
+            case Right(m) =>
+              m.content match {
+                case e: Execute.Error => Seq(e)
+                case _                => Nil
+              }
+          }
+      }
+      .flatten
+      .toList
+
   def inspectRepliesHtml: Seq[String] =
     generatedMessages
       .iterator