From ac6e0237be33ee85d021496f0115076bd97e2759 Mon Sep 17 00:00:00 2001 From: Stephen Amar Date: Tue, 17 Mar 2026 23:54:14 +0000 Subject: [PATCH] Fix stack trace frame names to show enclosing function instead of callee Stack traces previously labeled each frame with the name of the function being called at that position, rather than the function containing that position. This made traces confusing, especially for cross-file max-stack errors where the frame name didn't match the file shown. Rework formatError to scan frames from outermost to innermost, computing the enclosing function 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. --- sjsonnet/src/sjsonnet/Error.scala | 73 +++++++++++-------- .../recursive_thunk.jsonnet.golden | 10 +-- .../go_test_suite/std.flatmap5.jsonnet.golden | 3 +- .../go_test_suite/tailstrict2.jsonnet.golden | 3 +- .../error.max_stack_cross_file.jsonnet | 9 +++ .../error.max_stack_cross_file.jsonnet.golden | 5 ++ ...rror.max_stack_cross_file_helper.libsonnet | 8 ++ .../test_suite/error.01.jsonnet.golden | 5 +- ....recursive_function_nonterm.jsonnet.golden | 4 +- .../error.tailstrict_stack.jsonnet.golden | 5 +- 10 files changed, 78 insertions(+), 47 deletions(-) create mode 100644 sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file_helper.libsonnet diff --git a/sjsonnet/src/sjsonnet/Error.scala b/sjsonnet/src/sjsonnet/Error.scala index ed25709b..3a7a12b7 100644 --- a/sjsonnet/src/sjsonnet/Error.scala +++ b/sjsonnet/src/sjsonnet/Error.scala @@ -105,40 +105,51 @@ object Error { val allFrames = err.stack.reverse val namedFrames = allFrames.filter(_.exprErrorString != null) - val frames = if (namedFrames.nonEmpty) namedFrames else allFrames + val rawFrames = if (namedFrames.nonEmpty) namedFrames else allFrames + + // Compute enclosing function scope for each frame by scanning from + // outermost to innermost. Each frame's callee name tells us what function + // was entered; builtin calls (std.*) are transparent and don't change scope. + val scopeNames = new Array[String](rawFrames.length) + var currentScope: String = rootName + var si = rawFrames.length - 1 + while (si >= 0) { + scopeNames(si) = currentScope + val frameName = rawFrames(si).exprErrorString + if (frameName != null && !isBuiltinName(frameName)) { + currentScope = frameName + } + si -= 1 + } + if (rawFrames.nonEmpty) { + scopeNames(0) = Option(rawFrames(0).exprErrorString).getOrElse(rootName) + } + + // Deduplicate consecutive same-scope frames, keeping the innermost (first) + val frames = new java.util.ArrayList[(Frame, String)](rawFrames.length) + si = 0 + while (si < rawFrames.length) { + val name = scopeNames(si) + if (frames.isEmpty || frames.get(frames.size - 1)._2 != name) { + frames.add((rawFrames(si), name)) + } + si += 1 + } var msg = err.getMessage var frameStart = 0 - while ( - frameStart < frames.length - 1 && - (frames(frameStart).collapseIntoMessage || - frames(frameStart).pos == frames(frameStart + 1).pos) - ) { - val name = Option(frames(frameStart).exprErrorString).getOrElse(rootName) - msg = "[" + name + "] " + msg - frameStart += 1 + // Collapse innermost frame name into message when flagged, hiding it from "at" lines + if (frames.size > 0 && rawFrames(0).collapseIntoMessage) { + msg = "[" + frames.get(0)._2 + "] " + msg + frameStart = 1 } - if (frameStart < frames.length && frames(frameStart).collapseIntoMessage) { - val f = frames(frameStart) - val name = Option(f.exprErrorString).getOrElse(rootName) - msg = "[" + name + "] " + msg - sb.append(msg) - appendFrame(sb, f, rootName) - } else { - sb.append(msg) - var i = frameStart - while (i < frames.length) { - val f = frames(i) - val name = - if (i == frames.length - 1) { - val n = f.exprErrorString - if (n == null || n == rootName) rootName else n - } else - Option(f.exprErrorString).getOrElse(rootName) - appendFrame(sb, f, name) - i += 1 - } + sb.append(msg) + var i = frameStart + while (i < frames.size) { + val (f, name) = frames.get(i) + appendFrame(sb, f, name) + i += 1 } if (err.getCause != null) { sb.append("\nCaused by: ") @@ -166,6 +177,10 @@ object Error { } } + private def isBuiltinName(name: String): Boolean = + name.startsWith("std.") || name == "default" || + name == "object comprehension" || name == "array comprehension" + private def appendFrame(sb: StringBuilder, f: Frame, name: String): Unit = { sb.append("\n at [").append(name).append("]") if (f.pos != null) { diff --git a/sjsonnet/test/resources/go_test_suite/recursive_thunk.jsonnet.golden b/sjsonnet/test/resources/go_test_suite/recursive_thunk.jsonnet.golden index b1914463..b87a9611 100644 --- a/sjsonnet/test/resources/go_test_suite/recursive_thunk.jsonnet.golden +++ b/sjsonnet/test/resources/go_test_suite/recursive_thunk.jsonnet.golden @@ -1,9 +1,7 @@ sjsonnet.Error: xxx at [bar].(recursive_thunk.jsonnet:1:35) - at [foo].(recursive_thunk.jsonnet:2:23) - at [bar].(recursive_thunk.jsonnet:2:19) - at [foo].(recursive_thunk.jsonnet:2:23) - at [bar].(recursive_thunk.jsonnet:2:19) - at [foo].(recursive_thunk.jsonnet:3:4) - at [].(recursive_thunk.jsonnet:1:7) + at [foo].(recursive_thunk.jsonnet:2:19) + at [bar].(recursive_thunk.jsonnet:2:23) + at [foo].(recursive_thunk.jsonnet:2:19) + at [].(recursive_thunk.jsonnet:3:4) diff --git a/sjsonnet/test/resources/go_test_suite/std.flatmap5.jsonnet.golden b/sjsonnet/test/resources/go_test_suite/std.flatmap5.jsonnet.golden index 03fe722a..3bce1359 100644 --- a/sjsonnet/test/resources/go_test_suite/std.flatmap5.jsonnet.golden +++ b/sjsonnet/test/resources/go_test_suite/std.flatmap5.jsonnet.golden @@ -1,5 +1,4 @@ sjsonnet.Error: a at [std.flatMap].(std.flatmap5.jsonnet:1:21) - at [std.type].(std.flatmap5.jsonnet:2:9) - at [].(std.flatmap5.jsonnet:1:7) + at [].(std.flatmap5.jsonnet:2:9) diff --git a/sjsonnet/test/resources/go_test_suite/tailstrict2.jsonnet.golden b/sjsonnet/test/resources/go_test_suite/tailstrict2.jsonnet.golden index 442a7ace..205ca551 100644 --- a/sjsonnet/test/resources/go_test_suite/tailstrict2.jsonnet.golden +++ b/sjsonnet/test/resources/go_test_suite/tailstrict2.jsonnet.golden @@ -1,5 +1,4 @@ sjsonnet.Error: xxx at [e].(tailstrict2.jsonnet:1:13) - at [anonymous].(tailstrict2.jsonnet:2:19) - at [].(tailstrict2.jsonnet:1:7) + at [].(tailstrict2.jsonnet:2:19) diff --git a/sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file.jsonnet b/sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file.jsonnet new file mode 100644 index 00000000..fe302204 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file.jsonnet @@ -0,0 +1,9 @@ +local b = import "error.max_stack_cross_file_helper.libsonnet"; + +local m = function() ( + local foo = { bar: "hi" }; + local bar = b.method(foo); + std.toString(foo) + " " + bar +); + +m() diff --git a/sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file.jsonnet.golden new file mode 100644 index 00000000..55c6f218 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file.jsonnet.golden @@ -0,0 +1,5 @@ +sjsonnet.Error: [search] Max stack frames exceeded. + at [method].(error.max_stack_cross_file_helper.libsonnet:5:37) + at [m].(error.max_stack_cross_file.jsonnet:5:23) + at [].(error.max_stack_cross_file.jsonnet:9:2) + diff --git a/sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file_helper.libsonnet b/sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file_helper.libsonnet new file mode 100644 index 00000000..bdf42239 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.max_stack_cross_file_helper.libsonnet @@ -0,0 +1,8 @@ +local search(s) = search(s); + +{ + method: function(arg) ( + assert std.objectHas(arg, search("foo")); + arg + ) +} diff --git a/sjsonnet/test/resources/test_suite/error.01.jsonnet.golden b/sjsonnet/test/resources/test_suite/error.01.jsonnet.golden index 1994fc82..d9212c46 100644 --- a/sjsonnet/test/resources/test_suite/error.01.jsonnet.golden +++ b/sjsonnet/test/resources/test_suite/error.01.jsonnet.golden @@ -1,6 +1,5 @@ sjsonnet.Error: foo at [bananas].(error.01.jsonnet:17:29) - at [oranges].(error.01.jsonnet:19:35) - at [apples].(error.01.jsonnet:20:7) - at [].(error.01.jsonnet:17:7) + at [apples].(error.01.jsonnet:19:35) + at [].(error.01.jsonnet:20:7) diff --git a/sjsonnet/test/resources/test_suite/error.recursive_function_nonterm.jsonnet.golden b/sjsonnet/test/resources/test_suite/error.recursive_function_nonterm.jsonnet.golden index 9ef9fa4c..9adc8002 100644 --- a/sjsonnet/test/resources/test_suite/error.recursive_function_nonterm.jsonnet.golden +++ b/sjsonnet/test/resources/test_suite/error.recursive_function_nonterm.jsonnet.golden @@ -1,3 +1,3 @@ sjsonnet.Error: [f] Max stack frames exceeded. - at [f].(error.recursive_function_nonterm.jsonnet:20:2) - at [].(error.recursive_function_nonterm.jsonnet:17:7) + at [].(error.recursive_function_nonterm.jsonnet:20:2) + diff --git a/sjsonnet/test/resources/test_suite/error.tailstrict_stack.jsonnet.golden b/sjsonnet/test/resources/test_suite/error.tailstrict_stack.jsonnet.golden index 9272e080..ac76e16b 100644 --- a/sjsonnet/test/resources/test_suite/error.tailstrict_stack.jsonnet.golden +++ b/sjsonnet/test/resources/test_suite/error.tailstrict_stack.jsonnet.golden @@ -1,6 +1,5 @@ sjsonnet.Error: n is 0 at [sum_from_one_to_n].(error.tailstrict_stack.jsonnet:10:9) - at [sum_from_one_to_n].(error.tailstrict_stack.jsonnet:14:37) - at [foo].(error.tailstrict_stack.jsonnet:19:18) - at [].(error.tailstrict_stack.jsonnet:1:7) + at [foo].(error.tailstrict_stack.jsonnet:14:37) + at [].(error.tailstrict_stack.jsonnet:19:18)