From 57130908b35d3a1d18fdbb1f7152d6d9bf46c568 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sat, 4 Apr 2026 15:39:01 +0200 Subject: [PATCH] LibJS+LibWeb: Make DOMException hold an [[ErrorData]] slot Split JS::ErrorData out of JS::Error so that it can be used both by JS::Error and WebIDL::DOMException. This adds support for Error.isError to DOMException, also letting us report DOMException stack information to the console. --- Libraries/LibJS/CMakeLists.txt | 1 + Libraries/LibJS/Console.cpp | 4 +- Libraries/LibJS/Console.h | 4 +- Libraries/LibJS/Forward.h | 1 + Libraries/LibJS/Runtime/Error.cpp | 99 +------------- Libraries/LibJS/Runtime/Error.h | 30 +---- Libraries/LibJS/Runtime/ErrorConstructor.cpp | 9 +- Libraries/LibJS/Runtime/ErrorData.cpp | 112 ++++++++++++++++ Libraries/LibJS/Runtime/ErrorData.h | 53 ++++++++ Libraries/LibJS/Runtime/ErrorPrototype.cpp | 16 +-- Libraries/LibJS/Runtime/Object.h | 3 + Libraries/LibJS/Runtime/ObjectPrototype.cpp | 2 +- Libraries/LibWeb/HTML/ErrorInformation.cpp | 4 +- .../HTML/Scripting/ExceptionReporter.cpp | 24 ++-- Libraries/LibWeb/WebIDL/DOMException.cpp | 8 ++ Libraries/LibWeb/WebIDL/DOMException.h | 6 + Services/WebContent/DevToolsConsoleClient.cpp | 16 +-- Services/WebContent/DevToolsConsoleClient.h | 2 +- .../DOMException-custom-bindings.any.txt | 20 +++ .../DOMException-is-error.any.txt | 6 + .../DOMException-custom-bindings.any.html | 15 +++ .../DOMException-custom-bindings.any.js | 122 ++++++++++++++++++ .../DOMException-is-error.any.html | 15 +++ .../DOMException-is-error.any.js | 9 ++ 24 files changed, 429 insertions(+), 152 deletions(-) create mode 100644 Libraries/LibJS/Runtime/ErrorData.cpp create mode 100644 Libraries/LibJS/Runtime/ErrorData.h create mode 100644 Tests/LibWeb/Text/expected/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.txt create mode 100644 Tests/LibWeb/Text/expected/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.txt create mode 100644 Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.html create mode 100644 Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.js create mode 100644 Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.html create mode 100644 Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.js diff --git a/Libraries/LibJS/CMakeLists.txt b/Libraries/LibJS/CMakeLists.txt index 0a7a7c479c..b849451355 100644 --- a/Libraries/LibJS/CMakeLists.txt +++ b/Libraries/LibJS/CMakeLists.txt @@ -79,6 +79,7 @@ set(SOURCES Runtime/Environment.cpp Runtime/Error.cpp Runtime/ErrorConstructor.cpp + Runtime/ErrorData.cpp Runtime/ErrorPrototype.cpp Runtime/ErrorTypes.cpp Runtime/ExecutionContext.cpp diff --git a/Libraries/LibJS/Console.cpp b/Libraries/LibJS/Console.cpp index 12c070dc3c..4257e71efe 100644 --- a/Libraries/LibJS/Console.cpp +++ b/Libraries/LibJS/Console.cpp @@ -753,10 +753,10 @@ void Console::output_debug_message(LogLevel log_level, StringView output) const } } -void Console::report_exception(JS::Error const& exception, bool in_promise) const +void Console::report_exception(String const& name, String const& message, JS::ErrorData const& error_data, bool in_promise) const { if (m_client) - m_client->report_exception(exception, in_promise); + m_client->report_exception(name, message, error_data, in_promise); } ThrowCompletionOr Console::value_vector_to_string(GC::RootVector const& values) diff --git a/Libraries/LibJS/Console.h b/Libraries/LibJS/Console.h index f11a599ca3..92e10d0511 100644 --- a/Libraries/LibJS/Console.h +++ b/Libraries/LibJS/Console.h @@ -97,7 +97,7 @@ public: ThrowCompletionOr time_end(); void output_debug_message(LogLevel log_level, StringView output) const; - void report_exception(JS::Error const&, bool) const; + void report_exception(String const& name, String const& message, JS::ErrorData const&, bool) const; private: explicit Console(Realm&); @@ -126,7 +126,7 @@ public: virtual ThrowCompletionOr printer(Console::LogLevel log_level, PrinterArguments) = 0; virtual void add_css_style_to_current_message(StringView) { } - virtual void report_exception(JS::Error const&, bool) { } + virtual void report_exception(String const&, String const&, JS::ErrorData const&, bool) { } virtual void clear() = 0; virtual void end_group() = 0; diff --git a/Libraries/LibJS/Forward.h b/Libraries/LibJS/Forward.h index 4f6ee961a5..11e0a3eddb 100644 --- a/Libraries/LibJS/Forward.h +++ b/Libraries/LibJS/Forward.h @@ -180,6 +180,7 @@ struct DisposableResource; class ECMAScriptFunctionObject; class Environment; class Error; +class ErrorData; class ErrorType; struct ExecutionContext; struct ExportEntry; diff --git a/Libraries/LibJS/Runtime/Error.cpp b/Libraries/LibJS/Runtime/Error.cpp index 31cbd1c527..55d459b0c1 100644 --- a/Libraries/LibJS/Runtime/Error.cpp +++ b/Libraries/LibJS/Runtime/Error.cpp @@ -5,27 +5,14 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include #include #include -#include -#include #include -#include namespace JS { GC_DEFINE_ALLOCATOR(Error); -static SourceRange dummy_source_range { SourceCode::create({}, {}), {}, {} }; - -SourceRange const& TracebackFrame::source_range() const -{ - if (!cached_source_range.has_value()) - return dummy_source_range; - return *cached_source_range; -} - GC::Ref Error::create(Realm& realm) { return realm.create(realm.intrinsics().error_prototype()); @@ -43,16 +30,21 @@ GC::Ref Error::create(Realm& realm, StringView message) return create(realm, Utf16String::from_utf8(message)); } +Utf16String Error::stack_string(CompactTraceback compact) const +{ + return ErrorData::stack_string(compact); +} + Error::Error(Object& prototype) : Object(ConstructWithPrototypeTag::Tag, prototype) + , ErrorData(prototype.vm()) { - populate_stack(); } void Error::visit_edges(Visitor& visitor) { Base::visit_edges(visitor); - visitor.visit(m_cached_string); + ErrorData::visit_edges(visitor); } // 20.5.8.1 InstallErrorCause ( O, options ), https://tc39.es/ecma262/#sec-installerrorcause @@ -81,83 +73,6 @@ void Error::set_message(Utf16String message) define_direct_property(vm.names.message, PrimitiveString::create(vm, move(message)), attr); } -void Error::populate_stack() -{ - auto stack_trace = vm().stack_trace(); - m_traceback.ensure_capacity(stack_trace.size()); - for (auto& element : stack_trace) { - auto* context = element.execution_context; - m_traceback.append({ - .function_name = context->function ? context->function->name_for_call_stack() : ""_utf16, - .cached_source_range = move(element.source_range), - }); - } -} - -Utf16String Error::stack_string(CompactTraceback compact) const -{ - if (m_traceback.is_empty()) - return {}; - - StringBuilder stack_string_builder(StringBuilder::Mode::UTF16); - - // Note: We roughly follow V8's formatting - auto append_frame = [&](TracebackFrame const& frame) { - auto const& function_name = frame.function_name; - auto const& 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.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() ? ""_utf16 : function_name); - } - }; - - auto is_same_frame = [](TracebackFrame const& a, TracebackFrame const& b) { - if (a.function_name.is_empty() && b.function_name.is_empty()) { - auto const& source_range_a = a.source_range(); - auto const& 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 stack_string_builder.to_utf16_string(); -} - #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName, ArrayType) \ GC_DEFINE_ALLOCATOR(ClassName); \ GC::Ref ClassName::create(Realm& realm) \ diff --git a/Libraries/LibJS/Runtime/Error.h b/Libraries/LibJS/Runtime/Error.h index 19f7d5ed79..1eb25b5fa9 100644 --- a/Libraries/LibJS/Runtime/Error.h +++ b/Libraries/LibJS/Runtime/Error.h @@ -12,24 +12,14 @@ #include #include #include +#include #include -#include namespace JS { -struct JS_API TracebackFrame { - Utf16String function_name; - [[nodiscard]] SourceRange const& source_range() const; - - Optional cached_source_range; -}; - -enum CompactTraceback { - No, - Yes, -}; - -class JS_API Error : public Object { +class JS_API Error + : public Object + , public ErrorData { JS_OBJECT(Error, Object); GC_DECLARE_ALLOCATOR(Error); @@ -46,11 +36,6 @@ public: void set_message(Utf16String); - Vector const& traceback() const { return m_traceback; } - - void set_cached_string(GC::Ref string) { m_cached_string = string; } - GC::Ptr cached_string() const { return m_cached_string; } - protected: explicit Error(Object& prototype); @@ -58,11 +43,8 @@ protected: private: virtual bool is_error_object() const final { return true; } - - void populate_stack(); - Vector m_traceback; - - GC::Ptr m_cached_string; + virtual ErrorData* error_data() final { return this; } + virtual ErrorData const* error_data() const final { return this; } }; template<> diff --git a/Libraries/LibJS/Runtime/ErrorConstructor.cpp b/Libraries/LibJS/Runtime/ErrorConstructor.cpp index ba5d796aee..38ccc62960 100644 --- a/Libraries/LibJS/Runtime/ErrorConstructor.cpp +++ b/Libraries/LibJS/Runtime/ErrorConstructor.cpp @@ -127,9 +127,16 @@ JS_ENUMERATE_NATIVE_ERRORS JS_DEFINE_NATIVE_FUNCTION(ErrorConstructor::is_error) { // 1. If arg is not an Object, return false. + auto object = vm.argument(0).as_if(); + if (!object) + return false; + // 2. If arg does not have an [[ErrorData]] internal slot, return false. + if (!object->has_error_data()) + return false; + // 3. Return true. - return vm.argument(0).is(); + return true; } } diff --git a/Libraries/LibJS/Runtime/ErrorData.cpp b/Libraries/LibJS/Runtime/ErrorData.cpp new file mode 100644 index 0000000000..0132a37a55 --- /dev/null +++ b/Libraries/LibJS/Runtime/ErrorData.cpp @@ -0,0 +1,112 @@ +/* + * Copyright (c) 2020-2025, Andreas Kling + * Copyright (c) 2021-2023, Linus Groh + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include +#include + +namespace JS { + +static SourceRange dummy_source_range { SourceCode::create({}, {}), {}, {} }; + +SourceRange const& TracebackFrame::source_range() const +{ + if (!cached_source_range.has_value()) + return dummy_source_range; + return *cached_source_range; +} + +ErrorData::ErrorData(VM& vm) +{ + populate_stack(vm); +} + +void ErrorData::visit_edges(Cell::Visitor& visitor) +{ + visitor.visit(m_cached_string); +} + +void ErrorData::populate_stack(VM& vm) +{ + auto stack_trace = vm.stack_trace(); + m_traceback.ensure_capacity(stack_trace.size()); + for (auto& element : stack_trace) { + auto* context = element.execution_context; + m_traceback.append({ + .function_name = context->function ? context->function->name_for_call_stack() : ""_utf16, + .cached_source_range = move(element.source_range), + }); + } +} + +Utf16String ErrorData::stack_string(CompactTraceback compact) const +{ + if (m_traceback.is_empty()) + return {}; + + StringBuilder stack_string_builder(StringBuilder::Mode::UTF16); + + // Note: We roughly follow V8's formatting + auto append_frame = [&](TracebackFrame const& frame) { + auto const& function_name = frame.function_name; + auto const& 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.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() ? ""_utf16 : function_name); + } + }; + + auto is_same_frame = [](TracebackFrame const& a, TracebackFrame const& b) { + if (a.function_name.is_empty() && b.function_name.is_empty()) { + auto const& source_range_a = a.source_range(); + auto const& 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 Error.prototype.stack + 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 stack_string_builder.to_utf16_string(); +} + +} diff --git a/Libraries/LibJS/Runtime/ErrorData.h b/Libraries/LibJS/Runtime/ErrorData.h new file mode 100644 index 0000000000..779b942fb8 --- /dev/null +++ b/Libraries/LibJS/Runtime/ErrorData.h @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2020, Andreas Kling + * Copyright (c) 2021-2022, Linus Groh + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace JS { + +struct JS_API TracebackFrame { + Utf16String function_name; + [[nodiscard]] SourceRange const& source_range() const; + + Optional cached_source_range; +}; + +enum CompactTraceback { + No, + Yes, +}; + +class JS_API ErrorData { +public: + explicit ErrorData(VM&); + + [[nodiscard]] Utf16String stack_string(CompactTraceback compact = CompactTraceback::No) const; + [[nodiscard]] Vector const& traceback() const { return m_traceback; } + + void set_cached_string(GC::Ref string) { m_cached_string = string; } + [[nodiscard]] GC::Ptr cached_string() const { return m_cached_string; } + +protected: + void visit_edges(Cell::Visitor&); + +private: + void populate_stack(VM&); + + Vector m_traceback; + GC::Ptr m_cached_string; +}; + +} diff --git a/Libraries/LibJS/Runtime/ErrorPrototype.cpp b/Libraries/LibJS/Runtime/ErrorPrototype.cpp index 4192a58ce2..fc0baed0f3 100644 --- a/Libraries/LibJS/Runtime/ErrorPrototype.cpp +++ b/Libraries/LibJS/Runtime/ErrorPrototype.cpp @@ -78,35 +78,35 @@ JS_DEFINE_NATIVE_FUNCTION(ErrorPrototype::stack_getter) auto this_object = TRY(PrototypeObject::this_object(vm)); // 3. If E does not have an [[ErrorData]] internal slot, return undefined. - auto* error = as_if(*this_object); - if (!error) + auto* error_data = this_object->error_data(); + if (!error_data) return js_undefined(); // OPTIMIZATION: Avoid recomputing the stack string if we already have it cached. // At least one major engine does this as well, so it's not expected that changing // the name or message properties updates the stack string. - if (error->cached_string()) - return error->cached_string(); + if (error_data->cached_string()) + return error_data->cached_string(); // 4. Return ? GetStackString(error). // NOTE: These steps are not implemented based on the proposal, but to roughly follow behavior of other browsers. String name {}; - if (auto name_property = TRY(error->get(vm.names.name)); !name_property.is_undefined()) + if (auto name_property = TRY(this_object->get(vm.names.name)); !name_property.is_undefined()) name = TRY(name_property.to_string(vm)); else name = "Error"_string; Utf16String message {}; - if (auto message_property = TRY(error->get(vm.names.message)); !message_property.is_undefined()) + if (auto message_property = TRY(this_object->get(vm.names.message)); !message_property.is_undefined()) message = TRY(message_property.to_utf16_string(vm)); auto header = message.is_empty() ? move(name) : MUST(String::formatted("{}: {}", name, message)); - auto string = PrimitiveString::create(vm, Utf16String::formatted("{}\n{}", header, error->stack_string())); - error->set_cached_string(string); + auto string = PrimitiveString::create(vm, Utf16String::formatted("{}\n{}", header, error_data->stack_string())); + error_data->set_cached_string(string); return string; } diff --git a/Libraries/LibJS/Runtime/Object.h b/Libraries/LibJS/Runtime/Object.h index d58ea9ec79..5533f142b8 100644 --- a/Libraries/LibJS/Runtime/Object.h +++ b/Libraries/LibJS/Runtime/Object.h @@ -263,6 +263,9 @@ public: virtual bool is_set_object() const { return false; } virtual bool is_map_object() const { return false; } virtual bool is_weak_map() const { return false; } + virtual ErrorData* error_data() { return nullptr; } + virtual ErrorData const* error_data() const { return nullptr; } + bool has_error_data() const { return error_data(); } virtual bool is_typed_array_base() const { return false; } #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName, Type) \ diff --git a/Libraries/LibJS/Runtime/ObjectPrototype.cpp b/Libraries/LibJS/Runtime/ObjectPrototype.cpp index 142e9a8d17..3f0d5580d6 100644 --- a/Libraries/LibJS/Runtime/ObjectPrototype.cpp +++ b/Libraries/LibJS/Runtime/ObjectPrototype.cpp @@ -160,7 +160,7 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectPrototype::to_string) else if (object->is_function()) builtin_tag = "Function"sv; // 8. Else if O has an [[ErrorData]] internal slot, let builtinTag be "Error". - else if (is(*object)) + else if (object->has_error_data()) builtin_tag = "Error"sv; // 9. Else if O has a [[BooleanData]] internal slot, let builtinTag be "Boolean". else if (is(*object)) diff --git a/Libraries/LibWeb/HTML/ErrorInformation.cpp b/Libraries/LibWeb/HTML/ErrorInformation.cpp index b4a2d58f99..ad2769ee45 100644 --- a/Libraries/LibWeb/HTML/ErrorInformation.cpp +++ b/Libraries/LibWeb/HTML/ErrorInformation.cpp @@ -36,8 +36,8 @@ ErrorInformation extract_error_information(JS::VM& vm, JS::Value exception) // to the entire source document! Calculate that somehow. // NB: If we got an Error object, then try and extract the information from the location the object was made. - if (auto error = exception.as_if()) { - for (auto const& frame : error->traceback()) { + if (auto object = exception.as_if(); object && object->error_data()) { + for (auto const& frame : object->error_data()->traceback()) { auto source_range = frame.source_range(); if (source_range.start.line != 0 || source_range.start.column != 0) { attributes.filename = MUST(String::from_byte_string(source_range.filename())); diff --git a/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp b/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp index c2b848c8bb..40b21b39b5 100644 --- a/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp +++ b/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp @@ -35,21 +35,27 @@ void report_exception_to_console(JS::Value value, JS::Realm& realm, ErrorInPromi } else { dbgln("\033[31;1mUnhandled JavaScript exception{}:\033[0m [{}] {}", error_in_promise == ErrorInPromise::Yes ? " (in promise)" : "", name, message); } - if (is(object)) { - // FIXME: We should be doing this for DOMException as well - // https://webidl.spec.whatwg.org/#js-DOMException-specialness - // "Additionally, if an implementation gives native Error objects special powers or nonstandard properties (such as a stack property), it should also expose those on DOMException objects." - auto const& error_value = static_cast(object); - dbgln("{}", error_value.stack_string(JS::CompactTraceback::Yes)); - console.report_exception(error_value, error_in_promise == ErrorInPromise::Yes); - + if (auto const* error_data = object.error_data()) { + String exception_name; + String exception_message; + if (auto const* exception = as_if(object)) { + exception_name = exception->name().to_string(); + exception_message = MUST(exception->message().view().to_utf8()); + } else { + exception_name = name.to_string_without_side_effects(); + exception_message = message.to_string_without_side_effects(); + } + dbgln("{}", error_data->stack_string(JS::CompactTraceback::Yes)); + console.report_exception(exception_name, exception_message, *error_data, error_in_promise == ErrorInPromise::Yes); return; } } else { 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_utf16_string_without_side_effects()), error_in_promise == ErrorInPromise::Yes); + auto message = value.to_string_without_side_effects(); + auto error = JS::Error::create(realm, Utf16String::from_utf8(message)); + console.report_exception("Error"_string, message, *error, error_in_promise == ErrorInPromise::Yes); } // https://html.spec.whatwg.org/multipage/webappapis.html#report-the-exception diff --git a/Libraries/LibWeb/WebIDL/DOMException.cpp b/Libraries/LibWeb/WebIDL/DOMException.cpp index 1d0ac762b5..93d7582dad 100644 --- a/Libraries/LibWeb/WebIDL/DOMException.cpp +++ b/Libraries/LibWeb/WebIDL/DOMException.cpp @@ -30,6 +30,7 @@ GC::Ref DOMException::construct_impl(JS::Realm& realm, Utf16String DOMException::DOMException(JS::Realm& realm, FlyString name, Utf16String const& message) : PlatformObject(realm) + , ErrorData(realm.vm()) , m_name(move(name)) , m_message(message) { @@ -37,6 +38,7 @@ DOMException::DOMException(JS::Realm& realm, FlyString name, Utf16String const& DOMException::DOMException(JS::Realm& realm) : PlatformObject(realm) + , ErrorData(realm.vm()) { } @@ -48,6 +50,12 @@ void DOMException::initialize(JS::Realm& realm) Base::initialize(realm); } +void DOMException::visit_edges(Visitor& visitor) +{ + Base::visit_edges(visitor); + ErrorData::visit_edges(visitor); +} + WebIDL::ExceptionOr DOMException::serialization_steps(HTML::TransferDataEncoder& serialized, bool, HTML::SerializationMemory&) { // 1. Set serialized.[[Name]] to value’s name. diff --git a/Libraries/LibWeb/WebIDL/DOMException.h b/Libraries/LibWeb/WebIDL/DOMException.h index 1ab45d5e94..e9c682c0ba 100644 --- a/Libraries/LibWeb/WebIDL/DOMException.h +++ b/Libraries/LibWeb/WebIDL/DOMException.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -94,6 +95,7 @@ static u16 get_legacy_code_for_name(FlyString const& name) // https://webidl.spec.whatwg.org/#idl-DOMException class WEB_API DOMException : public Bindings::PlatformObject + , public JS::ErrorData , public Bindings::Serializable { WEB_PLATFORM_OBJECT(DOMException, Bindings::PlatformObject); GC_DECLARE_ALLOCATOR(DOMException); @@ -120,8 +122,12 @@ protected: explicit DOMException(JS::Realm&); virtual void initialize(JS::Realm&) override; + virtual void visit_edges(Visitor&) override; private: + virtual ErrorData* error_data() final { return this; } + virtual ErrorData const* error_data() const final { return this; } + FlyString m_name; Utf16FlyString m_message; }; diff --git a/Services/WebContent/DevToolsConsoleClient.cpp b/Services/WebContent/DevToolsConsoleClient.cpp index 88f4068d32..65468666da 100644 --- a/Services/WebContent/DevToolsConsoleClient.cpp +++ b/Services/WebContent/DevToolsConsoleClient.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -99,17 +100,12 @@ void DevToolsConsoleClient::handle_result(JS::Value result) m_client->did_execute_js_console_input(serialize_js_value(m_realm, result)); } -void DevToolsConsoleClient::report_exception(JS::Error const& exception, bool in_promise) +void DevToolsConsoleClient::report_exception(String const& name, String const& message, JS::ErrorData const& error_data, bool in_promise) { - auto& vm = exception.vm(); - - auto name = exception.get_without_side_effects(vm.names.name); - auto message = exception.get_without_side_effects(vm.names.message); - Vector trace; - trace.ensure_capacity(exception.traceback().size()); + trace.ensure_capacity(error_data.traceback().size()); - for (auto const& frame : exception.traceback()) { + for (auto const& frame : error_data.traceback()) { auto const& source_range = frame.source_range(); WebView::StackFrame stack_frame; @@ -129,8 +125,8 @@ void DevToolsConsoleClient::report_exception(JS::Error const& exception, bool in send_console_output({ .timestamp = UnixDateTime::now(), .output = WebView::ConsoleError { - .name = name.to_string_without_side_effects(), - .message = message.to_string_without_side_effects(), + .name = name, + .message = message, .trace = move(trace), .inside_promise = in_promise, }, diff --git a/Services/WebContent/DevToolsConsoleClient.h b/Services/WebContent/DevToolsConsoleClient.h index 09abd9b709..28bd429f05 100644 --- a/Services/WebContent/DevToolsConsoleClient.h +++ b/Services/WebContent/DevToolsConsoleClient.h @@ -29,7 +29,7 @@ private: DevToolsConsoleClient(JS::Realm&, JS::Console&, PageClient&, ConsoleGlobalEnvironmentExtensions&); virtual void handle_result(JS::Value) override; - virtual void report_exception(JS::Error const&, bool) override; + virtual void report_exception(String const& name, String const& message, JS::ErrorData const&, bool) override; virtual void end_group() override { } virtual void clear() override { } diff --git a/Tests/LibWeb/Text/expected/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.txt b/Tests/LibWeb/Text/expected/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.txt new file mode 100644 index 0000000000..c30d4c029b --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.txt @@ -0,0 +1,20 @@ +Harness status: OK + +Found 15 tests + +15 Pass +Pass Cannot construct without new +Pass inherits from Error: prototype-side +Pass does not inherit from Error: class-side +Pass message property descriptor +Pass message getter performs brand checks (i.e. is not [LegacyLenientThis]) +Pass name property descriptor +Pass name getter performs brand checks (i.e. is not [LegacyLenientThis]) +Pass code property descriptor +Pass code getter performs brand checks (i.e. is not [LegacyLenientThis]) +Pass code property is not affected by shadowing the name property +Pass Object.prototype.toString behavior is like other interfaces +Pass Inherits its toString() from Error.prototype +Pass toString() behavior from Error.prototype applies as expected +Pass DOMException.prototype.toString() applied to DOMException.prototype throws because of name/message brand checks +Pass If the implementation has a stack property on normal errors, it also does on DOMExceptions \ No newline at end of file diff --git a/Tests/LibWeb/Text/expected/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.txt b/Tests/LibWeb/Text/expected/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.txt new file mode 100644 index 0000000000..2d5e058273 --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.txt @@ -0,0 +1,6 @@ +Harness status: OK + +Found 1 tests + +1 Pass +Pass DOMException-is-error \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.html b/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.html new file mode 100644 index 0000000000..f3bf2135a5 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.html @@ -0,0 +1,15 @@ + + + + + + + +
+ diff --git a/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.js b/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.js new file mode 100644 index 0000000000..d1c86930d4 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.js @@ -0,0 +1,122 @@ +// META: global=window,dedicatedworker,shadowrealm + +"use strict"; + +test(() => { + assert_throws_js(TypeError, () => DOMException()); +}, "Cannot construct without new"); + +test(() => { + assert_equals(Object.getPrototypeOf(DOMException.prototype), Error.prototype); +}, "inherits from Error: prototype-side"); + +test(() => { + assert_equals(Object.getPrototypeOf(DOMException), Function.prototype); +}, "does not inherit from Error: class-side"); + +test(() => { + const e = new DOMException("message", "name"); + assert_false(e.hasOwnProperty("message"), "property is not own"); + + const propDesc = Object.getOwnPropertyDescriptor(DOMException.prototype, "message"); + assert_equals(typeof propDesc.get, "function", "property descriptor is a getter"); + assert_equals(propDesc.set, undefined, "property descriptor is not a setter"); + assert_true(propDesc.enumerable, "property descriptor enumerable"); + assert_true(propDesc.configurable, "property descriptor configurable"); +}, "message property descriptor"); + +test(() => { + const getter = Object.getOwnPropertyDescriptor(DOMException.prototype, "message").get; + + assert_throws_js(TypeError, () => getter.apply({})); +}, "message getter performs brand checks (i.e. is not [LegacyLenientThis])"); + +test(() => { + const e = new DOMException("message", "name"); + assert_false(e.hasOwnProperty("name"), "property is not own"); + + const propDesc = Object.getOwnPropertyDescriptor(DOMException.prototype, "name"); + assert_equals(typeof propDesc.get, "function", "property descriptor is a getter"); + assert_equals(propDesc.set, undefined, "property descriptor is not a setter"); + assert_true(propDesc.enumerable, "property descriptor enumerable"); + assert_true(propDesc.configurable, "property descriptor configurable"); +}, "name property descriptor"); + +test(() => { + const getter = Object.getOwnPropertyDescriptor(DOMException.prototype, "name").get; + + assert_throws_js(TypeError, () => getter.apply({})); +}, "name getter performs brand checks (i.e. is not [LegacyLenientThis])"); + +test(() => { + const e = new DOMException("message", "name"); + assert_false(e.hasOwnProperty("code"), "property is not own"); + + const propDesc = Object.getOwnPropertyDescriptor(DOMException.prototype, "code"); + assert_equals(typeof propDesc.get, "function", "property descriptor is a getter"); + assert_equals(propDesc.set, undefined, "property descriptor is not a setter"); + assert_true(propDesc.enumerable, "property descriptor enumerable"); + assert_true(propDesc.configurable, "property descriptor configurable"); +}, "code property descriptor"); + +test(() => { + const getter = Object.getOwnPropertyDescriptor(DOMException.prototype, "code").get; + + assert_throws_js(TypeError, () => getter.apply({})); +}, "code getter performs brand checks (i.e. is not [LegacyLenientThis])"); + +test(() => { + const e = new DOMException("message", "InvalidCharacterError"); + assert_equals(e.code, 5, "Initially the code is set to 5"); + + Object.defineProperty(e, "name", { + value: "WrongDocumentError" + }); + + assert_equals(e.code, 5, "The code is still set to 5"); +}, "code property is not affected by shadowing the name property"); + +test(() => { + const e = new DOMException("message", "name"); + assert_equals(Object.prototype.toString.call(e), "[object DOMException]"); +}, "Object.prototype.toString behavior is like other interfaces"); + +test(() => { + const e = new DOMException("message", "name"); + assert_false(e.hasOwnProperty("toString"), "toString must not exist on the instance"); + assert_false(DOMException.prototype.hasOwnProperty("toString"), "toString must not exist on DOMException.prototype"); + assert_equals(typeof e.toString, "function", "toString must still exist (via Error.prototype)"); +}, "Inherits its toString() from Error.prototype"); + +test(() => { + const e = new DOMException("message", "name"); + assert_equals(e.toString(), "name: message", + "The default Error.prototype.toString() behavior must work on supplied name and message"); + + Object.defineProperty(e, "name", { value: "new name" }); + Object.defineProperty(e, "message", { value: "new message" }); + assert_equals(e.toString(), "new name: new message", + "The default Error.prototype.toString() behavior must work on shadowed names and messages"); +}, "toString() behavior from Error.prototype applies as expected"); + +test(() => { + assert_throws_js(TypeError, () => DOMException.prototype.toString()); +}, "DOMException.prototype.toString() applied to DOMException.prototype throws because of name/message brand checks"); + +test(() => { + let stackOnNormalErrors; + try { + throw new Error("normal error"); + } catch (e) { + stackOnNormalErrors = e.stack; + } + + let stackOnDOMException; + try { + throw new DOMException("message", "name"); + } catch (e) { + stackOnDOMException = e.stack; + } + + assert_equals(typeof stackOnDOMException, typeof stackOnNormalErrors, "The typeof values must match"); +}, "If the implementation has a stack property on normal errors, it also does on DOMExceptions"); diff --git a/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.html b/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.html new file mode 100644 index 0000000000..edaab22ca6 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.html @@ -0,0 +1,15 @@ + + + + + + + +
+ diff --git a/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.js b/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.js new file mode 100644 index 0000000000..6f3097b222 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/webidl/ecmascript-binding/es-exceptions/DOMException-is-error.any.js @@ -0,0 +1,9 @@ +// META: global=window,dedicatedworker,shadowrealm + +'use strict'; + +test(function() { + // https://github.com/tc39/proposal-is-error/issues/9 + // https://github.com/whatwg/webidl/pull/1421 + assert_true(Error.isError(new DOMException())); +});