From 62c7608a2575beba8427887c008d122456109a8e Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sat, 24 Apr 2021 18:25:05 +0200 Subject: [PATCH] LibJS+LibWeb: Move exception logging and remove should_log_exceptions LibWeb is now responsible for logging unhandled exceptions itself, which means set_should_log_exceptions() is no longer used and can be removed. It turned out to be not the best option for web page exception logging, as we would have no indication regarding whether the exception was later handled of not. --- Userland/Libraries/LibJS/Runtime/VM.cpp | 25 ------------------- Userland/Libraries/LibJS/Runtime/VM.h | 4 --- .../LibWeb/Bindings/MainThreadVM.cpp | 4 +-- Userland/Libraries/LibWeb/DOM/Document.cpp | 25 +++++++++++++++++++ 4 files changed, 26 insertions(+), 32 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index c1a38a6def..84330f8e6e 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -272,31 +272,6 @@ Value VM::construct(Function& function, Function& new_target, Optional= 0; --i) { - const auto& source_range = m_call_stack[i]->current_node->source_range(); - auto function_name = m_call_stack[i]->function_name; - if (function_name.is_empty()) - function_name = ""; - dbgln(" {} at {}:{}:{}", function_name, source_range.filename, source_range.start.line, source_range.start.column); - } - } - set_exception(exception); unwind(ScopeType::Try); } diff --git a/Userland/Libraries/LibJS/Runtime/VM.h b/Userland/Libraries/LibJS/Runtime/VM.h index e9e8c8f823..78501e0b10 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.h +++ b/Userland/Libraries/LibJS/Runtime/VM.h @@ -54,9 +54,6 @@ public: static NonnullRefPtr create(); ~VM(); - bool should_log_exceptions() const { return m_should_log_exceptions; } - void set_should_log_exceptions(bool b) { m_should_log_exceptions = b; } - Heap& heap() { return m_heap; } const Heap& heap() const { return m_heap; } @@ -271,7 +268,6 @@ private: Shape* m_scope_object_shape { nullptr }; bool m_underscore_is_last_value { false }; - bool m_should_log_exceptions { false }; }; template<> diff --git a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp index fb01437707..44edb408d0 100644 --- a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp +++ b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp @@ -12,10 +12,8 @@ namespace Web::Bindings { JS::VM& main_thread_vm() { static RefPtr vm; - if (!vm) { + if (!vm) vm = JS::VM::create(); - vm->set_should_log_exceptions(true); - } return *vm; } diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index e0cffb8965..500927fd21 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -563,6 +563,31 @@ JS::Interpreter& Document::interpreter() vm.on_call_stack_emptied = [this] { auto& vm = m_interpreter->vm(); vm.run_queued_promise_jobs(); + // Note: This is not an exception check for the promise jobs, they will just leave any + // exception that already exists intact and never throw a new one (without cleaning it + // up, that is). Taking care of any previous unhandled exception just happens to be the + // very last thing we want to do, even after running promise jobs. + if (auto* exception = vm.exception()) { + auto value = exception->value(); + if (value.is_object()) { + auto& object = value.as_object(); + auto name = object.get_without_side_effects(vm.names.name).value_or(JS::js_undefined()); + auto message = object.get_without_side_effects(vm.names.message).value_or(JS::js_undefined()); + if (name.is_accessor() || name.is_native_property() || message.is_accessor() || message.is_native_property()) { + // The result is not going to be useful, let's just print the value. This affects DOMExceptions, for example. + dbgln("Unhandled JavaScript exception: {}", value); + } else { + dbgln("Unhandled JavaScript exception: [{}] {}", name, message); + } + } else { + dbgln("Unhandled JavaScript exception: {}", value); + } + for (auto& traceback_frame : exception->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); + } + } }; m_interpreter = JS::Interpreter::create(vm, *m_window); }