Skip to content

Commit 95d64e0

Browse files
Fix stack trace frame names to show enclosing function instead of callee (#652)
## Summary - Rework `Error.formatError` to label each stack frame with the **enclosing function** (the function whose body contains that position) instead of the **callee** (the function being called). This matches the convention used by Java/Python stack traces. - The fix scans frames outermost-to-innermost to compute the enclosing scope for each position. Builtin calls (`std.*`), comprehensions, and default parameter evaluations are transparent and don't create new scopes. Consecutive same-scope frames are deduplicated. - Add regression test for cross-file max-stack-frames error (`error.max_stack_cross_file`). ### Before ``` sjsonnet.Error: [search] Max stack frames exceeded. at [search].(b.jsonnet:5:37) at [std.objectHas].(b.jsonnet:5:25) at [method].(a.jsonnet:5:23) at [m].(a.jsonnet:9:2) at [<root>].(a.jsonnet:1:7) ``` ### After ``` sjsonnet.Error: [search] Max stack frames exceeded. at [method].(b.jsonnet:5:37) at [m].(a.jsonnet:5:23) at [<root>].(a.jsonnet:9:2) ``` Each `[name]` now reflects the function you are **in**, not the function you **called**. ## Test plan - [x] All existing JVM tests pass (140/140) - [x] Updated 6 golden files to match corrected trace format - [x] Added new regression test `error.max_stack_cross_file` covering cross-file recursive max-stack error
1 parent da1dd8d commit 95d64e0

10 files changed

Lines changed: 78 additions & 47 deletions

sjsonnet/src/sjsonnet/Error.scala

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -105,40 +105,51 @@ object Error {
105105

106106
val allFrames = err.stack.reverse
107107
val namedFrames = allFrames.filter(_.exprErrorString != null)
108-
val frames = if (namedFrames.nonEmpty) namedFrames else allFrames
108+
val rawFrames = if (namedFrames.nonEmpty) namedFrames else allFrames
109+
110+
// Compute enclosing function scope for each frame by scanning from
111+
// outermost to innermost. Each frame's callee name tells us what function
112+
// was entered; builtin calls (std.*) are transparent and don't change scope.
113+
val scopeNames = new Array[String](rawFrames.length)
114+
var currentScope: String = rootName
115+
var si = rawFrames.length - 1
116+
while (si >= 0) {
117+
scopeNames(si) = currentScope
118+
val frameName = rawFrames(si).exprErrorString
119+
if (frameName != null && !isBuiltinName(frameName)) {
120+
currentScope = frameName
121+
}
122+
si -= 1
123+
}
124+
if (rawFrames.nonEmpty) {
125+
scopeNames(0) = Option(rawFrames(0).exprErrorString).getOrElse(rootName)
126+
}
127+
128+
// Deduplicate consecutive same-scope frames, keeping the innermost (first)
129+
val frames = new java.util.ArrayList[(Frame, String)](rawFrames.length)
130+
si = 0
131+
while (si < rawFrames.length) {
132+
val name = scopeNames(si)
133+
if (frames.isEmpty || frames.get(frames.size - 1)._2 != name) {
134+
frames.add((rawFrames(si), name))
135+
}
136+
si += 1
137+
}
109138

110139
var msg = err.getMessage
111140
var frameStart = 0
112-
while (
113-
frameStart < frames.length - 1 &&
114-
(frames(frameStart).collapseIntoMessage ||
115-
frames(frameStart).pos == frames(frameStart + 1).pos)
116-
) {
117-
val name = Option(frames(frameStart).exprErrorString).getOrElse(rootName)
118-
msg = "[" + name + "] " + msg
119-
frameStart += 1
141+
// Collapse innermost frame name into message when flagged, hiding it from "at" lines
142+
if (frames.size > 0 && rawFrames(0).collapseIntoMessage) {
143+
msg = "[" + frames.get(0)._2 + "] " + msg
144+
frameStart = 1
120145
}
121146

122-
if (frameStart < frames.length && frames(frameStart).collapseIntoMessage) {
123-
val f = frames(frameStart)
124-
val name = Option(f.exprErrorString).getOrElse(rootName)
125-
msg = "[" + name + "] " + msg
126-
sb.append(msg)
127-
appendFrame(sb, f, rootName)
128-
} else {
129-
sb.append(msg)
130-
var i = frameStart
131-
while (i < frames.length) {
132-
val f = frames(i)
133-
val name =
134-
if (i == frames.length - 1) {
135-
val n = f.exprErrorString
136-
if (n == null || n == rootName) rootName else n
137-
} else
138-
Option(f.exprErrorString).getOrElse(rootName)
139-
appendFrame(sb, f, name)
140-
i += 1
141-
}
147+
sb.append(msg)
148+
var i = frameStart
149+
while (i < frames.size) {
150+
val (f, name) = frames.get(i)
151+
appendFrame(sb, f, name)
152+
i += 1
142153
}
143154
if (err.getCause != null) {
144155
sb.append("\nCaused by: ")
@@ -166,6 +177,10 @@ object Error {
166177
}
167178
}
168179

180+
private def isBuiltinName(name: String): Boolean =
181+
name.startsWith("std.") || name == "default" ||
182+
name == "object comprehension" || name == "array comprehension"
183+
169184
private def appendFrame(sb: StringBuilder, f: Frame, name: String): Unit = {
170185
sb.append("\n at [").append(name).append("]")
171186
if (f.pos != null) {
Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
sjsonnet.Error: xxx
22
at [bar].(recursive_thunk.jsonnet:1:35)
3-
at [foo].(recursive_thunk.jsonnet:2:23)
4-
at [bar].(recursive_thunk.jsonnet:2:19)
5-
at [foo].(recursive_thunk.jsonnet:2:23)
6-
at [bar].(recursive_thunk.jsonnet:2:19)
7-
at [foo].(recursive_thunk.jsonnet:3:4)
8-
at [<root>].(recursive_thunk.jsonnet:1:7)
3+
at [foo].(recursive_thunk.jsonnet:2:19)
4+
at [bar].(recursive_thunk.jsonnet:2:23)
5+
at [foo].(recursive_thunk.jsonnet:2:19)
6+
at [<root>].(recursive_thunk.jsonnet:3:4)
97

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
sjsonnet.Error: a
22
at [std.flatMap].(std.flatmap5.jsonnet:1:21)
3-
at [std.type].(std.flatmap5.jsonnet:2:9)
4-
at [<root>].(std.flatmap5.jsonnet:1:7)
3+
at [<root>].(std.flatmap5.jsonnet:2:9)
54

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
sjsonnet.Error: xxx
22
at [e].(tailstrict2.jsonnet:1:13)
3-
at [anonymous].(tailstrict2.jsonnet:2:19)
4-
at [<root>].(tailstrict2.jsonnet:1:7)
3+
at [<root>].(tailstrict2.jsonnet:2:19)
54

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
local b = import "error.max_stack_cross_file_helper.libsonnet";
2+
3+
local m = function() (
4+
local foo = { bar: "hi" };
5+
local bar = b.method(foo);
6+
std.toString(foo) + " " + bar
7+
);
8+
9+
m()
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
sjsonnet.Error: [search] Max stack frames exceeded.
2+
at [method].(error.max_stack_cross_file_helper.libsonnet:5:37)
3+
at [m].(error.max_stack_cross_file.jsonnet:5:23)
4+
at [<root>].(error.max_stack_cross_file.jsonnet:9:2)
5+
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
local search(s) = search(s);
2+
3+
{
4+
method: function(arg) (
5+
assert std.objectHas(arg, search("foo"));
6+
arg
7+
)
8+
}
Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
sjsonnet.Error: foo
22
at [bananas].(error.01.jsonnet:17:29)
3-
at [oranges].(error.01.jsonnet:19:35)
4-
at [apples].(error.01.jsonnet:20:7)
5-
at [<root>].(error.01.jsonnet:17:7)
3+
at [apples].(error.01.jsonnet:19:35)
4+
at [<root>].(error.01.jsonnet:20:7)
65

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
sjsonnet.Error: [f] Max stack frames exceeded.
2-
at [f].(error.recursive_function_nonterm.jsonnet:20:2)
3-
at [<root>].(error.recursive_function_nonterm.jsonnet:17:7)
2+
at [<root>].(error.recursive_function_nonterm.jsonnet:20:2)
3+
Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
sjsonnet.Error: n is 0
22
at [sum_from_one_to_n].(error.tailstrict_stack.jsonnet:10:9)
3-
at [sum_from_one_to_n].(error.tailstrict_stack.jsonnet:14:37)
4-
at [foo].(error.tailstrict_stack.jsonnet:19:18)
5-
at [<root>].(error.tailstrict_stack.jsonnet:1:7)
3+
at [foo].(error.tailstrict_stack.jsonnet:14:37)
4+
at [<root>].(error.tailstrict_stack.jsonnet:19:18)
65

0 commit comments

Comments
 (0)