diff options
author | davidot <davidot@serenityos.org> | 2022-02-07 15:12:41 +0100 |
---|---|---|
committer | Linus Groh <mail@linusgroh.de> | 2022-02-08 09:12:42 +0000 |
commit | 9264f9d24e10c3ca8d522450bc9152e69cbd6cda (patch) | |
tree | 68ad59cc9fb5328c8925e6f790c2da805dadbbe0 | |
parent | 4ef1e8f226caff2a0d9002cb5e30fbd7bc99b7b3 (diff) | |
download | serenity-9264f9d24e10c3ca8d522450bc9152e69cbd6cda.zip |
LibJS+Everywhere: Remove VM::exception() and most related functions
This commit removes all exception related code:
Remove VM::exception(), VM::throw_exception() etc. Any leftover
throw_exception calls are moved to throw_completion.
The one method left is clear_exception() which is now a no-op. Most of
these calls are just to clear whatever exception might have been thrown
when handling a Completion. So to have a cleaner commit this will be
removed in a next commit.
It also removes the actual Exception and TemporaryClearException classes
since these are no longer used.
In any spot where the exception was actually used an attempt was made to
preserve that behavior. However since it is no longer tracked by the VM
we cannot access exceptions which were thrown in previous calls.
There are two such cases which might have different behavior:
- In Web::DOM::Document::interpreter() the on_call_stack_emptied hook
used to print any uncaught exception but this is now no longer
possible as the VM does not store uncaught exceptions.
- In js the code used to be interruptable by throwing an exception on
the VM. This is no longer possible but was already somewhat fragile
before as you could happen to throw an exception just before a VERIFY.
35 files changed, 145 insertions, 349 deletions
diff --git a/Tests/LibJS/test-bytecode-js.cpp b/Tests/LibJS/test-bytecode-js.cpp index 5c21d25fd8..d6f0c8792d 100644 --- a/Tests/LibJS/test-bytecode-js.cpp +++ b/Tests/LibJS/test-bytecode-js.cpp @@ -25,8 +25,7 @@ #define EXPECT_NO_EXCEPTION(executable) \ auto executable = JS::Bytecode::Generator::generate(program); \ auto result = bytecode_interpreter.run(*executable); \ - EXPECT(!result.is_error()); \ - EXPECT(!vm->exception()); + EXPECT(!result.is_error()); #define EXPECT_NO_EXCEPTION_WITH_OPTIMIZATIONS(executable) \ auto& passes = JS::Bytecode::Interpreter::optimization_pipeline(); \ @@ -34,8 +33,7 @@ \ auto result_with_optimizations = bytecode_interpreter.run(*executable); \ \ - EXPECT(!result_with_optimizations.is_error()); \ - EXPECT(!vm->exception()) + EXPECT(!result_with_optimizations.is_error()); #define EXPECT_NO_EXCEPTION_ALL(source) \ SETUP_AND_PARSE(source) \ @@ -117,7 +115,6 @@ TEST_CASE(loading_multiple_files) auto executable = JS::Bytecode::Generator::generate(test_file_program); auto result = bytecode_interpreter.run(*executable); EXPECT(!result.is_error()); - EXPECT(!vm->exception()); } } diff --git a/Tests/LibWeb/test-web.cpp b/Tests/LibWeb/test-web.cpp index fd21598bbb..d778cd1bc3 100644 --- a/Tests/LibWeb/test-web.cpp +++ b/Tests/LibWeb/test-web.cpp @@ -22,8 +22,8 @@ TEST_ROOT("Userland/Libraries/LibWeb/Tests"); RefPtr<Web::InProcessWebView> g_page_view; RefPtr<GUI::Application> g_app; Optional<URL> next_page_to_load; -Vector<Function<void(JS::Object&)>> after_initial_load_hooks; -Vector<Function<void(JS::Object&)>> before_initial_load_hooks; +Vector<Function<JS::ThrowCompletionOr<void>(JS::Object&)>> after_initial_load_hooks; +Vector<Function<JS::ThrowCompletionOr<void>(JS::Object&)>> before_initial_load_hooks; TESTJS_MAIN_HOOK() { @@ -62,8 +62,9 @@ TESTJS_GLOBAL_FUNCTION(after_initial_page_load, afterInitialPageLoad) return vm.throw_completion<JS::TypeError>(global_object, JS::ErrorType::NotAnObjectOfType, "Function"); } - after_initial_load_hooks.append([fn = JS::make_handle(&function.as_function()), &global_object](auto& page_object) { - [[maybe_unused]] auto unused = JS::call(global_object, const_cast<JS::FunctionObject&>(*fn.cell()), JS::js_undefined(), &page_object); + after_initial_load_hooks.append([fn = JS::make_handle(&function.as_function()), &global_object](auto& page_object) -> JS::ThrowCompletionOr<void> { + TRY(JS::call(global_object, const_cast<JS::FunctionObject&>(*fn.cell()), JS::js_undefined(), &page_object)); + return {}; }); return JS::js_undefined(); } @@ -76,8 +77,9 @@ TESTJS_GLOBAL_FUNCTION(before_initial_page_load, beforeInitialPageLoad) return vm.throw_completion<JS::TypeError>(global_object, JS::ErrorType::NotAnObjectOfType, "Function"); } - before_initial_load_hooks.append([fn = JS::make_handle(&function.as_function()), &global_object](auto& page_object) { - [[maybe_unused]] auto unused = JS::call(global_object, const_cast<JS::FunctionObject&>(*fn.cell()), JS::js_undefined(), &page_object); + before_initial_load_hooks.append([fn = JS::make_handle(&function.as_function()), &global_object](auto& page_object) -> JS::ThrowCompletionOr<void> { + TRY(JS::call(global_object, const_cast<JS::FunctionObject&>(*fn.cell()), JS::js_undefined(), &page_object)); + return {}; }); return JS::js_undefined(); } @@ -89,12 +91,14 @@ TESTJS_GLOBAL_FUNCTION(wait_for_page_to_load, waitForPageToLoad) // Run the "before" hooks for (auto& entry : before_initial_load_hooks) - entry(document->interpreter().global_object()); + TRY(entry(document->interpreter().global_object())); // Set the load hook Web::LoadRequest request; request.set_url(next_page_to_load.value()); + JS::ThrowCompletionOr<void> result = {}; + auto& loader = Web::ResourceLoader::the(); loader.load_sync( request, @@ -103,22 +107,23 @@ TESTJS_GLOBAL_FUNCTION(wait_for_page_to_load, waitForPageToLoad) // Now parse the HTML page. parser.run(next_page_to_load.value()); g_page_view->set_document(&parser.document()); - if (vm.exception()) { - // FIXME: Should we do something about this? the document itself threw unhandled exceptions... - vm.clear_exception(); - } + // Note: Unhandled exceptions are just dropped here. // Run the "after" hooks for (auto& entry : after_initial_load_hooks) { - entry(document->interpreter().global_object()); - if (vm.exception()) + auto ran_or_error = entry(document->interpreter().global_object()); + if (ran_or_error.is_error()) { + result = ran_or_error.release_error(); break; + } } }, [&](auto&, auto) { dbgln("Load of resource {} failed", next_page_to_load.value()); - vm.throw_exception<JS::TypeError>(global_object, "Resource load failed"); + result = vm.template throw_completion<JS::TypeError>(global_object, "Resource load failed"); }); + if (result.is_error()) + return result.release_error(); return JS::js_undefined(); } diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 4bdb39771b..da4899613f 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -456,7 +456,6 @@ Completion SuperCall::execute(Interpreter& interpreter, GlobalObject& global_obj // 3. Let func be ! GetSuperConstructor(). auto* func = get_super_constructor(interpreter.vm()); - VERIFY(!vm.exception()); // 4. Let argList be ? ArgumentListEvaluation of Arguments. MarkedValueList arg_list(vm.heap()); @@ -3686,7 +3685,6 @@ Completion TryStatement::execute(Interpreter& interpreter, GlobalObject& global_ // TryStatement : try Block Catch // TryStatement : try Block Catch Finally if (m_handler) { - vm.clear_exception(); // 2. If B.[[Type]] is throw, let C be CatchClauseEvaluation of Catch with argument B.[[Value]]. if (block_result.type() == Completion::Type::Throw) result = catch_clause_evaluation(*block_result.value()); @@ -3702,12 +3700,6 @@ Completion TryStatement::execute(Interpreter& interpreter, GlobalObject& global_ // TryStatement : try Block Finally // TryStatement : try Block Catch Finally if (m_finalizer) { - // NOTE: Temporary until VM::exception() is removed - // Keep, if any, and then clear the current exception so we can - // execute() the finalizer without an exception in our way. - auto* previous_exception = vm.exception(); - vm.clear_exception(); - // 4. Let F be the result of evaluating Finally. auto finalizer_result = m_finalizer->execute(interpreter, global_object); @@ -3715,12 +3707,6 @@ Completion TryStatement::execute(Interpreter& interpreter, GlobalObject& global_ if (finalizer_result.type() == Completion::Type::Normal) finalizer_result = move(result); - // NOTE: Temporary until VM::exception() is removed - // If we previously had an exception and we're carrying over - // the catch block completion, restore it. - if (finalizer_result.type() == Completion::Type::Normal && previous_exception) - vm.set_exception(*previous_exception); - // 6. Return Completion(UpdateEmpty(F, undefined)). return finalizer_result.update_empty(js_undefined()); } @@ -3748,8 +3734,6 @@ Completion ThrowStatement::execute(Interpreter& interpreter, GlobalObject& globa auto value = TRY(m_argument->execute(interpreter, global_object)).release_value(); // 3. Return ThrowCompletion(exprValue). - // TODO: Remove this once we get rid of VM::exception() - interpreter.vm().throw_exception(global_object, value); return throw_completion(value); } diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index 6b6c286ece..f4cfc5d5e3 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -80,13 +80,6 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& e while (!pc.at_end()) { auto& instruction = *pc; auto ran_or_error = instruction.execute(*this); - if (vm().exception()) { - if (!ran_or_error.is_error()) { - // FIXME: Until exception is removed use this to make sure we always get the error if there is one. - ran_or_error = throw_completion(vm().exception()->value()); - } - vm().clear_exception(); - } if (ran_or_error.is_error()) { auto exception_value = *ran_or_error.throw_completion().value(); m_saved_exception = make_handle(exception_value); diff --git a/Userland/Libraries/LibJS/CMakeLists.txt b/Userland/Libraries/LibJS/CMakeLists.txt index efc496e64e..ce9e02a252 100644 --- a/Userland/Libraries/LibJS/CMakeLists.txt +++ b/Userland/Libraries/LibJS/CMakeLists.txt @@ -72,7 +72,6 @@ set(SOURCES Runtime/ErrorConstructor.cpp Runtime/ErrorPrototype.cpp Runtime/ErrorTypes.cpp - Runtime/Exception.cpp Runtime/FinalizationRegistry.cpp Runtime/FinalizationRegistryConstructor.cpp Runtime/FinalizationRegistryPrototype.cpp diff --git a/Userland/Libraries/LibJS/CyclicModule.cpp b/Userland/Libraries/LibJS/CyclicModule.cpp index 3414103e80..bac58e22e8 100644 --- a/Userland/Libraries/LibJS/CyclicModule.cpp +++ b/Userland/Libraries/LibJS/CyclicModule.cpp @@ -207,8 +207,6 @@ ThrowCompletionOr<Promise*> CyclicModule::evaluate(VM& vm) // 8. Let result be InnerModuleEvaluation(module, stack, 0). auto result = inner_module_evaluation(vm, stack, 0); - VERIFY(!vm.exception()); - // 9. If result is an abrupt completion, then if (result.is_throw_completion()) { VERIFY(!m_evaluation_error.is_error()); @@ -238,8 +236,6 @@ ThrowCompletionOr<Promise*> CyclicModule::evaluate(VM& vm) // d. Perform ! Call(capability.[[Reject]], undefined, « result.[[Value]] »). MUST(call(global_object, m_top_level_capability->reject, js_undefined(), *result.throw_completion().value())); - - VERIFY(!vm.exception()); } // 10. Else, else { diff --git a/Userland/Libraries/LibJS/Forward.h b/Userland/Libraries/LibJS/Forward.h index 188fb0224b..ead0c0ebcb 100644 --- a/Userland/Libraries/LibJS/Forward.h +++ b/Userland/Libraries/LibJS/Forward.h @@ -147,7 +147,6 @@ class ECMAScriptFunctionObject; class Environment; class Error; class ErrorType; -class Exception; class Expression; class FunctionEnvironment; class FunctionNode; diff --git a/Userland/Libraries/LibJS/Interpreter.cpp b/Userland/Libraries/LibJS/Interpreter.cpp index 1e7bf35847..430cc4855d 100644 --- a/Userland/Libraries/LibJS/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Interpreter.cpp @@ -44,7 +44,6 @@ Interpreter::~Interpreter() ThrowCompletionOr<Value> Interpreter::run(Script& script_record) { auto& vm = this->vm(); - VERIFY(!vm.exception()); VM::InterpreterExecutionScope scope(*this); @@ -136,24 +135,15 @@ ThrowCompletionOr<Value> Interpreter::run(SourceTextModule& module) // To avoid work we use link_and_eval_module however that can already be // dangerous if the vm loaded other modules. auto& vm = this->vm(); - VERIFY(!vm.exception()); VM::InterpreterExecutionScope scope(*this); - auto evaluated_or_error = vm.link_and_eval_module({}, module); - // This error does not set vm.exception so we set that here for the stuff that needs it - if (evaluated_or_error.is_throw_completion()) { - auto* error = vm.heap().allocate<Exception>(global_object(), *(evaluated_or_error.throw_completion().value())); - vm.set_exception(*error); - return evaluated_or_error.throw_completion(); - } - VERIFY(!vm.exception()); + TRY(vm.link_and_eval_module({}, module)); vm.run_queued_promise_jobs(); vm.run_queued_finalization_registry_cleanup_jobs(); - VERIFY(!vm.exception()); return js_undefined(); } diff --git a/Userland/Libraries/LibJS/Interpreter.h b/Userland/Libraries/LibJS/Interpreter.h index ea838e9bec..c96ddbc65c 100644 --- a/Userland/Libraries/LibJS/Interpreter.h +++ b/Userland/Libraries/LibJS/Interpreter.h @@ -18,7 +18,6 @@ #include <LibJS/Runtime/Completion.h> #include <LibJS/Runtime/DeclarativeEnvironment.h> #include <LibJS/Runtime/ErrorTypes.h> -#include <LibJS/Runtime/Exception.h> #include <LibJS/Runtime/GlobalEnvironment.h> #include <LibJS/Runtime/GlobalObject.h> #include <LibJS/Runtime/MarkedValueList.h> @@ -116,7 +115,6 @@ public: ALWAYS_INLINE VM& vm() { return *m_vm; } ALWAYS_INLINE const VM& vm() const { return *m_vm; } ALWAYS_INLINE Heap& heap() { return vm().heap(); } - ALWAYS_INLINE Exception* exception() { return vm().exception(); } Environment* lexical_environment() { return vm().lexical_environment(); } diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp index e466b2bed2..a1f31d33cf 100644 --- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp @@ -964,7 +964,6 @@ Object* create_mapped_arguments_object(GlobalObject& global_object, FunctionObje // 8. Set obj.[[Delete]] as specified in 10.4.4.5. // 9. Set obj.[[Prototype]] to %Object.prototype%. auto* object = vm.heap().allocate<ArgumentsObject>(global_object, global_object, environment); - VERIFY(!vm.exception()); // 14. Let index be 0. // 15. Repeat, while index < len, diff --git a/Userland/Libraries/LibJS/Runtime/Array.h b/Userland/Libraries/LibJS/Runtime/Array.h index b16f809e0d..35cbf07561 100644 --- a/Userland/Libraries/LibJS/Runtime/Array.h +++ b/Userland/Libraries/LibJS/Runtime/Array.h @@ -26,13 +26,11 @@ public: template<typename T> static Array* create_from(GlobalObject& global_object, Span<T const> elements, Function<Value(T const&)> map_fn) { - auto& vm = global_object.vm(); auto values = MarkedValueList { global_object.heap() }; values.ensure_capacity(elements.size()); - for (auto const& element : elements) { + for (auto const& element : elements) values.append(map_fn(element)); - VERIFY(!vm.exception()); - } + return Array::create_from(global_object, values); } diff --git a/Userland/Libraries/LibJS/Runtime/Completion.cpp b/Userland/Libraries/LibJS/Runtime/Completion.cpp index d1187dec34..0376b5a481 100644 --- a/Userland/Libraries/LibJS/Runtime/Completion.cpp +++ b/Userland/Libraries/LibJS/Runtime/Completion.cpp @@ -115,10 +115,6 @@ ThrowCompletionOr<Value> await(GlobalObject& global_object, Value value) if (success.value()) return result; - // NOTE: This is temporary until we remove VM::exception(). It's required as callers of - // AwaitExpression still need to check for an exception rather than a completion - // type as long as ASTNode::execute() returns a plain Value. - vm.throw_exception(global_object, result); return throw_completion(result); } diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp index ffcdf335a6..524059518a 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp @@ -551,7 +551,6 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia }); }); - VERIFY(!vm.exception()); auto* private_environment = callee_context.private_environment; for (auto& declaration : functions_to_initialize) { auto* function = ECMAScriptFunctionObject::create(global_object(), declaration.name(), declaration.source_text(), declaration.body(), declaration.parameters(), declaration.function_length(), lex_environment, private_environment, declaration.kind(), declaration.is_strict_mode(), declaration.might_need_arguments_object(), declaration.contains_direct_call_to_eval()); @@ -731,7 +730,6 @@ void async_block_start(VM& vm, NonnullRefPtr<Statement> const& async_body, Promi VERIFY(result.type() == Completion::Type::Throw); // ii. Perform ! Call(promiseCapability.[[Reject]], undefined, « result.[[Value]] »). - vm.clear_exception(); MUST(call(global_object, promise_capability.reject, js_undefined(), *result.value())); } // g. Return. @@ -781,14 +779,11 @@ Completion ECMAScriptFunctionObject::ordinary_call_evaluate_body() m_bytecode_executable->dump(); } auto result_and_frame = bytecode_interpreter->run_and_return_frame(*m_bytecode_executable, nullptr); - if (auto* exception = vm.exception()) - return throw_completion(exception->value()); VERIFY(result_and_frame.frame != nullptr); - if (result_and_frame.value.is_error()) { - vm.throw_exception(bytecode_interpreter->global_object(), *result_and_frame.value.release_error().value()); - return throw_completion(vm.exception()->value()); - } + if (result_and_frame.value.is_error()) + return result_and_frame.value.release_error(); + auto result = result_and_frame.value.release_value(); // NOTE: Running the bytecode should eventually return a completion. diff --git a/Userland/Libraries/LibJS/Runtime/Exception.cpp b/Userland/Libraries/LibJS/Runtime/Exception.cpp deleted file mode 100644 index 39792bf8ff..0000000000 --- a/Userland/Libraries/LibJS/Runtime/Exception.cpp +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright (c) 2020, Andreas Kling <kling@serenityos.org> - * Copyright (c) 2021, Linus Groh <linusg@serenityos.org> - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include <AK/String.h> -#include <LibJS/AST.h> -#include <LibJS/Interpreter.h> -#include <LibJS/Runtime/Exception.h> -#include <LibJS/Runtime/VM.h> - -namespace JS { - -Exception::Exception(Value value) - : m_value(value) -{ - auto& vm = this->vm(); - 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 = "<anonymous>"; - m_traceback.empend( - move(function_name), - // We might not have an AST node associated with the execution context, e.g. in promise - // reaction jobs (which aren't called anywhere from the source code). - // They're not going to generate any _unhandled_ exceptions though, so a meaningless - // source range is fine. - context->current_node ? context->current_node->source_range() : SourceRange {}); - } -} - -void Exception::visit_edges(Visitor& visitor) -{ - Cell::visit_edges(visitor); - visitor.visit(m_value); -} - -} diff --git a/Userland/Libraries/LibJS/Runtime/Exception.h b/Userland/Libraries/LibJS/Runtime/Exception.h deleted file mode 100644 index d7e62e7d8d..0000000000 --- a/Userland/Libraries/LibJS/Runtime/Exception.h +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright (c) 2020, Andreas Kling <kling@serenityos.org> - * Copyright (c) 2021, Linus Groh <linusg@serenityos.org> - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include <AK/FlyString.h> -#include <AK/Vector.h> -#include <LibJS/Heap/Cell.h> -#include <LibJS/Runtime/Error.h> -#include <LibJS/Runtime/Value.h> -#include <LibJS/SourceRange.h> - -namespace JS { - -class Exception : public Cell { -public: - explicit Exception(Value); - virtual ~Exception() override = default; - - Value value() const { return m_value; } - const Vector<TracebackFrame, 32>& traceback() const { return m_traceback; } - -private: - virtual const char* class_name() const override { return "Exception"; } - virtual void visit_edges(Visitor&) override; - - Value m_value; - Vector<TracebackFrame, 32> m_traceback; -}; - -} diff --git a/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp b/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp index fb6d3819b9..78d35700cd 100644 --- a/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp @@ -104,8 +104,8 @@ ThrowCompletionOr<Value> GeneratorObject::next_impl(VM& vm, GlobalObject& global // Pretend that 'yield' returned the passed value, or threw if (value_to_throw.has_value()) { - vm.throw_exception(global_object, value_to_throw.release_value()); bytecode_interpreter->accumulator() = js_undefined(); + return throw_completion(value_to_throw.release_value()); } else { bytecode_interpreter->accumulator() = next_argument.value_or(js_undefined()); } diff --git a/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp b/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp index f0837a2d6a..a36b14c284 100644 --- a/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp @@ -11,7 +11,6 @@ #include <LibJS/Runtime/FunctionObject.h> #include <LibJS/Runtime/GlobalObject.h> #include <LibJS/Runtime/IteratorOperations.h> -#include <LibJS/Runtime/TemporaryClearException.h> namespace JS { @@ -141,10 +140,6 @@ static Completion iterator_close_impl(GlobalObject& global_object, Iterator cons // 2. Let iterator be iteratorRecord.[[Iterator]]. auto* iterator = iterator_record.iterator; - // The callers of iterator_close() are often in an exceptional state. - // Temporarily clear that exception for invocation(s) to Call. - TemporaryClearException clear_exception(vm); - // 3. Let innerResult be GetMethod(iterator, "return"). auto inner_result = ThrowCompletionOr<Value> { js_undefined() }; auto get_method_result = Value(iterator).get_method(global_object, vm.names.return_); diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index fc89950b51..49cce3dee8 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -18,7 +18,6 @@ #include <LibJS/Runtime/PropertyDescriptor.h> #include <LibJS/Runtime/ProxyObject.h> #include <LibJS/Runtime/Shape.h> -#include <LibJS/Runtime/TemporaryClearException.h> #include <LibJS/Runtime/Value.h> namespace JS { diff --git a/Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp b/Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp index 7dfccdc21b..16c18b6aaf 100644 --- a/Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp @@ -17,7 +17,6 @@ #include <LibJS/Runtime/PromiseConstructor.h> #include <LibJS/Runtime/PromiseReaction.h> #include <LibJS/Runtime/PromiseResolvingElementFunctions.h> -#include <LibJS/Runtime/TemporaryClearException.h> namespace JS { @@ -209,7 +208,6 @@ static ThrowCompletionOr<Value> perform_promise_any(GlobalObject& global_object, MUST(error->define_property_or_throw(vm.names.errors, { .value = errors_array, .writable = true, .enumerable = false, .configurable = true })); // 3. Return ThrowCompletion(error). - vm.throw_exception(global_object, error); return throw_completion(error); }, [&](PromiseValueList& errors, RemainingElements& remaining_elements_count, Value next_promise, size_t index) { @@ -304,14 +302,12 @@ ThrowCompletionOr<Object*> PromiseConstructor::construct(FunctionObject& new_tar auto [resolve_function, reject_function] = promise->create_resolving_functions(); // 9. Let completion be Call(executor, undefined, « resolvingFunctions.[[Resolve]], resolvingFunctions.[[Reject]] »). - (void)JS::call(global_object, executor.as_function(), js_undefined(), &resolve_function, &reject_function); + auto completion = JS::call(global_object, executor.as_function(), js_undefined(), &resolve_function, &reject_function); // 10. If completion is an abrupt completion, then - if (auto* exception = vm.exception()) { - vm.clear_exception(); - + if (completion.is_error()) { // a. Perform ? Call(resolvingFunctions.[[Reject]], undefined, « completion.[[Value]] »). - TRY(JS::call(global_object, reject_function, js_undefined(), exception->value())); + TRY(JS::call(global_object, reject_function, js_undefined(), *completion.release_error().value())); } // 11. Return promise. diff --git a/Userland/Libraries/LibJS/Runtime/PromiseJobs.cpp b/Userland/Libraries/LibJS/Runtime/PromiseJobs.cpp index ea21bd2b55..d1fbbae63f 100644 --- a/Userland/Libraries/LibJS/Runtime/PromiseJobs.cpp +++ b/Userland/Libraries/LibJS/Runtime/PromiseJobs.cpp @@ -70,7 +70,7 @@ ThrowCompletionOr<Value> PromiseReactionJob::call() // f. If promiseCapability is undefined, then if (!promise_capability.has_value()) { // i. Assert: handlerResult is not an abrupt completion. - VERIFY(!vm.exception()); + VERIFY(!handler_result.is_abrupt()); // ii. Return NormalCompletion(empty). dbgln_if(PROMISE_DEBUG, "[PromiseReactionJob @ {}]: Reaction has no PromiseCapability, returning empty value", this); diff --git a/Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp b/Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp index 0a1afff773..c504672ca5 100644 --- a/Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp @@ -138,9 +138,8 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::finally) auto* promise = TRY(promise_resolve(global_object, constructor, result)); // iv. Let thrower be ! CreateBuiltinFunction(throwReason, 0, "", « »). - auto* thrower = NativeFunction::create(global_object, "", [reason](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> { + auto* thrower = NativeFunction::create(global_object, "", [reason](auto&, auto&) -> ThrowCompletionOr<Value> { // 1. Return ThrowCompletion(reason). - vm.throw_exception(global_object, reason); return throw_completion(reason); }); diff --git a/Userland/Libraries/LibJS/Runtime/TemporaryClearException.h b/Userland/Libraries/LibJS/Runtime/TemporaryClearException.h deleted file mode 100644 index d4069a991c..0000000000 --- a/Userland/Libraries/LibJS/Runtime/TemporaryClearException.h +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) 2021, Linus Groh <linusg@serenityos.org> - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include <LibJS/Runtime/Exception.h> -#include <LibJS/Runtime/VM.h> - -namespace JS { - -class TemporaryClearException { -public: - explicit TemporaryClearException(VM& vm) - : m_vm(vm) - , m_previous_exception(vm.exception()) - { - m_vm.clear_exception(); - } - - ~TemporaryClearException() - { - if (m_previous_exception) - m_vm.set_exception(*m_previous_exception); - } - -private: - VM& m_vm; - Exception* m_previous_exception; -}; - -} diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index 91b04e9d05..9d849c1f3a 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -26,7 +26,6 @@ #include <LibJS/Runtime/PromiseReaction.h> #include <LibJS/Runtime/Reference.h> #include <LibJS/Runtime/Symbol.h> -#include <LibJS/Runtime/TemporaryClearException.h> #include <LibJS/Runtime/VM.h> #include <LibJS/SourceTextModule.h> #include <LibJS/SyntheticModule.h> @@ -152,8 +151,6 @@ void VM::gather_roots(HashTable<Cell*>& roots) for (auto* string : m_single_ascii_character_strings) roots.set(string); - roots.set(m_exception); - auto gather_roots_from_execution_context_stack = [&roots](Vector<ExecutionContext*> const& stack) { for (auto& execution_context : stack) { if (execution_context->this_value.is_cell()) @@ -564,11 +561,6 @@ ThrowCompletionOr<void> VM::initialize_instance_elements(Object& object, ECMAScr return {}; } -void VM::throw_exception(Exception& exception) -{ - set_exception(exception); -} - // 9.4.4 ResolveThisBinding ( ), https://tc39.es/ecma262/#sec-resolvethisbinding ThrowCompletionOr<Value> VM::resolve_this_binding(GlobalObject& global_object) { @@ -610,9 +602,7 @@ bool VM::in_strict_mode() const void VM::run_queued_promise_jobs() { dbgln_if(PROMISE_DEBUG, "Running queued promise jobs"); - // Temporarily get rid of the exception, if any - job functions must be called - // either way, and that can't happen if we already have an exception stored. - TemporaryClearException temporary_clear_exception(*this); + while (!m_promise_jobs.is_empty()) { auto* job = m_promise_jobs.take_first(); dbgln_if(PROMISE_DEBUG, "Calling promise job function @ {}", job); @@ -633,15 +623,11 @@ void VM::run_queued_promise_jobs() // This doesn't match the spec, it actually defines that Job Abstract Closures must return // a normal completion. In reality that's not the case however, and all major engines clear // exceptions when running Promise jobs. See the commit where these two lines were initially - // added for a much more detailed explanation. - clear_exception(); + // added for a much more detailed explanation. (Hash: a53542e0a3fbd7bf22b685d87f0473e489e1cf42) if (pushed_execution_context) pop_execution_context(); } - // Ensure no job has created a new exception, they must clean up after themselves. - // If they don't, we help a little (see above) so that this assumption remains valid. - VERIFY(!m_exception); } // 9.5.4 HostEnqueuePromiseJob ( job, realm ), https://tc39.es/ecma262/#sec-hostenqueuepromisejob @@ -654,7 +640,8 @@ void VM::run_queued_finalization_registry_cleanup_jobs() { while (!m_finalization_registry_cleanup_jobs.is_empty()) { auto* registry = m_finalization_registry_cleanup_jobs.take_first(); - registry->cleanup(); + // FIXME: Handle any uncatched exceptions here. + (void)registry->cleanup(); } } @@ -793,8 +780,6 @@ ThrowCompletionOr<void> VM::link_and_eval_module(Module& module) dbgln_if(JS_MODULE_DEBUG, "[JS MODULE] Linking passed, now evaluating module {}", filepath); auto evaluated_or_error = module.evaluate(*this); - VERIFY(!exception()); - if (evaluated_or_error.is_error()) return evaluated_or_error.throw_completion(); @@ -806,10 +791,8 @@ ThrowCompletionOr<void> VM::link_and_eval_module(Module& module) // FIXME: This will break if we start doing promises actually asynchronously. VERIFY(evaluated_value->state() != Promise::State::Pending); - if (evaluated_value->state() == Promise::State::Rejected) { - VERIFY(!exception()); + if (evaluated_value->state() == Promise::State::Rejected) return JS::throw_completion(evaluated_value->result()); - } dbgln_if(JS_MODULE_DEBUG, "[JS MODULE] Evaluating passed for module {}", module.filename()); return {}; @@ -954,13 +937,10 @@ void VM::import_module_dynamically(ScriptOrModule referencing_script_or_module, } } - VERIFY(!exception()); // Note: If host_resolve_imported_module returns a module it has been loaded successfully and the next call in finish_dynamic_import will retrieve it again. auto module_or_error = host_resolve_imported_module(referencing_script_or_module, module_request); dbgln_if(JS_MODULE_DEBUG, "[JS MODULE] HostImportModuleDynamically(..., {}) -> {}", module_request.module_specifier, module_or_error.is_error() ? "failed" : "passed"); if (module_or_error.is_throw_completion()) { - // Note: We should not leak the exception thrown in host_resolve_imported_module. - clear_exception(); promise->reject(*module_or_error.throw_completion().value()); } else { auto module = module_or_error.release_value(); @@ -969,11 +949,8 @@ void VM::import_module_dynamically(ScriptOrModule referencing_script_or_module, auto evaluated_or_error = link_and_eval_module(source_text_module); if (evaluated_or_error.is_throw_completion()) { - // Note: Again we don't want to leak the exception from link_and_eval_module. - clear_exception(); promise->reject(*evaluated_or_error.throw_completion().value()); } else { - VERIFY(!exception()); promise->fulfill(js_undefined()); } } @@ -1001,7 +978,6 @@ void VM::finish_dynamic_import(ScriptOrModule referencing_script_or_module, Modu // d. Let namespace be GetModuleNamespace(moduleRecord). auto namespace_ = module_record->get_module_namespace(vm); - VERIFY(!vm.exception()); // e. If namespace is an abrupt completion, then if (namespace_.is_throw_completion()) { // i. Perform ! Call(promiseCapability.[[Reject]], undefined, « namespace.[[Value]] »). @@ -1033,8 +1009,6 @@ void VM::finish_dynamic_import(ScriptOrModule referencing_script_or_module, Modu // 5. Perform ! PerformPromiseThen(innerPromise, onFulfilled, onRejected). inner_promise->perform_then(on_fulfilled, on_rejected, {}); - - VERIFY(!exception()); } } diff --git a/Userland/Libraries/LibJS/Runtime/VM.h b/Userland/Libraries/LibJS/Runtime/VM.h index f8b9e38e2b..2f970785a3 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.h +++ b/Userland/Libraries/LibJS/Runtime/VM.h @@ -19,7 +19,6 @@ #include <LibJS/Runtime/Completion.h> #include <LibJS/Runtime/Error.h> #include <LibJS/Runtime/ErrorTypes.h> -#include <LibJS/Runtime/Exception.h> #include <LibJS/Runtime/ExecutionContext.h> #include <LibJS/Runtime/Iterator.h> #include <LibJS/Runtime/MarkedValueList.h> @@ -49,10 +48,7 @@ public: void push_interpreter(Interpreter&); void pop_interpreter(Interpreter&); - Exception* exception() { return m_exception; } - void set_exception(Exception& exception) { m_exception = &exception; } - void clear_exception() { m_exception = nullptr; } - + void clear_exception() { } void dump_backtrace() const; class InterpreterExecutionScope { @@ -90,7 +86,6 @@ public: ThrowCompletionOr<void> push_execution_context(ExecutionContext& context, GlobalObject& global_object) { - VERIFY(!exception()); // Ensure we got some stack space left, so the next function call doesn't kill us. if (did_reach_stack_space_limit()) return throw_completion<InternalError>(global_object, ErrorType::CallStackSizeExceeded); @@ -160,32 +155,11 @@ public: ThrowCompletionOr<Reference> resolve_binding(FlyString const&, Environment* = nullptr); ThrowCompletionOr<Reference> get_identifier_reference(Environment*, FlyString, bool strict, size_t hops = 0); - template<typename T, typename... Args> - void throw_exception(GlobalObject& global_object, Args&&... args) - { - return throw_exception(global_object, T::create(global_object, forward<Args>(args)...)); - } - - void throw_exception(Exception&); - void throw_exception(GlobalObject& global_object, Value value) - { - return throw_exception(*heap().allocate<Exception>(global_object, value)); - } - - template<typename T, typename... Args> - void throw_exception(GlobalObject& global_object, ErrorType type, Args&&... args) - { - return throw_exception(global_object, T::create(global_object, String::formatted(type.message(), forward<Args>(args)...))); - } - // 5.2.3.2 Throw an Exception, https://tc39.es/ecma262/#sec-throw-an-exception template<typename T, typename... Args> Completion throw_completion(GlobalObject& global_object, Args&&... args) { - auto* error = T::create(global_object, forward<Args>(args)...); - // NOTE: This is temporary until we remove VM::exception(). - throw_exception(global_object, error); - return JS::throw_completion(error); + return JS::throw_completion(T::create(global_object, forward<Args>(args)...)); } template<typename T, typename... Args> @@ -255,8 +229,6 @@ private: void import_module_dynamically(ScriptOrModule referencing_script_or_module, ModuleRequest module_request, PromiseCapability promise_capability); void finish_dynamic_import(ScriptOrModule referencing_script_or_module, ModuleRequest module_request, PromiseCapability promise_capability, Promise* inner_promise); - Exception* m_exception { nullptr }; - HashMap<String, PrimitiveString*> m_string_cache; Heap m_heap; diff --git a/Userland/Libraries/LibJS/SourceTextModule.cpp b/Userland/Libraries/LibJS/SourceTextModule.cpp index 066e97ac89..406a318e65 100644 --- a/Userland/Libraries/LibJS/SourceTextModule.cpp +++ b/Userland/Libraries/LibJS/SourceTextModule.cpp @@ -664,7 +664,6 @@ Completion SourceTextModule::execute_module(VM& vm, Optional<PromiseCapability> // d. Suspend moduleContext and remove it from the execution context stack. vm.pop_execution_context(); - vm.clear_exception(); // e. Resume the context that is now on the top of the execution context stack as the running execution context. // FIXME: We don't have resume yet. diff --git a/Userland/Libraries/LibTest/JavaScriptTestRunner.h b/Userland/Libraries/LibTest/JavaScriptTestRunner.h index f126409ca9..a83d250d11 100644 --- a/Userland/Libraries/LibTest/JavaScriptTestRunner.h +++ b/Userland/Libraries/LibTest/JavaScriptTestRunner.h @@ -363,9 +363,6 @@ inline JSFileResult TestRunner::run_file_test(const String& test_path) g_vm->pop_execution_context(); } - if (g_vm->exception()) - g_vm->clear_exception(); - auto test_json = get_test_results(*interpreter); if (test_json.is_error()) { warnln("Received malformed JSON from test \"{}\"", test_path); diff --git a/Userland/Libraries/LibWeb/CMakeLists.txt b/Userland/Libraries/LibWeb/CMakeLists.txt index ef3adaf176..e5c31dea25 100644 --- a/Userland/Libraries/LibWeb/CMakeLists.txt +++ b/Userland/Libraries/LibWeb/CMakeLists.txt @@ -194,6 +194,7 @@ set(SOURCES HTML/Parser/ListOfActiveFormattingElements.cpp HTML/Parser/StackOfOpenElements.cpp HTML/Scripting/ClassicScript.cpp + HTML/Scripting/ExceptionReporter.cpp HTML/Scripting/Script.cpp HTML/SyntaxHighlighter/SyntaxHighlighter.cpp HTML/TagNames.cpp diff --git a/Userland/Libraries/LibWeb/DOM/AbortSignal.cpp b/Userland/Libraries/LibWeb/DOM/AbortSignal.cpp index 1cb2e23cc7..a8b887c77f 100644 --- a/Userland/Libraries/LibWeb/DOM/AbortSignal.cpp +++ b/Userland/Libraries/LibWeb/DOM/AbortSignal.cpp @@ -76,16 +76,10 @@ HTML::EventHandler AbortSignal::onabort() // https://dom.spec.whatwg.org/#dom-abortsignal-throwifaborted JS::ThrowCompletionOr<void> AbortSignal::throw_if_aborted() const { - auto& global_object = wrapper()->global_object(); - auto& vm = global_object.vm(); - // The throwIfAborted() method steps are to throw this’s abort reason, if this is aborted. if (!aborted()) return {}; - // FIXME: Remove this once VM::exception() has been removed. - vm.throw_exception(global_object, m_abort_reason); - return JS::throw_completion(m_abort_reason); } diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 7e66f898b2..3e2421c6fc 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -53,6 +53,7 @@ #include <LibWeb/HTML/HTMLScriptElement.h> #include <LibWeb/HTML/HTMLTitleElement.h> #include <LibWeb/HTML/MessageEvent.h> +#include <LibWeb/HTML/Scripting/ExceptionReporter.h> #include <LibWeb/Layout/BlockFormattingContext.h> #include <LibWeb/Layout/InitialContainingBlock.h> #include <LibWeb/Layout/TreeBuilder.h> @@ -641,32 +642,6 @@ JS::Interpreter& Document::interpreter() // FIXME: This isn't exactly the right place for this. HTML::main_thread_event_loop().perform_a_microtask_checkpoint(); - // 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() || message.is_accessor()) { - // The result is not going to be useful, let's just print the value. This affects DOMExceptions, for example. - dbgln("\033[31;1mUnhandled JavaScript exception:\033[0m {}", value); - } else { - dbgln("\033[31;1mUnhandled JavaScript exception:\033[0m [{}] {}", name, message); - } - } else { - dbgln("\033[31;1mUnhandled JavaScript exception:\033[0m {}", 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); - } - } - vm.finish_execution_generation(); }; } @@ -685,10 +660,10 @@ JS::Value Document::run_javascript(StringView source, StringView filename) auto result = interpreter.run(script_or_error.value()); - auto& vm = interpreter.vm(); if (result.is_error()) { // FIXME: I'm sure the spec could tell us something about error propagation here! - vm.clear_exception(); + HTML::report_exception(result); + return {}; } return result.value(); diff --git a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp index d4ec043ebe..45c3f53611 100644 --- a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp +++ b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp @@ -23,6 +23,7 @@ #include <LibWeb/DOM/ShadowRoot.h> #include <LibWeb/DOM/Window.h> #include <LibWeb/HTML/EventNames.h> +#include <LibWeb/HTML/Scripting/ExceptionReporter.h> #include <LibWeb/UIEvents/MouseEvent.h> namespace Web::DOM { @@ -89,11 +90,17 @@ bool EventDispatcher::inner_invoke(Event& event, Vector<EventTarget::EventListen auto* this_value = Bindings::wrap(global, *event.current_target()); auto* wrapped_event = Bindings::wrap(global, event); - auto& vm = global.vm(); - [[maybe_unused]] auto rc = JS::call(global, function, this_value, wrapped_event); - if (vm.exception()) { - vm.clear_exception(); - // FIXME: Set legacyOutputDidListenersThrowFlag if given. (Only used by IndexedDB currently) + + // 10. Call a user object’s operation with listener’s callback, "handleEvent", « event », and event’s currentTarget attribute value. + auto result = JS::call(global, function, this_value, wrapped_event); + + // If this throws an exception, then: + if (result.is_error()) { + // 1. Report the exception. + VERIFY(result.throw_completion().value().has_value()); + HTML::report_exception(*result.throw_completion().value()); + + // FIXME: 2. Set legacyOutputDidListenersThrowFlag if given. (Only used by IndexedDB currently) } event.set_in_passive_listener(false); diff --git a/Userland/Libraries/LibWeb/DOM/Window.cpp b/Userland/Libraries/LibWeb/DOM/Window.cpp index 04cc04b80e..072e77560b 100644 --- a/Userland/Libraries/LibWeb/DOM/Window.cpp +++ b/Userland/Libraries/LibWeb/DOM/Window.cpp @@ -19,6 +19,7 @@ #include <LibWeb/HTML/BrowsingContext.h> #include <LibWeb/HTML/EventLoop/EventLoop.h> #include <LibWeb/HTML/PageTransitionEvent.h> +#include <LibWeb/HTML/Scripting/ExceptionReporter.h> #include <LibWeb/HighResolutionTime/Performance.h> #include <LibWeb/Layout/InitialContainingBlock.h> #include <LibWeb/Page/Page.h> @@ -170,11 +171,9 @@ void Window::timer_did_fire(Badge<Timer>, Timer& timer) HTML::queue_global_task(HTML::Task::Source::TimerTask, associated_document(), [this, strong_this = NonnullRefPtr(*this), strong_timer = NonnullRefPtr(timer)]() mutable { // We should not be here if there's no JS wrapper for the Window object. VERIFY(wrapper()); - auto& vm = wrapper()->vm(); - - [[maybe_unused]] auto rc = JS::call(wrapper()->global_object(), strong_timer->callback(), wrapper()); - if (vm.exception()) - vm.clear_exception(); + auto result = JS::call(wrapper()->global_object(), strong_timer->callback(), wrapper()); + if (result.is_error()) + HTML::report_exception(result); }); } @@ -198,14 +197,18 @@ void Window::clear_interval(i32 timer_id) m_timers.remove(timer_id); } +// https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html#run-the-animation-frame-callbacks i32 Window::request_animation_frame(JS::FunctionObject& js_callback) { auto callback = request_animation_frame_driver().add([this, handle = JS::make_handle(&js_callback)](i32 id) mutable { auto& function = *handle.cell(); - auto& vm = function.vm(); - (void)JS::call(function.global_object(), function, JS::js_undefined(), JS::Value(performance().now())); - if (vm.exception()) - vm.clear_exception(); + // 3. Invoke callback, passing now as the only argument, + auto result = JS::call(function.global_object(), function, JS::js_undefined(), JS::Value(performance().now())); + + // and if an exception is thrown, report the exception. + if (result.is_error()) + HTML::report_exception(result); + m_request_animation_frame_callbacks.remove(id); }); m_request_animation_frame_callbacks.set(callback->id(), callback); @@ -399,11 +402,10 @@ void Window::queue_microtask(JS::FunctionObject& callback) { // The queueMicrotask(callback) method must queue a microtask to invoke callback, HTML::queue_a_microtask(associated_document(), [&callback, handle = JS::make_handle(&callback)]() { - auto& vm = callback.vm(); - [[maybe_unused]] auto rc = JS::call(callback.global_object(), callback, JS::js_null()); - // FIXME: ...and if callback throws an exception, report the exception. - if (vm.exception()) - vm.clear_exception(); + auto result = JS::call(callback.global_object(), callback, JS::js_null()); + // and if callback throws an exception, report the exception. + if (result.is_error()) + HTML::report_exception(result); }); } diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/ClassicScript.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/ClassicScript.cpp index b60ab646c9..4d43864e83 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/ClassicScript.cpp +++ b/Userland/Libraries/LibWeb/HTML/Scripting/ClassicScript.cpp @@ -8,6 +8,7 @@ #include <LibCore/ElapsedTimer.h> #include <LibJS/Interpreter.h> #include <LibWeb/HTML/Scripting/ClassicScript.h> +#include <LibWeb/HTML/Scripting/ExceptionReporter.h> namespace Web::HTML { @@ -81,7 +82,7 @@ JS::Value ClassicScript::run(RethrowErrors rethrow_errors) dbgln_if(HTML_SCRIPT_DEBUG, "ClassicScript: Finished running script {}, Duration: {}ms", filename(), timer.elapsed()); if (evaluation_status.is_error()) { // FIXME: Propagate error according to the spec. - interpreter->vm().clear_exception(); + report_exception(evaluation_status); return {}; } return evaluation_status.value(); diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp new file mode 100644 index 0000000000..8135cb98d9 --- /dev/null +++ b/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2022, David Tuin <davidot@serenityos.org> + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include <AK/TypeCasts.h> +#include <LibJS/Runtime/VM.h> +#include <LibJS/Runtime/Value.h> +#include <LibWeb/HTML/Scripting/ExceptionReporter.h> + +namespace Web::HTML { + +// https://html.spec.whatwg.org/#report-the-exception +void report_exception(JS::ThrowCompletionOr<JS::Value> const& result) +{ + // FIXME: This is just old code, and does not strictly follow the spec of report an exception. + // FIXME: We should probably also report these exceptions to the JS console. + VERIFY(result.throw_completion().value().has_value()); + auto thrown_value = *result.throw_completion().value(); + if (thrown_value.is_object()) { + auto& object = thrown_value.as_object(); + auto& vm = object.vm(); + 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() || message.is_accessor()) { + // The result is not going to be useful, let's just print the value. This affects DOMExceptions, for example. + dbgln("\033[31;1mUnhandled JavaScript exception:\033[0m {}", thrown_value); + } else { + dbgln("\033[31;1mUnhandled JavaScript exception:\033[0m [{}] {}", name, message); + } + if (is<JS::Error>(object)) { + auto const& error_value = static_cast<JS::Error const&>(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; + dbgln(" {} at {}:{}:{}", function_name, source_range.filename, source_range.start.line, source_range.start.column); + } + } + } else { + dbgln("\033[31;1mUnhandled JavaScript exception:\033[0m {}", thrown_value); + } +} + +} diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.h b/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.h new file mode 100644 index 0000000000..a931ca4575 --- /dev/null +++ b/Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.h @@ -0,0 +1,15 @@ +/* + * Copyright (c) 2022, David Tuin <davidot@serenityos.org> + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include <LibJS/Forward.h> + +namespace Web::HTML { + +void report_exception(JS::ThrowCompletionOr<JS::Value> const& value); + +} diff --git a/Userland/Utilities/js.cpp b/Userland/Utilities/js.cpp index 3b98ceeb12..3219136226 100644 --- a/Userland/Utilities/js.cpp +++ b/Userland/Utilities/js.cpp @@ -891,7 +891,6 @@ static void print_value(JS::Value value, HashTable<JS::Object*>& seen_objects) auto prototype_or_error = object.internal_get_prototype_of(); if (prototype_or_error.has_value() && prototype_or_error.value() == object.global_object().error_prototype()) return print_error(object, seen_objects); - vm->clear_exception(); if (is<JS::RegExpObject>(object)) return print_regexp_object(object, seen_objects); @@ -1059,7 +1058,7 @@ static bool parse_and_run(JS::Interpreter& interpreter, StringView source, Strin if (!hint.is_empty()) outln("{}", hint); outln(error.to_string()); - vm->throw_exception<JS::SyntaxError>(interpreter.global_object(), error.to_string()); + result = JS::SyntaxError::create(interpreter.global_object(), error.to_string()); } else { auto return_early = run_script_or_module(script_or_error.value()); if (return_early == ReturnEarly::Yes) @@ -1073,7 +1072,7 @@ static bool parse_and_run(JS::Interpreter& interpreter, StringView source, Strin if (!hint.is_empty()) outln("{}", hint); outln(error.to_string()); - vm->throw_exception<JS::SyntaxError>(interpreter.global_object(), error.to_string()); + result = JS::SyntaxError::create(interpreter.global_object(), error.to_string()); } else { auto return_early = run_script_or_module(module_or_error.value()); if (return_early == ReturnEarly::Yes) @@ -1081,12 +1080,13 @@ static bool parse_and_run(JS::Interpreter& interpreter, StringView source, Strin } } - auto handle_exception = [&] { - auto* exception = vm->exception(); - vm->clear_exception(); + auto handle_exception = [&](JS::Value thrown_value) { js_out("Uncaught exception: "); - print(exception->value()); - auto& traceback = exception->traceback(); + print(thrown_value); + + 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(); if (traceback.size() > 1) { unsigned repetitions = 0; for (size_t i = 0; i < traceback.size(); ++i) { @@ -1117,21 +1117,11 @@ static bool parse_and_run(JS::Interpreter& interpreter, StringView source, Strin last_value = JS::make_handle(result.value()); if (result.is_error()) { - if (!vm->exception()) { - // Until js no longer relies on vm->exception() we have to set it in case the exception was cleared - VERIFY(result.throw_completion().value().has_value()); - auto throw_value = result.release_error().release_value().release_value(); - auto* exception = interpreter.heap().allocate<JS::Exception>(interpreter.global_object(), throw_value); - vm->set_exception(*exception); - } - handle_exception(); + VERIFY(result.throw_completion().value().has_value()); + handle_exception(*result.release_error().value()); return false; } else if (s_print_last_result) { print(result.value()); - if (vm->exception()) { - handle_exception(); - return false; - } } return true; } @@ -1401,10 +1391,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments) }; OwnPtr<JS::Interpreter> interpreter; - interrupt_interpreter = [&] { - auto error = JS::Error::create(interpreter->global_object(), "Received SIGINT"); - vm->throw_exception(interpreter->global_object(), error); - }; + // FIXME: Figure out some way to interrupt the interpreter now that vm.exception() is gone. if (script_paths.is_empty()) { s_print_last_result = true; |