Skip to content

Commit ad4edb1

Browse files
Refactor stack traces: named frames, root frame, and builtin collapsing (#635)
## Summary Rework error stack traces to produce cleaner, more informative output using the existing Java stack trace mechanism (no custom call stack). This is an alternative to #633 that avoids the performance regression by resolving names at parse time and filtering frames during formatting rather than maintaining a separate call stack. ### Key changes - **Named frames from AST**: `callTargetName` extracts function names from `Apply` AST nodes at the expression level (`[foo]`, `[std.assertEqual]`, `[anonymous]`), replacing opaque node type names like `[Apply0]`, `[Apply3]`. - **Frame filtering**: Only `Apply*` and `ApplyBuiltin*` expression frames are kept in stack traces, filtering out intermediate AST node types that add noise. - **Root frame**: Every stack trace now includes a `[<root>]` outermost frame pointing to the top-level expression, injected by `Interpreter.evaluateImpl`. - **Builtin error collapsing**: When an error originates from inside a builtin's Scala code (e.g., type checking, missing arguments), the builtin name is collapsed into the error message as a prefix rather than shown as a separate frame: - `sjsonnet.Error: [std.manifestTomlEx] Wrong parameter type: expected Object, got array` - `sjsonnet.Error: [foo] Too many args, has 2 parameter(s)` - **Same-position collapsing**: Adjacent frames at the same file position are collapsed to avoid redundant lines. - **Caused-by chain**: `formatError` now includes `Caused by:` output for wrapped exceptions (e.g., native function panics). - **Removed redundant prefixes**: `"Function "` prefix removed from error messages since the frame name now provides that context. ### Example outputs Builtin error (collapsed into message): ``` sjsonnet.Error: [std.assertEqual] assertEqual failed: {"x":1} != {"x":2} at [<root>].(assert_equal4.jsonnet:1:16) ``` Function parameter error (collapsed): ``` sjsonnet.Error: [anonymous] parameter x not bound in call at [<root>].(bad_function_call.jsonnet:1:16) ``` Error inside function body (full trace): ``` sjsonnet.Error: xxx at [foo].(error_in_method.jsonnet:1:23) at [<root>].(error_in_method.jsonnet:1:7) ``` Native function panic (with caused-by): ``` sjsonnet.Error: [std.nativePanic] Internal Error at [<root>].(native_panic.jsonnet:1:26) Caused by: java.lang.RuntimeException: native function panic ``` ## Test plan - [x] All JVM tests pass (140/140) - [x] Golden files regenerated for test_suite, go_test_suite, new_test_suite (340 files) - [x] Unit test expectations updated (EvaluatorTests, StdFlatMapTests, StdWithKeyFTests, Std0150FunctionsTests) - [x] Native/extvar golden files manually verified (require special test harness args) - [x] JS BaseFileTests updated for new format
1 parent d9b631d commit ad4edb1

361 files changed

Lines changed: 4860 additions & 4699 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ The assembly JAR is at `out/sjsonnet/jvm/3.3.7/assembly.dest/out.jar`. Run it wi
2525

2626
## Formatting
2727

28+
We follow the Databricks [Scala style guide](https://raw.githubusercontent.com/databricks/scala-style-guide/refs/heads/master/README.md).
29+
2830
Scala sources are formatted with [scalafmt](https://scalameta.org/scalafmt/) (config in `.scalafmt.conf`). The `SjsonnetCrossModule` and `bench` modules mix in `ScalafmtModule`.
2931

3032
```bash

sjsonnet/src/sjsonnet/Error.scala

Lines changed: 113 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,41 @@ import scala.util.control.NonFatal
77
* An exception that can keep track of the Sjsonnet call-stack while it is propagating upwards. This
88
* helps provide good error messages with line numbers pointing towards user code.
99
*/
10-
class Error(msg: String, stack: List[Error.Frame] = Nil, underlying: Option[Throwable] = None)
10+
class Error(msg: String, val stack: List[Error.Frame] = Nil, underlying: Option[Throwable] = None)
1111
extends Exception(msg, underlying.orNull) {
1212

1313
setStackTrace(stack.reverseIterator.map(_.ste).toArray)
1414

1515
override def fillInStackTrace: Throwable = this
1616

1717
def addFrame(pos: Position, expr: Expr = null)(implicit ev: EvalErrorScope): Error = {
18-
if (stack.isEmpty || alwaysAddPos(expr)) {
19-
val exprErrorString = if (expr == null) null else expr.exprErrorString
20-
addFrameString(pos, exprErrorString)
18+
if (stack.isEmpty) {
19+
val name = if (expr != null && isApplyOrBuiltin(expr)) expr.exprErrorString else null
20+
if (name != null)
21+
copy(stack = new Error.Frame(pos, name, collapseIntoMessage = true) :: Nil)
22+
else
23+
addFrameString(pos, name)
24+
} else if (expr != null && isApplyOrBuiltin(expr)) {
25+
addFrameString(pos, expr.exprErrorString)
2126
} else this
2227
}
2328

2429
def addFrameString(pos: Position, exprErrorString: String)(implicit ev: EvalErrorScope): Error = {
25-
val newFrame = new Error.Frame(pos, exprErrorString)
2630
stack match {
31+
case s :: ss if s.exprErrorString == null && exprErrorString != null =>
32+
val collapse = s.pos == pos
33+
copy(stack = new Error.Frame(s.pos, exprErrorString, collapse) :: ss)
2734
case s :: ss if s.pos == pos =>
28-
if (s.exprErrorString == null && exprErrorString != null) copy(stack = newFrame :: ss)
35+
if (s.exprErrorString == null && exprErrorString != null)
36+
copy(stack = new Error.Frame(pos, exprErrorString, collapseIntoMessage = true) :: ss)
2937
else this
30-
case _ => copy(stack = newFrame :: stack)
38+
case _ => copy(stack = new Error.Frame(pos, exprErrorString) :: stack)
3139
}
3240
}
3341

42+
def forceAddFrame(pos: Position, name: String)(implicit ev: EvalErrorScope): Error =
43+
copy(stack = new Error.Frame(pos, name) :: stack)
44+
3445
def asSeenFrom(ev: EvalErrorScope): Error =
3546
copy(stack = stack.map(_.asSeenFrom(ev)))
3647

@@ -40,15 +51,20 @@ class Error(msg: String, stack: List[Error.Frame] = Nil, underlying: Option[Thro
4051
underlying: Option[Throwable] = underlying) =
4152
new Error(msg, stack, underlying)
4253

43-
private def alwaysAddPos(expr: Expr): Boolean = expr match {
44-
case _: Expr.LocalExpr | _: Expr.Arr | _: Expr.ObjExtend | _: Expr.ObjBody | _: Expr.IfElse =>
45-
false
46-
case _ => true
54+
private def isApplyOrBuiltin(expr: Expr): Boolean = expr match {
55+
case _: Expr.Apply | _: Expr.Apply0 | _: Expr.Apply1 | _: Expr.Apply2 | _: Expr.Apply3 |
56+
_: Expr.ApplyBuiltin | _: Expr.ApplyBuiltin0 | _: Expr.ApplyBuiltin1 |
57+
_: Expr.ApplyBuiltin2 | _: Expr.ApplyBuiltin3 | _: Expr.ApplyBuiltin4 =>
58+
true
59+
case _ => false
4760
}
4861
}
4962

5063
object Error {
51-
final class Frame(val pos: Position, val exprErrorString: String)(implicit ev: EvalErrorScope) {
64+
final class Frame(
65+
val pos: Position,
66+
val exprErrorString: String,
67+
val collapseIntoMessage: Boolean = false)(implicit ev: EvalErrorScope) {
5268
val ste: StackTraceElement = {
5369
val cl = if (exprErrorString == null) "" else s"[$exprErrorString]"
5470
val (frameFile, frameLine) = ev.prettyIndex(pos) match {
@@ -59,7 +75,8 @@ object Error {
5975
}
6076

6177
def asSeenFrom(ev: EvalErrorScope): Frame =
62-
if (ev eq this.ev) this else new Frame(pos, exprErrorString)(ev)
78+
if (ev eq this.ev) this
79+
else new Frame(pos, exprErrorString, collapseIntoMessage)(ev)
6380
}
6481

6582
def withStackFrame[T](expr: Expr)(implicit
@@ -70,29 +87,97 @@ object Error {
7087
}
7188

7289
def fail(msg: String, expr: Expr)(implicit ev: EvalErrorScope): Nothing =
73-
fail(msg, expr.pos, expr.exprErrorString)
90+
fail(msg, expr.pos)
7491

7592
def fail(msg: String, pos: Position, cl: String = null)(implicit ev: EvalErrorScope): Nothing =
7693
throw new Error(msg, new Frame(pos, cl) :: Nil, None)
7794

7895
def fail(msg: String): Nothing =
7996
throw new Error(msg)
8097

81-
def formatError(e: Throwable): String = {
82-
val s = new StringWriter()
83-
val p = new PrintWriter(s)
84-
try {
85-
e.printStackTrace(p)
86-
s.toString.replace("\t", " ")
87-
} finally {
88-
p.close()
98+
private val rootName = Util.wrapInLessThanGreaterThan("root")
99+
100+
def formatError(e: Throwable): String = e match {
101+
case err: Error if err.stack.nonEmpty =>
102+
val sb = new StringBuilder
103+
sb.append(err.getClass.getName).append(": ")
104+
105+
val allFrames = err.stack.reverse
106+
val namedFrames = allFrames.filter(_.exprErrorString != null)
107+
val frames = if (namedFrames.nonEmpty) namedFrames else allFrames
108+
109+
var msg = err.getMessage
110+
var frameStart = 0
111+
while (
112+
frameStart < frames.length - 1 &&
113+
(frames(frameStart).collapseIntoMessage ||
114+
frames(frameStart).pos == frames(frameStart + 1).pos)
115+
) {
116+
val name = Option(frames(frameStart).exprErrorString).getOrElse(rootName)
117+
msg = "[" + name + "] " + msg
118+
frameStart += 1
119+
}
120+
121+
if (frameStart < frames.length && frames(frameStart).collapseIntoMessage) {
122+
val f = frames(frameStart)
123+
val name = Option(f.exprErrorString).getOrElse(rootName)
124+
msg = "[" + name + "] " + msg
125+
sb.append(msg)
126+
appendFrame(sb, f, rootName)
127+
} else {
128+
sb.append(msg)
129+
var i = frameStart
130+
while (i < frames.length) {
131+
val f = frames(i)
132+
val name =
133+
if (i == frames.length - 1) {
134+
val n = f.exprErrorString
135+
if (n == null || n == rootName) rootName else n
136+
} else
137+
Option(f.exprErrorString).getOrElse(rootName)
138+
appendFrame(sb, f, name)
139+
i += 1
140+
}
141+
}
142+
if (err.getCause != null) {
143+
sb.append("\nCaused by: ")
144+
val s = new StringWriter()
145+
val p = new PrintWriter(s)
146+
try {
147+
err.getCause.printStackTrace(p)
148+
sb.append(s.toString.replace("\t", " "))
149+
} finally {
150+
p.close()
151+
}
152+
} else {
153+
sb.append('\n')
154+
}
155+
sb.toString
156+
157+
case _ =>
158+
val s = new StringWriter()
159+
val p = new PrintWriter(s)
160+
try {
161+
e.printStackTrace(p)
162+
s.toString.replace("\t", " ")
163+
} finally {
164+
p.close()
165+
}
166+
}
167+
168+
private def appendFrame(sb: StringBuilder, f: Frame, name: String): Unit = {
169+
sb.append("\n at [").append(name).append("]")
170+
if (f.pos != null) {
171+
sb.append(".(")
172+
sb.append(f.ste.getFileName).append(':').append(f.ste.getLineNumber)
173+
sb.append(')')
89174
}
90175
}
91176
}
92177

93178
class ParseError(
94179
msg: String,
95-
stack: List[Error.Frame] = Nil,
180+
override val stack: List[Error.Frame] = Nil,
96181
underlying: Option[Throwable] = None,
97182
val offset: Int = -1)
98183
extends Error(msg, stack, underlying) {
@@ -104,7 +189,10 @@ class ParseError(
104189
new ParseError(msg, stack, underlying, offset)
105190
}
106191

107-
class StaticError(msg: String, stack: List[Error.Frame] = Nil, underlying: Option[Throwable] = None)
192+
class StaticError(
193+
msg: String,
194+
override val stack: List[Error.Frame] = Nil,
195+
underlying: Option[Throwable] = None)
108196
extends Error(msg, stack, underlying) {
109197

110198
override protected def copy(
@@ -116,7 +204,7 @@ class StaticError(msg: String, stack: List[Error.Frame] = Nil, underlying: Optio
116204

117205
object StaticError {
118206
def fail(msg: String, expr: Expr)(implicit ev: EvalErrorScope): Nothing =
119-
throw new StaticError(msg, new Error.Frame(expr.pos, expr.exprErrorString) :: Nil, None)
207+
throw new StaticError(msg, new Error.Frame(expr.pos, null) :: Nil, None)
120208
}
121209

122210
trait EvalErrorScope {

sjsonnet/src/sjsonnet/Evaluator.scala

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -826,12 +826,11 @@ class Evaluator(
826826
val a = asserts(i)
827827
if (!visitExpr(a.value)(newScope).isInstanceOf[Val.True]) {
828828
a.msg match {
829-
case null => Error.fail("Assertion failed", a.value.pos, "Assert")
829+
case null => Error.fail("Assertion failed", a.value.pos)
830830
case msg =>
831831
Error.fail(
832832
"Assertion failed: " + visitExpr(msg)(newScope).cast[Val.Str].str,
833-
a.value.pos,
834-
"Assert"
833+
a.value.pos
835834
)
836835
}
837836
}

sjsonnet/src/sjsonnet/Expr.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ trait TailstrictableExpr extends Expr {
4545
}
4646

4747
object Expr {
48+
private[sjsonnet] def callTargetName(value: Expr): String = value match {
49+
case ValidId(_, name, _) => name
50+
case Id(_, name) => name
51+
case Select(_, _, name) => name
52+
case f: Val.Builtin => f.qualifiedName
53+
case f: Val.Func =>
54+
val n = f.functionName
55+
if (n != null) n else "anonymous"
56+
case _ => "anonymous"
57+
}
58+
4859
private final def arrStr(a: Array[?]): String = {
4960
if (a == null) "null" else a.mkString("[", ", ", "]")
5061
}
@@ -212,18 +223,22 @@ object Expr {
212223
tailstrict: Boolean)
213224
extends TailstrictableExpr {
214225
final override private[sjsonnet] def tag = ExprTags.Apply
226+
override def exprErrorString: String = Expr.callTargetName(value)
215227
}
216228
final case class Apply0(pos: Position, value: Expr, tailstrict: Boolean)
217229
extends TailstrictableExpr {
218230
final override private[sjsonnet] def tag = ExprTags.Apply0
231+
override def exprErrorString: String = Expr.callTargetName(value)
219232
}
220233
final case class Apply1(pos: Position, value: Expr, a1: Expr, tailstrict: Boolean)
221234
extends TailstrictableExpr {
222235
final override private[sjsonnet] def tag = ExprTags.Apply1
236+
override def exprErrorString: String = Expr.callTargetName(value)
223237
}
224238
final case class Apply2(pos: Position, value: Expr, a1: Expr, a2: Expr, tailstrict: Boolean)
225239
extends TailstrictableExpr {
226240
final override private[sjsonnet] def tag = ExprTags.Apply2
241+
override def exprErrorString: String = Expr.callTargetName(value)
227242
}
228243
final case class Apply3(
229244
pos: Position,
@@ -234,6 +249,7 @@ object Expr {
234249
tailstrict: Boolean)
235250
extends TailstrictableExpr {
236251
final override private[sjsonnet] def tag = ExprTags.Apply3
252+
override def exprErrorString: String = Expr.callTargetName(value)
237253
}
238254
final case class ApplyBuiltin(
239255
pos: Position,

sjsonnet/src/sjsonnet/Interpreter.scala

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ class Interpreter(
180180
(for {
181181
v <- evaluate(txt, path)
182182
r <- materialize(v, visitor)
183-
} yield r).left.map(Error.formatError)
183+
} yield r).left.map(e => Error.formatError(ensureRootFrame(e)))
184184

185185
private def handleException[T](f: => T): Either[Error, T] = {
186186
try Right(f)
@@ -198,11 +198,41 @@ class Interpreter(
198198
result
199199
}
200200

201+
private val rootName = Util.wrapInLessThanGreaterThan("root")
202+
203+
@volatile private var lastTopLevelPos: Position = _
204+
205+
private def addRootFrame(e: Error, expr: Expr): Error = {
206+
implicit val ev: EvalErrorScope = evaluator
207+
e.forceAddFrame(expr.pos, rootName)
208+
}
209+
210+
private def ensureRootFrame(e: Error): Error = {
211+
if (e.stack.exists(_.exprErrorString == rootName)) e
212+
else {
213+
implicit val ev: EvalErrorScope = evaluator
214+
val pos = lastTopLevelPos
215+
if (pos == null) e
216+
else e.forceAddFrame(pos, rootName)
217+
}
218+
}
219+
201220
private def evaluateImpl(txt: String, path: Path): Either[Error, Val] = {
202221
val resolvedImport = StaticResolvedFile(txt)
203222
resolver.cache(path) = resolvedImport
204223
resolver.parse(path, resolvedImport)(evaluator) flatMap { case (expr, _) =>
205-
handleException(evaluator.visitExpr(expr)(ValScope.empty)) flatMap {
224+
lastTopLevelPos = expr.pos
225+
handleException {
226+
try evaluator.visitExpr(expr)(ValScope.empty)
227+
catch {
228+
case e: Error => throw addRootFrame(e, expr)
229+
case NonFatal(e) =>
230+
throw addRootFrame(
231+
new Error("Internal Error", Nil, Some(e)),
232+
expr
233+
)
234+
}
235+
} flatMap {
206236
case f: Val.Func =>
207237
val defaults2 = f.params.defaultExprs.clone()
208238
val tlaExpressions = collection.mutable.Set.empty[Expr]
@@ -226,7 +256,23 @@ class Interpreter(
226256
)
227257
}
228258
}
229-
handleException(out.apply0(f.pos)(evaluator, TailstrictModeDisabled))
259+
handleException {
260+
implicit val ev: EvalErrorScope = evaluator
261+
val funcName = {
262+
val n = f.functionName
263+
if (n != null) n else "anonymous"
264+
}
265+
try out.apply0(f.pos)(evaluator, TailstrictModeDisabled)
266+
catch {
267+
case e: Error =>
268+
throw addRootFrame(e.addFrameString(f.pos, funcName), expr)
269+
case NonFatal(e) =>
270+
throw addRootFrame(
271+
new Error("Internal Error", Nil, Some(e)).addFrameString(f.pos, funcName),
272+
expr
273+
)
274+
}
275+
}
230276
case x => Right(x)
231277
}
232278
}

0 commit comments

Comments
 (0)