From 40aad77ab1bd15b76ef0a58664400ac0f34430b6 Mon Sep 17 00:00:00 2001 From: Pavel Date: Sat, 8 Oct 2022 19:38:32 +0400 Subject: WebContent+LibWeb+LibJS: Report exceptions to the JS console Print exceptions passed to `HTML::report_exception` in the JS console Refactored `ExceptionReporter`: in order to report exception now you need to pass the relevant realm in it. For passed `JS::Value` we now create `JS::Error` object to print value as the error message. --- Userland/Libraries/LibJS/Console.cpp | 6 +++ Userland/Libraries/LibJS/Console.h | 4 +- Userland/Libraries/LibJS/MarkupGenerator.cpp | 47 ++++++++++++++-------- Userland/Libraries/LibJS/MarkupGenerator.h | 6 ++- .../Libraries/LibWeb/Bindings/MainThreadVM.cpp | 6 +-- Userland/Libraries/LibWeb/DOM/Document.cpp | 2 +- Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp | 2 +- .../LibWeb/HTML/Scripting/ClassicScript.cpp | 2 +- .../LibWeb/HTML/Scripting/Environments.cpp | 2 +- .../LibWeb/HTML/Scripting/ExceptionReporter.cpp | 26 +++++++----- .../LibWeb/HTML/Scripting/ExceptionReporter.h | 8 ++-- Userland/Libraries/LibWeb/HTML/Window.cpp | 14 ++++--- .../WebContent/WebContentConsoleClient.cpp | 18 +++------ .../Services/WebContent/WebContentConsoleClient.h | 1 + 14 files changed, 86 insertions(+), 58 deletions(-) (limited to 'Userland') diff --git a/Userland/Libraries/LibJS/Console.cpp b/Userland/Libraries/LibJS/Console.cpp index 165723e7c6..2e6be717e9 100644 --- a/Userland/Libraries/LibJS/Console.cpp +++ b/Userland/Libraries/LibJS/Console.cpp @@ -453,6 +453,12 @@ void Console::output_debug_message([[maybe_unused]] LogLevel log_level, [[maybe_ #endif } +void Console::report_exception(JS::Error const& exception, bool in_promise) const +{ + if (m_client) + m_client->report_exception(exception, in_promise); +} + ThrowCompletionOr Console::value_vector_to_string(MarkedVector const& values) { auto& vm = realm().vm(); diff --git a/Userland/Libraries/LibJS/Console.h b/Userland/Libraries/LibJS/Console.h index 86dc68c60e..85a979485b 100644 --- a/Userland/Libraries/LibJS/Console.h +++ b/Userland/Libraries/LibJS/Console.h @@ -82,6 +82,7 @@ public: ThrowCompletionOr time_end(); void output_debug_message(LogLevel log_level, String output) const; + void report_exception(JS::Error const&, bool) const; private: ThrowCompletionOr value_vector_to_string(MarkedVector const&); @@ -108,7 +109,8 @@ public: ThrowCompletionOr> formatter(MarkedVector const& args); virtual ThrowCompletionOr printer(Console::LogLevel log_level, PrinterArguments) = 0; - virtual void add_css_style_to_current_message(StringView) {}; + virtual void add_css_style_to_current_message(StringView) { } + virtual void report_exception(JS::Error const&, bool) { } virtual void clear() = 0; virtual void end_group() = 0; diff --git a/Userland/Libraries/LibJS/MarkupGenerator.cpp b/Userland/Libraries/LibJS/MarkupGenerator.cpp index 995d72ce10..33afb5fe1f 100644 --- a/Userland/Libraries/LibJS/MarkupGenerator.cpp +++ b/Userland/Libraries/LibJS/MarkupGenerator.cpp @@ -36,11 +36,10 @@ String MarkupGenerator::html_from_value(Value value) return output_html.to_string(); } -String MarkupGenerator::html_from_error(Object& object) +String MarkupGenerator::html_from_error(Error const& object, bool in_promise) { StringBuilder output_html; - HashTable seen_objects; - error_to_html(object, output_html, seen_objects); + error_to_html(object, output_html, in_promise); return output_html.to_string(); } @@ -70,8 +69,6 @@ void MarkupGenerator::value_to_html(Value value, StringBuilder& output_html, Has return function_to_html(object, output_html, seen_objects); if (is(object)) return date_to_html(object, output_html, seen_objects); - if (is(object)) - return error_to_html(object, output_html, seen_objects); return object_to_html(object, output_html, seen_objects); } @@ -145,19 +142,35 @@ void MarkupGenerator::date_to_html(Object const& date, StringBuilder& html_outpu html_output.appendff("Date {}", to_date_string(static_cast(date).date_value())); } -void MarkupGenerator::error_to_html(Object const& object, StringBuilder& html_output, HashTable&) +void MarkupGenerator::trace_to_html(TracebackFrame const& traceback_frame, StringBuilder& html_output) { - auto& vm = object.vm(); - auto name = object.get_without_side_effects(vm.names.name).value_or(js_undefined()); - auto message = object.get_without_side_effects(vm.names.message).value_or(js_undefined()); - if (name.is_accessor() || message.is_accessor()) { - html_output.append(wrap_string_in_style(Value(&object).to_string_without_side_effects(), StyleType::Invalid)); - } else { - auto name_string = name.to_string_without_side_effects(); - auto message_string = message.to_string_without_side_effects(); - html_output.append(wrap_string_in_style(String::formatted("[{}]", name_string), StyleType::Invalid)); - if (!message_string.is_empty()) - html_output.appendff(": {}", escape_html_entities(message_string)); + auto function_name = escape_html_entities(traceback_frame.function_name); + auto [line, column, _] = traceback_frame.source_range.start; + auto get_filename_from_path = [&](StringView filename) -> StringView { + auto last_slash_index = filename.find_last('/'); + return last_slash_index.has_value() ? filename.substring_view(*last_slash_index + 1) : filename; + }; + auto filename = escape_html_entities(get_filename_from_path(traceback_frame.source_range.filename)); + auto trace = String::formatted("at {} ({}:{}:{})", function_name, filename, line, column); + + html_output.appendff("  {}
", trace); +} + +void MarkupGenerator::error_to_html(Error const& error, StringBuilder& html_output, bool in_promise) +{ + auto& vm = error.vm(); + auto name = error.get_without_side_effects(vm.names.name).value_or(js_undefined()); + auto message = error.get_without_side_effects(vm.names.message).value_or(js_undefined()); + auto name_string = name.to_string_without_side_effects(); + auto message_string = message.to_string_without_side_effects(); + auto uncaught_message = String::formatted("Uncaught {}[{}]: ", in_promise ? "(in promise) " : "", name_string); + + html_output.append(wrap_string_in_style(uncaught_message, StyleType::Invalid)); + html_output.appendff("{}
", message_string.is_empty() ? "\"\"" : escape_html_entities(message_string)); + + for (size_t i = 0; i < error.traceback().size() - min(error.traceback().size(), 3); i++) { + auto& traceback_frame = error.traceback().at(i); + trace_to_html(traceback_frame, html_output); } } diff --git a/Userland/Libraries/LibJS/MarkupGenerator.h b/Userland/Libraries/LibJS/MarkupGenerator.h index f8bc16849c..00ba5b7f02 100644 --- a/Userland/Libraries/LibJS/MarkupGenerator.h +++ b/Userland/Libraries/LibJS/MarkupGenerator.h @@ -9,6 +9,7 @@ #include #include #include +#include namespace JS { @@ -16,7 +17,7 @@ class MarkupGenerator { public: static String html_from_source(StringView); static String html_from_value(Value); - static String html_from_error(Object&); + static String html_from_error(Error const&, bool); private: enum class StyleType { @@ -37,7 +38,8 @@ private: static void object_to_html(Object const&, StringBuilder& output_html, HashTable&); static void function_to_html(Object const&, StringBuilder& output_html, HashTable&); static void date_to_html(Object const&, StringBuilder& output_html, HashTable&); - static void error_to_html(Object const&, StringBuilder& output_html, HashTable&); + static void error_to_html(Error const&, StringBuilder& output_html, bool in_promise); + static void trace_to_html(TracebackFrame const&, StringBuilder& output_html); static String style_from_style_type(StyleType); static StyleType style_type_for_token(Token); diff --git a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp index 861d912f96..388310fa86 100644 --- a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp +++ b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp @@ -202,7 +202,7 @@ JS::VM& main_thread_vm() // 6. If result is an abrupt completion, then report the exception given by result.[[Value]]. if (result.is_error()) - HTML::report_exception(result); + HTML::report_exception(result, finalization_registry.realm()); }); }; @@ -275,7 +275,7 @@ JS::VM& main_thread_vm() // 5. If result is an abrupt completion, then report the exception given by result.[[Value]]. if (result.is_error()) - HTML::report_exception(result); + HTML::report_exception(result, job_settings->realm()); }); }; @@ -437,7 +437,7 @@ void queue_mutation_observer_microtask(DOM::Document& document) auto result = WebIDL::invoke_callback(callback, mutation_observer.ptr(), wrapped_records, mutation_observer.ptr()); if (result.is_abrupt()) - HTML::report_exception(result); + HTML::report_exception(result, realm); } } diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 8376aa11d8..36d7a6dc9c 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -1139,7 +1139,7 @@ JS::Value Document::run_javascript(StringView source, StringView filename) if (result.is_error()) { // FIXME: I'm sure the spec could tell us something about error propagation here! - HTML::report_exception(result); + HTML::report_exception(result, realm()); return {}; } diff --git a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp index bb5708f22c..e5f3676c11 100644 --- a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp +++ b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp @@ -114,7 +114,7 @@ bool EventDispatcher::inner_invoke(Event& event, Vector) // This algorithm results in promise rejections being marked as handled or not handled. These concepts parallel handled and not handled script errors. // If a rejection is still not handled after this, then the rejection may be reported to a developer console. if (not_handled) - HTML::print_error_from_value(promise.result(), ErrorInPromise::Yes); + HTML::report_exception_to_console(promise.result(), realm(), ErrorInPromise::Yes); } }); } diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp index bea5fbc44b..cb2fa3edd9 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp +++ b/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp @@ -5,15 +5,19 @@ */ #include +#include +#include #include #include +#include #include namespace Web::HTML { -void print_error_from_value(JS::Value value, ErrorInPromise error_in_promise) +void report_exception_to_console(JS::Value value, JS::Realm& realm, ErrorInPromise error_in_promise) { - // FIXME: We should probably also report these exceptions to the JS console. + auto& console = realm.intrinsics().console_object()->console(); + if (value.is_object()) { auto& object = value.as_object(); auto& vm = object.vm(); @@ -27,24 +31,28 @@ void print_error_from_value(JS::Value value, ErrorInPromise error_in_promise) } if (is(object)) { auto const& error_value = static_cast(object); - for (auto const& traceback_frame : error_value.traceback()) { - auto const& function_name = traceback_frame.function_name; - auto const& source_range = traceback_frame.source_range; + for (auto& traceback_frame : error_value.traceback()) { + 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; } } else { - dbgln("\033[31;1mUnhandled JavaScript exception:\033[0m {}", value); + dbgln("\033[31;1mUnhandled JavaScript exception{}:\033[0m {}", error_in_promise == ErrorInPromise::Yes ? " (in promise)" : "", value); } + + console.report_exception(*JS::Error::create(realm, value.to_string_without_side_effects()), error_in_promise == ErrorInPromise::Yes); } // https://html.spec.whatwg.org/#report-the-exception -void report_exception(JS::Completion const& throw_completion) +void report_exception(JS::Completion const& throw_completion, JS::Realm& realm) { - // FIXME: This is just old code, and does not strictly follow the spec of report an exception. VERIFY(throw_completion.type() == JS::Completion::Type::Throw); VERIFY(throw_completion.value().has_value()); - print_error_from_value(*throw_completion.value(), ErrorInPromise::No); + report_exception_to_console(*throw_completion.value(), realm, ErrorInPromise::No); } } diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.h b/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.h index f7d7061772..29e4597711 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.h +++ b/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.h @@ -15,14 +15,14 @@ enum class ErrorInPromise { Yes, }; -void print_error_from_value(JS::Value, ErrorInPromise); -void report_exception(JS::Completion const&); +void report_exception_to_console(JS::Value, JS::Realm&, ErrorInPromise); +void report_exception(JS::Completion const&, JS::Realm&); template -inline void report_exception(JS::ThrowCompletionOr const& result) +inline void report_exception(JS::ThrowCompletionOr const& result, JS::Realm& realm) { VERIFY(result.is_throw_completion()); - report_exception(result.throw_completion()); + report_exception(result.throw_completion(), realm); } } diff --git a/Userland/Libraries/LibWeb/HTML/Window.cpp b/Userland/Libraries/LibWeb/HTML/Window.cpp index dd6af7e591..181116d7b5 100644 --- a/Userland/Libraries/LibWeb/HTML/Window.cpp +++ b/Userland/Libraries/LibWeb/HTML/Window.cpp @@ -201,7 +201,7 @@ i32 Window::run_timer_initialization_steps(TimerHandler handler, i32 timeout, JS // 2. If handler is a Function, then invoke handler given arguments with the callback this value set to thisArg. If this throws an exception, catch it, and report the exception. [&](JS::Handle callback) { if (auto result = WebIDL::invoke_callback(*callback, window.ptr(), arguments); result.is_error()) - HTML::report_exception(result); + HTML::report_exception(result, weak_window->realm()); }, // 3. Otherwise: [&](String const& source) { @@ -271,7 +271,7 @@ i32 Window::request_animation_frame_impl(WebIDL::CallbackType& js_callback) // and if an exception is thrown, report the exception. if (result.is_error()) - HTML::report_exception(result); + HTML::report_exception(result, realm()); }); } @@ -495,11 +495,15 @@ void Window::fire_a_page_transition_event(FlyString const& event_name, bool pers void Window::queue_microtask_impl(WebIDL::CallbackType& callback) { // The queueMicrotask(callback) method must queue a microtask to invoke callback, - HTML::queue_a_microtask(&associated_document(), [&callback]() mutable { + HTML::queue_a_microtask(&associated_document(), [weak_window = make_weak_ptr(), &callback]() mutable { + JS::GCPtr window = weak_window.ptr(); + if (!window) + return; + auto result = WebIDL::invoke_callback(callback, {}); // and if callback throws an exception, report the exception. if (result.is_error()) - HTML::report_exception(result); + HTML::report_exception(result, window->realm()); }); } @@ -655,7 +659,7 @@ void Window::invoke_idle_callbacks() // 3. Call callback with deadlineArg as its argument. If an uncaught runtime script error occurs, then report the exception. auto result = callback->invoke(deadline_arg); if (result.is_error()) - HTML::report_exception(result); + HTML::report_exception(result, realm()); // 4. If window's list of runnable idle callbacks is not empty, queue a task which performs the steps // in the invoke idle callbacks algorithm with getDeadline and window as a parameters and return from this algorithm HTML::queue_global_task(HTML::Task::Source::IdleTask, *this, [this]() mutable { diff --git a/Userland/Services/WebContent/WebContentConsoleClient.cpp b/Userland/Services/WebContent/WebContentConsoleClient.cpp index fcff369d79..938e448d41 100644 --- a/Userland/Services/WebContent/WebContentConsoleClient.cpp +++ b/Userland/Services/WebContent/WebContentConsoleClient.cpp @@ -49,23 +49,15 @@ void WebContentConsoleClient::handle_input(String const& js_source) auto result = script->run(); - StringBuilder output_html; - - if (result.is_abrupt()) { - output_html.append("Uncaught exception: "sv); - auto error = *result.release_error().value(); - if (error.is_object()) - output_html.append(JS::MarkupGenerator::html_from_error(error.as_object())); - else - output_html.append(JS::MarkupGenerator::html_from_value(error)); - print_html(output_html.string_view()); - return; - } - if (result.value().has_value()) print_html(JS::MarkupGenerator::html_from_value(*result.value())); } +void WebContentConsoleClient::report_exception(JS::Error const& exception, bool in_promise) +{ + print_html(JS::MarkupGenerator::html_from_error(exception, in_promise)); +} + void WebContentConsoleClient::print_html(String const& line) { m_message_log.append({ .type = ConsoleOutput::Type::HTML, .data = line }); diff --git a/Userland/Services/WebContent/WebContentConsoleClient.h b/Userland/Services/WebContent/WebContentConsoleClient.h index c234bb779d..c3c80c394b 100644 --- a/Userland/Services/WebContent/WebContentConsoleClient.h +++ b/Userland/Services/WebContent/WebContentConsoleClient.h @@ -22,6 +22,7 @@ public: void handle_input(String const& js_source); void send_messages(i32 start_index); + void report_exception(JS::Error const&, bool) override; private: virtual void clear() override; -- cgit v1.2.3