LibJS+Embedders: Unify stack trace format for uncaught errors

Previously these handlers duplicated code and used formats that
were different from the one Error.prototype.stack uses.

Now they use the same Error::stack_string function, which accepts
a new parameter for compacting stack traces with repeating frames.

Meta/Lagom/Wasm/js_repl.cpp

@@ -165,31 +165,7 @@ static ErrorOr<bool> parse_and_run(JS::Realm& realm, StringView source, StringVi
 
         if (!thrown_value.is_object() || !is<JS::Error>(thrown_value.as_object()))
             return;
-        auto& traceback = static_cast<JS::Error const&>(thrown_value.as_object()).traceback();
+        displayln("{}", static_cast<JS::Error const&>(thrown_value.as_object()).stack_string(JS::CompactTraceback::Yes));
-        if (traceback.size() > 1) {
-            unsigned repetitions = 0;
-            for (size_t i = 0; i < traceback.size(); ++i) {
-                auto& traceback_frame = traceback[i];
-                if (i + 1 < traceback.size()) {
-                    auto& next_traceback_frame = traceback[i + 1];
-                    if (next_traceback_frame.function_name == traceback_frame.function_name) {
-                        repetitions++;
-                        continue;
-                    }
-                }
-                if (repetitions > 4) {
-                    // If more than 5 (1 + >4) consecutive function calls with the same name, print
-                    // the name only once and show the number of repetitions instead. This prevents
-                    // printing ridiculously large call stacks of recursive functions.
-                    displayln(" -> {}", traceback_frame.function_name);
-                    displayln(" {} more calls", repetitions);
-                } else {
-                    for (size_t j = 0; j < repetitions + 1; ++j)
-                        displayln(" -> {}", traceback_frame.function_name);
-                }
-                repetitions = 0;
-            }
-        }
     };
 
     if (!result.is_error())

Userland/Applications/Spreadsheet/Spreadsheet.cpp

@@ -72,11 +72,7 @@ Sheet::Sheet(Workbook& workbook)
                 if (thrown_value.is_object() && is<JS::Error>(thrown_value.as_object())) {
                     auto& error = static_cast<JS::Error const&>(thrown_value.as_object());
                     warnln(" with message '{}'", error.get_without_side_effects(vm.names.message));
-                    for (auto& traceback_frame : error.traceback()) {
+                    dbgln("{}", error.stack_string(JS::CompactTraceback::Yes));
-                        auto& function_name = traceback_frame.function_name;
-                        auto& source_range = traceback_frame.source_range();
-                        dbgln("  {} at {}:{}:{}", function_name, source_range.filename(), source_range.start.line, source_range.start.column);
-                    }
                 } else {
                     warnln();
                 }

Userland/Libraries/LibJS/Runtime/Error.cpp

@@ -79,15 +79,11 @@ void Error::populate_stack()
     m_traceback.ensure_capacity(vm.execution_context_stack().size());
     for (ssize_t i = vm.execution_context_stack().size() - 1; i >= 0; i--) {
         auto context = vm.execution_context_stack()[i];
-        auto function_name = context->function_name;
-        if (function_name.is_empty())
-            function_name = "<unknown>"sv;
-
         UnrealizedSourceRange range = {};
         if (context->instruction_stream_iterator.has_value())
             range = context->instruction_stream_iterator->source_range();
         TracebackFrame frame {
-            .function_name = move(function_name),
+            .function_name = context->function_name,
             .source_range_storage = range,
         };
 
@@ -95,29 +91,63 @@ void Error::populate_stack()
     }
 }
 
-String Error::stack_string() const
+String Error::stack_string(CompactTraceback compact) const
 {
     StringBuilder stack_string_builder;
 
     // Note: We roughly follow V8's formatting
-    // Note: The error's name and message get prepended by ErrorPrototype::stack
+    auto append_frame = [&](TracebackFrame const& frame) {
-    // Note: We don't want to capture the global execution context, so we omit the last frame
-    // FIXME: We generate a stack-frame for the Errors constructor, other engines do not
-    for (size_t i = 0; i < m_traceback.size() - 1; ++i) {
-        auto const& frame = m_traceback[i];
         auto function_name = frame.function_name;
         auto source_range = frame.source_range();
         // Note: Since we don't know whether we have a valid SourceRange here we just check for some default values.
         if (!source_range.filename().is_empty() || source_range.start.offset != 0 || source_range.end.offset != 0) {
 
-            if (function_name == "<unknown>"sv)
+            if (function_name.is_empty())
                 stack_string_builder.appendff("    at {}:{}:{}\n", source_range.filename(), source_range.start.line, source_range.start.column);
             else
                 stack_string_builder.appendff("    at {} ({}:{}:{})\n", function_name, source_range.filename(), source_range.start.line, source_range.start.column);
         } else {
             stack_string_builder.appendff("    at {}\n", function_name.is_empty() ? "<unknown>"sv : function_name.view());
         }
+    };
+
+    auto is_same_frame = [](TracebackFrame const& a, TracebackFrame const& b) {
+        if (a.function_name.is_empty() && b.function_name.is_empty()) {
+            auto source_range_a = a.source_range();
+            auto source_range_b = b.source_range();
+            return source_range_a.filename() == source_range_b.filename() && source_range_a.start.line == source_range_b.start.line;
+        }
+        return a.function_name == b.function_name;
+    };
+
+    // Note: We don't want to capture the global execution context, so we omit the last frame
+    // Note: The error's name and message get prepended by ErrorPrototype::stack
+    // FIXME: We generate a stack-frame for the Errors constructor, other engines do not
+    unsigned repetitions = 0;
+    size_t used_frames = m_traceback.size() - 1;
+    for (size_t i = 0; i < used_frames; ++i) {
+        auto const& frame = m_traceback[i];
+        if (compact == CompactTraceback::Yes && i + 1 < used_frames) {
+            auto const& next_traceback_frame = m_traceback[i + 1];
+            if (is_same_frame(frame, next_traceback_frame)) {
+                repetitions++;
+                continue;
+            }
+        }
+        if (repetitions > 4) {
+            // If more than 5 (1 + >4) consecutive function calls with the same name, print
+            // the name only once and show the number of repetitions instead. This prevents
+            // printing ridiculously large call stacks of recursive functions.
+            append_frame(frame);
+            stack_string_builder.appendff("    {} more calls\n", repetitions);
+        } else {
+            for (size_t j = 0; j < repetitions + 1; j++)
+                append_frame(frame);
+        }
+        repetitions = 0;
     }
+    for (size_t j = 0; j < repetitions; j++)
+        append_frame(m_traceback[used_frames - 1]);
 
     return MUST(stack_string_builder.to_string());
 }

Userland/Libraries/LibJS/Runtime/Error.h

@@ -22,6 +22,11 @@ struct TracebackFrame {
     mutable Variant<SourceRange, UnrealizedSourceRange> source_range_storage;
 };
 
+enum CompactTraceback {
+    No,
+    Yes,
+};
+
 class Error : public Object {
     JS_OBJECT(Error, Object);
 
@@ -32,7 +37,7 @@ public:
 
     virtual ~Error() override = default;
 
-    [[nodiscard]] String stack_string() const;
+    [[nodiscard]] String stack_string(CompactTraceback compact = CompactTraceback::No) const;
 
     ThrowCompletionOr<void> install_error_cause(Value options);
 

Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp

@@ -31,11 +31,7 @@ void report_exception_to_console(JS::Value value, JS::Realm& realm, ErrorInPromi
         }
         if (is<JS::Error>(object)) {
             auto const& error_value = static_cast<JS::Error const&>(object);
-            for (auto& traceback_frame : error_value.traceback()) {
+            dbgln("{}", error_value.stack_string(JS::CompactTraceback::Yes));
-                auto& function_name = traceback_frame.function_name;
-                auto& source_range = traceback_frame.source_range();
-                dbgln("  {} at {}:{}:{}", function_name, source_range.filename(), source_range.start.line, source_range.start.column);
-            }
             console.report_exception(error_value, error_in_promise == ErrorInPromise::Yes);
 
             return;

Userland/Utilities/js.cpp

@@ -251,36 +251,7 @@ static ErrorOr<bool> parse_and_run(JS::Realm& realm, StringView source, StringVi
 
         if (!thrown_value.is_object() || !is<JS::Error>(thrown_value.as_object()))
             return {};
-        auto const& traceback = static_cast<JS::Error const&>(thrown_value.as_object()).traceback();
+        warnln("{}", static_cast<JS::Error const&>(thrown_value.as_object()).stack_string(JS::CompactTraceback::Yes));
-        if (traceback.size() > 1) {
-            unsigned repetitions = 0;
-            for (size_t i = 0; i < traceback.size(); ++i) {
-                auto const& traceback_frame = traceback[i];
-                if (i + 1 < traceback.size()) {
-                    auto const& next_traceback_frame = traceback[i + 1];
-                    if (next_traceback_frame.function_name == traceback_frame.function_name) {
-                        repetitions++;
-                        continue;
-                    }
-                }
-                if (repetitions > 4) {
-                    // If more than 5 (1 + >4) consecutive function calls with the same name, print
-                    // the name only once and show the number of repetitions instead. This prevents
-                    // printing ridiculously large call stacks of recursive functions.
-                    warnln(" -> {}", traceback_frame.function_name);
-                    warnln(" {} more calls", repetitions);
-                } else {
-                    for (size_t j = 0; j < repetitions + 1; ++j) {
-                        warnln(" -> {} ({}:{},{})",
-                            traceback_frame.function_name,
-                            traceback_frame.source_range().code->filename(),
-                            traceback_frame.source_range().start.line,
-                            traceback_frame.source_range().start.column);
-                    }
-                }
-                repetitions = 0;
-            }
-        }
         return {};
     };