Improve error messages in Materializer (#238)

databricks · Dec 14, 2024 · 123e17f · 123e17f
1 parent 680b1a8
commit 123e17f
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 6 deletions.
diff --git a/sjsonnet/src/sjsonnet/Materializer.scala b/sjsonnet/src/sjsonnet/Materializer.scala
@@ -37,7 +37,8 @@ abstract class Materializer {
           )
           if(sort) {
             if(prevKey != null && k.compareTo(prevKey) <= 0)
-              Error.fail(s"""Internal error: Unexpected key "$k" after "$prevKey" in sorted object materialization""")
+              Error.fail(
+                s"""Internal error: Unexpected key "$k" after "$prevKey" in sorted object materialization""", v.pos)
             prevKey = k
           }
         }
@@ -56,14 +57,14 @@ abstract class Materializer {
       case Val.True(pos) => storePos(pos); visitor.visitTrue(-1)
       case Val.False(pos) => storePos(pos); visitor.visitFalse(-1)
       case Val.Null(pos) => storePos(pos); visitor.visitNull(-1)
-      case _: Val.Func =>
-        Error.fail("Couldn't manifest function as JSON")
+      case s: Val.Func =>
+        Error.fail("Couldn't manifest function with params [" + s.params.names.mkString(",") + "]", v.pos)
       case _ =>
-        Error.fail("Unknown value type " + v.prettyName)
+        Error.fail("Unknown value type", v.pos)
     }
   } catch {
     case _: StackOverflowError =>
-      Error.fail("Stackoverflow while materializing, possibly due to recursive value")
+      Error.fail("Stackoverflow while materializing, possibly due to recursive value", v.pos)
   }
 
   def reverse(pos: Position, v: ujson.Value): Val = v match{

diff --git a/sjsonnet/test/src-jvm/sjsonnet/ErrorTestsJvmOnly.scala b/sjsonnet/test/src-jvm/sjsonnet/ErrorTestsJvmOnly.scala
@@ -23,14 +23,17 @@ object ErrorTestsJvmOnly extends TestSuite {
   val tests = Tests{
     test("array_recursive_manifest") - check(
       """sjsonnet.Error: Stackoverflow while materializing, possibly due to recursive value
+        |    at .(sjsonnet/test/resources/test_suite/error.array_recursive_manifest.jsonnet:17:12)
         |""".stripMargin
     )
     test("obj_recursive") - check(
       """sjsonnet.Error: Stackoverflow while materializing, possibly due to recursive value
+        |    at .(sjsonnet/test/resources/test_suite/error.obj_recursive.jsonnet:17:3)
         |""".stripMargin
     )
     test("obj_recursive_manifest") - check(
       """sjsonnet.Error: Stackoverflow while materializing, possibly due to recursive value
+        |    at .(sjsonnet/test/resources/test_suite/error.obj_recursive_manifest.jsonnet:17:3)
         |""".stripMargin
     )
   }

diff --git a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala
@@ -30,7 +30,7 @@ object EvaluatorTests extends TestSuite{
       eval("local f(x) = function() true; f(42)") ==> ujson.True
       eval("local f(x) = function() true; f(42) == true") ==> ujson.False
       eval("local f(x) = function() true; f(42)() == true") ==> ujson.True
-      assert(evalErr("{foo: function() true}").startsWith("sjsonnet.Error: Couldn't manifest function as JSON"))
+      assert(evalErr("{foo: function() true}").startsWith("sjsonnet.Error: Couldn't manifest function with params"))
       eval("{foo: (function() true)()}") ==> ujson.Obj{"foo" -> ujson.True}
     }
     test("members") {