diff --git a/docs/reference/rust-review-checklist.md b/docs/reference/rust-review-checklist.md index 9fb00cae79e..763daa6b778 100644 --- a/docs/reference/rust-review-checklist.md +++ b/docs/reference/rust-review-checklist.md @@ -59,8 +59,10 @@ Unsafe code is concentrated at FFI boundaries. Review every `unsafe` block and ` - **Always** ensure that library crates (e.g., `dns`, `net`, `transpiler`) use `thiserror` for domain-specific error types. These should implement `Display` and have descriptive error messages. - **Always** ensure that JSG-facing crates (e.g., `api`, `jsg`) use `jsg::Error` with an - `ExceptionType` variant (e.g., `TypeError`, `RangeError`). Domain errors should implement - `From for jsg::Error` for ergonomic `?` usage. + `ExceptionType` variant. `TypeError`, `RangeError` and the other unprefixed variants throw + the JS built-in error; a `DOM`-prefixed variant throws a `DOMException` with the unprefixed + name (`DOMSyntaxError` throws `DOMException` named `SyntaxError`), as in C++ `JSG_REQUIRE`. + Domain errors should implement `From for jsg::Error` for ergonomic `?` usage. - **Never** use `panic!` for expected errors. Panics across FFI are undefined behavior. Use `Result` and propagate errors through the CXX bridge. The build denies `unwrap()`, `expect()`, `panic!`, `todo!` and `unimplemented!` in production code. `unreachable!` is allowed: a panic is diff --git a/src/per_isolate/AGENTS.md b/src/per_isolate/AGENTS.md index ae358f30187..07989399af5 100644 --- a/src/per_isolate/AGENTS.md +++ b/src/per_isolate/AGENTS.md @@ -18,6 +18,12 @@ at context creation, before any user code. C++ entry point: `view-extent`). - TypeScript `import type` / `export type` are used freely for type plumbing (fully erased; the loader only sees `module.exports`). +- `main.ts` first installs the TypeScript `DOMException` (`dom-exception.ts`), + which replaces the C++ class with one of the same shape. Native code creates + instances of it without running its constructor, and recognizes one by the + brand in its V8 private properties (`newDOMException()` and + `tryGetDOMExceptionFields()` in `src/workerd/jsg/jsg.h`); bootstrap code checks + for one with `utils.isDOMException()`, never `instanceof`. - `main.ts` installs the real stream globals (ReadableStream et al.) when the `typescript_implemented_streams` compat flag is enabled, plus the internal-testing `ReadableStreamDrainingReader` under diff --git a/src/per_isolate/dom-exception.ts b/src/per_isolate/dom-exception.ts new file mode 100644 index 00000000000..318e16b33ec --- /dev/null +++ b/src/per_isolate/dom-exception.ts @@ -0,0 +1,152 @@ +'use strict'; + +// The global DOMException (https://webidl.spec.whatwg.org/#idl-DOMException). +// +// A DOMException looks as the C++ JSG class's instances do (src/workerd/jsg/dom-exception.h): +// own read-only `code`, `name` and `message` properties, an enumerable own `stack`, and the +// legacy code constants on both the constructor and the prototype. Native code creates +// DOMExceptions without running this constructor (jsg::newDOMException()), over the prototype +// that install() hands it, and recognizes them by the brand that utils.initDOMException() sets. +// This constructor sets the same brand, so utils.isDOMException() is true for either kind, and +// for a subclass instance. +// +// DOMException is a function rather than a class so that its `prototype` property is +// writable, as a JSG class's is unless the immutable_api_prototypes flag is set. + +const { + ErrorCaptureStackTrace, + ErrorPrototype, + ObjectCreate, + ObjectDefineProperty, + ObjectFreeze, + ObjectGetOwnPropertyDescriptor, + ObjectGetPrototypeOf, + SymbolFor, + SymbolToStringTag, + TypeError, +} = primordials; + +// The legacy codes, in the order JSG defines the constants +// (JSG_DOM_EXCEPTION_FOR_EACH_ERROR_NAME). +const LEGACY_CODES: readonly (readonly [string, number])[] = [ + ['INDEX_SIZE_ERR', 1], + ['DOMSTRING_SIZE_ERR', 2], + ['HIERARCHY_REQUEST_ERR', 3], + ['WRONG_DOCUMENT_ERR', 4], + ['INVALID_CHARACTER_ERR', 5], + ['NO_DATA_ALLOWED_ERR', 6], + ['NO_MODIFICATION_ALLOWED_ERR', 7], + ['NOT_FOUND_ERR', 8], + ['NOT_SUPPORTED_ERR', 9], + ['INUSE_ATTRIBUTE_ERR', 10], + ['INVALID_STATE_ERR', 11], + ['SYNTAX_ERR', 12], + ['INVALID_MODIFICATION_ERR', 13], + ['NAMESPACE_ERR', 14], + ['INVALID_ACCESS_ERR', 15], + ['VALIDATION_ERR', 16], + ['TYPE_MISMATCH_ERR', 17], + ['SECURITY_ERR', 18], + ['NETWORK_ERR', 19], + ['ABORT_ERR', 20], + ['URL_MISMATCH_ERR', 21], + ['QUOTA_EXCEEDED_ERR', 22], + ['TIMEOUT_ERR', 23], + ['INVALID_NODE_TYPE_ERR', 24], + ['DATA_CLONE_ERR', 25], +]; + +// The message and name are converted as Web IDL converts a DOMString, which a template literal +// does too: a Symbol throws a TypeError. +function DOMException(this: object, ...args: unknown[]): void { + if (new.target === undefined) { + throw new TypeError( + "Failed to construct 'DOMException': Please use the 'new' operator, this object constructor cannot be called as a function." + ); + } + const message = args[0] === undefined ? '' : `${args[0] as string}`; + const name = args[1] === undefined ? 'Error' : `${args[1] as string}`; + utils.initDOMException(this, name, message); + // Leaves out this constructor's frame, but not a subclass constructor's. + ErrorCaptureStackTrace(this, DOMException); + // Error properties are not enumerable, but a DOMException's always have been. + ObjectDefineProperty(this, 'stack', { __proto__: null, enumerable: true }); +} + +// Defines `key` on `target` with the attributes it has on `source`, which JSG chooses by +// compatibility flags, and `value`. +function copyProperty( + target: object, + source: object, + key: string | symbol, + value: unknown +): void { + const descriptor = ObjectGetOwnPropertyDescriptor(source, key); + if (descriptor === undefined) { + throw new TypeError(`DOMException has no ${String(key)} to copy`); + } + ObjectDefineProperty(target, key, { ...descriptor, __proto__: null, value }); +} + +// Gives DOMException the properties of `native`, the C++ class it replaces, with their +// attributes and in their order. +function setUpClass(native: Function): void { + const nativePrototype = native.prototype as object; + copyProperty(DOMException, native, 'length', 0); + copyProperty(DOMException, native, 'name', 'DOMException'); + + const prototype = ObjectCreate(ErrorPrototype) as object; + for (let i = 0; i < LEGACY_CODES.length; i++) { + const [name, code] = LEGACY_CODES[i] as readonly [string, number]; + copyProperty(DOMException, native, name, code); + copyProperty(prototype, nativePrototype, name, code); + } + copyProperty(prototype, nativePrototype, 'constructor', DOMException); + // Present under the set_tostring_tag flag. + if ( + ObjectGetOwnPropertyDescriptor(nativePrototype, SymbolToStringTag) !== + undefined + ) { + copyProperty(prototype, nativePrototype, SymbolToStringTag, 'DOMException'); + } + // Tools recognize a runtime class by this symbol. + const internalClass = SymbolFor('cloudflare:internal-class'); + copyProperty(prototype, nativePrototype, internalClass, internalClass); + // util.inspect() formats an instance of a runtime class through this symbol. + const inspect = utils.getApiSymbol('kResourceTypeInspect'); + copyProperty( + prototype, + nativePrototype, + inspect, + ObjectFreeze({ __proto__: null }) + ); + + // Not writable under the immutable_api_prototypes flag. + copyProperty(DOMException, native, 'prototype', prototype); +} + +// Replaces the C++ class as the global DOMException, keeping the property where JSG defines it +// (on the global object's prototype) and its attributes, and has native code create instances +// of this class from now on. +function install(): void { + let owner: object | null = globalThis; + while (owner !== null) { + const descriptor = ObjectGetOwnPropertyDescriptor(owner, 'DOMException'); + if (descriptor !== undefined) { + setUpClass(descriptor.value as Function); + ObjectDefineProperty(owner, 'DOMException', { + ...descriptor, + __proto__: null, + value: DOMException, + }); + utils.setDOMExceptionPrototype(DOMException.prototype as object); + return; + } + owner = ObjectGetPrototypeOf(owner) as object | null; + } + throw new TypeError('the global object has no DOMException'); +} + +module.exports = { + install, +}; diff --git a/src/per_isolate/main.ts b/src/per_isolate/main.ts index bdaaa39547f..475ef53c0a1 100644 --- a/src/per_isolate/main.ts +++ b/src/per_isolate/main.ts @@ -33,6 +33,9 @@ // const { ObjectDefineProperties } = primordials; +// Native code creates DOMExceptions of the class installed here, so it is installed first. +require('dom-exception').install(); + if (compatFlags['typescript_implemented_streams']) { const { ReadableStream, diff --git a/src/per_isolate/per_isolate-env.d.ts b/src/per_isolate/per_isolate-env.d.ts index 8d6effd6da9..245dafa593e 100644 --- a/src/per_isolate/per_isolate-env.d.ts +++ b/src/per_isolate/per_isolate-env.d.ts @@ -115,6 +115,13 @@ declare const utils: { // surrogate counts the 3 bytes of U+FFFD), without encoding it. Throws a // TypeError for a non-string. utf8Length(str: string): number; + // Gives `object`, a DOMException being constructed, its brand and its own `code`, `name` and + // `message` properties (jsg::initDOMException()). Consumed by dom-exception. + initDOMException(object: object, name: string, message: string): void; + // Whether `value` is a DOMException, by its brand. Runs no JavaScript. + isDOMException(value: unknown): boolean; + // Makes `prototype` the prototype of the DOMExceptions native code creates. + setDOMExceptionPrototype(prototype: object): void; }; // An abort algorithm registration. Obtained only from diff --git a/src/per_isolate/primordials.ts b/src/per_isolate/primordials.ts index 648f39702d9..7c192a49be1 100644 --- a/src/per_isolate/primordials.ts +++ b/src/per_isolate/primordials.ts @@ -78,6 +78,15 @@ const WeakSetCtor = WeakSet; // Function const FunctionPrototypeBind = uncurryThis(FunctionPrototype.bind); +// Error +const ErrorPrototype = ErrorCtor.prototype; +// V8's Error.captureStackTrace(), which the standard library types leave out. +const ErrorCaptureStackTrace = ( + ErrorCtor as unknown as { + captureStackTrace(target: object, constructorOpt?: Function): void; + } +).captureStackTrace; + // Object const ObjectCreate = Object.create; const ObjectDefineProperty = Object.defineProperty; @@ -635,7 +644,6 @@ const EventTargetRemoveEventListener = uncurryThis( const TextDecoderCtor = globalThis.TextDecoder; const TextEncoderCtor = globalThis.TextEncoder; -const DOMException = globalThis.DOMException; const TextEncoderEncode = uncurryThis(TextEncoderCtor.prototype.encode) as ( encoder: TextEncoder, @@ -700,6 +708,10 @@ module.exports = ObjectFreeze({ FunctionPrototype, FunctionPrototypeBind, + // Error + ErrorPrototype, + ErrorCaptureStackTrace, + // Object ObjectPrototype, ObjectCreate, @@ -738,9 +750,6 @@ module.exports = ObjectFreeze({ ArrayPrototypeSlice, ArrayPrototypeSplice, - // DOMException - DOMException, - // Map MapPrototypeGet, MapPrototypeSet, diff --git a/src/per_isolate/webfs/AGENTS.md b/src/per_isolate/webfs/AGENTS.md index 4eed79362f9..850e769a0dd 100644 --- a/src/per_isolate/webfs/AGENTS.md +++ b/src/per_isolate/webfs/AGENTS.md @@ -65,8 +65,8 @@ Parts of the contract that are easy to break: a method body runs, so a bad option bag is a synchronous throw while a file that cannot be opened is a rejection. `createWritable` is therefore deliberately **not** an `async function` — that would turn the former into a - rejection too. It classifies by exception type: a DOMException becomes a - rejection, anything else is rethrown. `badOptionsThrowSynchronously` and + rejection too. It classifies by exception type: a DOMException (by its brand, + `utils.isDOMException()`) becomes a rejection, anything else is rethrown. `badOptionsThrowSynchronously` and `createWritableRejectsForeignReceiver` hold the two apart. - **The option bag is coerced, not type-checked.** JSG unwraps `jsg::Optional` with ToBoolean, so `{keepExistingData: 'false'}` opts diff --git a/src/per_isolate/webfs/writable-file-stream.ts b/src/per_isolate/webfs/writable-file-stream.ts index 6bba8e88bfe..b1e72adc47c 100644 --- a/src/per_isolate/webfs/writable-file-stream.ts +++ b/src/per_isolate/webfs/writable-file-stream.ts @@ -42,7 +42,6 @@ import type { } from '../webstreams/types'; const { - DOMException: NativeDOMException, ObjectDefineProperties, PromiseReject, PromiseResolve, @@ -50,7 +49,7 @@ const { TypeError, } = primordials; -const { createFileSystemWriteContext } = utils; +const { createFileSystemWriteContext, isDOMException } = utils; const { WritableStream, @@ -196,7 +195,7 @@ function createWritable( // A DOMException means the file could not be opened, which createWritable // reports by rejecting. Anything else is the receiver check failing, which // JSG performs before the method body runs and so throws. - if (err instanceof NativeDOMException) { + if (isDOMException(err)) { return PromiseReject(err); } throw err; diff --git a/src/rust/jsg-test/ffi.c++ b/src/rust/jsg-test/ffi.c++ index bc772005a2a..1b5cfb0ba82 100644 --- a/src/rust/jsg-test/ffi.c++ +++ b/src/rust/jsg-test/ffi.c++ @@ -292,6 +292,11 @@ bool isolates_share_group(Isolate* a, Isolate* b) { return a->GetGroup() == b->GetGroup(); } +void throw_kj_exception(::rust::Str description) { + kj::throwFatalException(kj::Exception(kj::Exception::Type::FAILED, __FILE__, __LINE__, + kj::heapString(description.data(), description.size()))); +} + void hold_across_gc(Isolate* isolate, ::rust::Fn<::workerd::rust::jsg::MaybeLocal(Isolate*)> create, ::rust::Fn check) { diff --git a/src/rust/jsg-test/ffi.h b/src/rust/jsg-test/ffi.h index ebd2731531e..bf1025273cc 100644 --- a/src/rust/jsg-test/ffi.h +++ b/src/rust/jsg-test/ffi.h @@ -116,6 +116,9 @@ ::workerd::rust::jsg::Local cpp_deserialize(Isolate* isolate, ::rust::Slice bool; + /// Throws a `kj::Exception` with `description`, such as a tunneled + /// `jsg-internal.DOMException(OperationError): ...`. + #[expect(clippy::allow_attributes)] + #[allow(dead_code)] + pub fn throw_kj_exception(description: &str) -> Result<()>; + /// Holds a value the way C++ code holds one that Rust created: calls `create`, keeps /// its result only in a `v8::Global` while a full GC runs, and calls `check` with it. /// Then resets the `Global` and runs a full GC again. See `hold_across_gc` in `ffi.h`. diff --git a/src/rust/jsg-test/tests/dom_exception.rs b/src/rust/jsg-test/tests/dom_exception.rs new file mode 100644 index 00000000000..ab7547c121b --- /dev/null +++ b/src/rust/jsg-test/tests/dom_exception.rs @@ -0,0 +1,372 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +//! The DOM-prefixed `ExceptionType`s throw a `DOMException`, as the C++ +//! `JSG_REQUIRE(..., DOMSyntaxError, ...)` error types do, and `FromJS` for +//! `jsg::Error` recognizes a `DOMException`. + +use jsg::ExceptionType; +use jsg::Number; +use jsg_macros::jsg_constructor; +use jsg_macros::jsg_method; +use jsg_macros::jsg_property; +use jsg_macros::jsg_resource; + +/// Each DOM-prefixed type with the name and legacy `code` the thrown +/// `DOMException` has (`src/workerd/jsg/dom-exception.h`). +#[rustfmt::skip] +const DOM_TYPES: [(ExceptionType, &str, u16); 13] = [ + ( + ExceptionType::DOMOperationError, + "OperationError", + 0, + ), + ( + ExceptionType::DOMDataError, + "DataError", + 0, + ), + ( + ExceptionType::DOMDataCloneError, + "DataCloneError", + 25, + ), + ( + ExceptionType::DOMInvalidAccessError, + "InvalidAccessError", + 15, + ), + ( + ExceptionType::DOMInvalidStateError, + "InvalidStateError", + 11, + ), + ( + ExceptionType::DOMInvalidCharacterError, + "InvalidCharacterError", + 5, + ), + ( + ExceptionType::DOMNotSupportedError, + "NotSupportedError", + 9, + ), + ( + ExceptionType::DOMSyntaxError, + "SyntaxError", + 12, + ), + ( + ExceptionType::DOMTimeoutError, + "TimeoutError", + 23, + ), + ( + ExceptionType::DOMTypeMismatchError, + "TypeMismatchError", + 17, + ), + ( + ExceptionType::DOMQuotaExceededError, + "QuotaExceededError", + 22, + ), + ( + ExceptionType::DOMAbortError, + "AbortError", + 20, + ), + ( + ExceptionType::DOMNotFoundError, + "NotFoundError", + 8, + ), +]; + +/// The JS error classes, which must stay built-in errors. +const JS_TYPES: [(ExceptionType, &str); 5] = [ + (ExceptionType::Error, "Error"), + (ExceptionType::TypeError, "TypeError"), + (ExceptionType::RangeError, "RangeError"), + (ExceptionType::ReferenceError, "ReferenceError"), + (ExceptionType::SyntaxError, "SyntaxError"), +]; + +#[jsg_resource] +struct Thrower; + +#[jsg_resource] +impl Thrower { + /// Fails with the DOM type at `index` in `DOM_TYPES`, or for a negative + /// `index`, the JS type at `-index - 1` in `JS_TYPES`. + #[jsg_method] + pub fn fail(&self, index: i32) -> Result { + let ty = match usize::try_from(index) { + Ok(i) => DOM_TYPES[i].0, + Err(_) => JS_TYPES[usize::try_from(-index - 1).unwrap()].0, + }; + Err(jsg::Error::from_type(ty, format!("failed {index}"))) + } + + /// Fails with a `DOMException` named `name`. + #[jsg_method] + pub fn fail_named(&self, name: String) -> Result { + Err(jsg::Error::new_dom_exception(&name, "failed")) + } + + /// Converts `value` to a `jsg::Error` and fails with it. + #[jsg_method] + pub fn rethrow<'s>( + &self, + lock: &mut jsg::Lock<'s>, + value: jsg::v8::Local<'s, jsg::v8::Value>, + ) -> Result { + Err(::from_js(lock, value)?) + } + + /// Fails with the error a tunneled KJ exception with `description` converts to. + #[jsg_method] + pub fn fail_tunneled(&self, description: String) -> Result { + Err(crate::ffi::throw_kj_exception(&description) + .unwrap_err() + .into()) + } + + #[jsg_property(readonly)] + pub fn get_failing(&self) -> Result { + Err(jsg::Error::new_dom_invalid_state_error("getter")) + } +} + +/// A resource whose constructor fails with a `DOMException`. +#[jsg_resource] +struct FailingConstructor; + +#[jsg_resource] +impl FailingConstructor { + #[jsg_constructor] + fn constructor() -> jsg::Result { + Err(jsg::Error::new_dom_not_supported_error("constructor")) + } +} + +fn set_thrower(lock: &mut jsg::Lock, ctx: &crate::EvalContext<'_>) { + ctx.set_global(lock, "thrower", jsg::Rc::new(Thrower)); +} + +/// Describes what `call` throws as `constructor name/name/message/code/is Error`. +fn describe_thrown(call: &str) -> String { + format!( + "(() => {{ + try {{ {call}; return 'did not throw'; }} + catch (e) {{ + return [e.constructor.name, e.name, e.message, e.code, e instanceof Error].join('/'); + }} + }})()" + ) +} + +#[test] +fn dom_types_throw_dom_exceptions() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + set_thrower(lock, ctx); + for (index, (_, name, code)) in DOM_TYPES.iter().enumerate() { + let thrown: String = ctx + .eval(lock, &describe_thrown(&format!("thrower.fail({index})"))) + .unwrap(); + assert_eq!( + thrown, + format!("DOMException/{name}/failed {index}/{code}/true") + ); + } + Ok(()) + }); +} + +#[test] +fn js_types_throw_built_in_errors() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + set_thrower(lock, ctx); + for (i, (_, name)) in JS_TYPES.iter().enumerate() { + let index = -i32::try_from(i).unwrap() - 1; + let call = format!("thrower.fail({index})"); + let thrown: String = ctx.eval(lock, &describe_thrown(&call)).unwrap(); + assert_eq!(thrown, format!("{name}/{name}/failed {index}//true")); + let is_built_in: bool = ctx + .eval( + lock, + &format!("(() => {{ try {{ {call} }} catch (e) {{ return e instanceof {name}; }} }})()"), + ) + .unwrap(); + assert!(is_built_in, "{name}"); + } + Ok(()) + }); +} + +#[test] +fn thrown_dom_exceptions_convert_back_to_their_type() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + set_thrower(lock, ctx); + for (index, (ty, _, _)) in DOM_TYPES.iter().enumerate() { + let err = ctx + .eval::(lock, &format!("thrower.fail({index})")) + .unwrap_err() + .unwrap_jsg_err(lock); + assert_eq!(err.name, *ty); + assert_eq!(err.message, format!("failed {index}")); + } + let err = ctx + .eval::(lock, "thrower.fail(-5)") + .unwrap_err() + .unwrap_jsg_err(lock); + assert_eq!(err.name, ExceptionType::SyntaxError); + Ok(()) + }); +} + +#[test] +fn script_created_dom_exceptions_convert() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + set_thrower(lock, ctx); + ctx.eval_raw( + lock, + "globalThis.DOMException = (() => { try { thrower.fail(0) } catch (e) { return e.constructor; } })()", + ) + .unwrap(); + + let convert = |lock: &mut jsg::Lock, expr: &str| { + let value = ctx.eval_raw(lock, expr).unwrap(); + ::from_js(lock, value).unwrap() + }; + + let err = convert(lock, "new DOMException('m', 'AbortError')"); + assert_eq!(err.name, ExceptionType::DOMAbortError); + assert_eq!(err.message, "m"); + + // A name without a DOM-prefixed type, legacy or chosen by the script. + for name in ["IndexSizeError", "CustomError"] { + let err = convert(lock, &format!("new DOMException('m', '{name}')")); + assert_eq!(err.name, ExceptionType::Error, "{name}"); + assert_eq!(err.message, "m", "{name}"); + } + + // The name and message it was created with, not the configurable JS properties. + let err = convert( + lock, + "(() => { + const e = new DOMException('m', 'DataError'); + Object.defineProperty(e, 'name', { value: 'TypeError' }); + Object.defineProperty(e, 'message', { value: 'overridden' }); + if (e.name !== 'TypeError' || e.message !== 'overridden') throw 'not overridden'; + return e; + })()", + ); + assert_eq!(err.name, ExceptionType::DOMDataError); + assert_eq!(err.message, "m"); + + // A subclass instance is a DOMException too. + let err = convert( + lock, + "(() => { class Sub extends DOMException {} return new Sub('m', 'NotFoundError'); })()", + ); + assert_eq!(err.name, ExceptionType::DOMNotFoundError); + + // Plain objects are still not errors. + let value = ctx.eval_raw(lock, "({ name: 'AbortError', message: 'm' })").unwrap(); + let err = ::from_js(lock, value).unwrap_err(); + assert_eq!(err.name, ExceptionType::TypeError); + Ok(()) + }); +} + +#[test] +fn constructors_and_properties_throw_dom_exceptions() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + set_thrower(lock, ctx); + let constructor = jsg::resource::function_template_of::(lock); + ctx.set_global(lock, "FailingConstructor", constructor); + let thrown: String = ctx + .eval(lock, &describe_thrown("new FailingConstructor()")) + .unwrap(); + assert_eq!(thrown, "DOMException/NotSupportedError/constructor/9/true"); + let thrown: String = ctx.eval(lock, &describe_thrown("thrower.failing")).unwrap(); + assert_eq!(thrown, "DOMException/InvalidStateError/getter/11/true"); + Ok(()) + }); +} + +/// A `DOMException` name without a DOM-prefixed type keeps its name, both when created in Rust +/// and when converted from JavaScript, as C++ JSG keeps it. +#[test] +fn dom_exception_names_without_a_type_are_kept() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + set_thrower(lock, ctx); + for name in ["IndexSizeError", "CustomError"] { + let err = jsg::Error::new_dom_exception(name, "m"); + assert_eq!(err.name, ExceptionType::Error); + assert_eq!(err.dom_exception_name(), Some(name)); + assert_eq!(err.to_string(), format!("DOMException({name}): m")); + + let thrown: String = ctx + .eval(lock, &describe_thrown(&format!("thrower.failNamed('{name}')"))) + .unwrap(); + let code = if name == "IndexSizeError" { "1" } else { "0" }; + assert_eq!(thrown, format!("DOMException/{name}/failed/{code}/true")); + + let thrown: String = ctx + .eval( + lock, + &describe_thrown(&format!( + "thrower.rethrow((() => {{ try {{ thrower.failNamed('{name}') }} catch (e) {{ return e; }} }})())" + )), + ) + .unwrap(); + assert_eq!(thrown, format!("DOMException/{name}/failed/{code}/true")); + } + // A name with a type gets the type. + let err = jsg::Error::new_dom_exception("AbortError", "m"); + assert_eq!(err.name, ExceptionType::DOMAbortError); + assert_eq!(err.dom_exception_name(), Some("AbortError")); + assert_eq!(jsg::Error::new_type_error("m").dom_exception_name(), None); + Ok(()) + }); +} + +/// A tunneled `jsg.DOMException` is thrown as that `DOMException`, and an internal one as a +/// `DOMException` with the redacted message, as C++ throws it. +#[test] +fn tunneled_dom_exceptions_throw_dom_exceptions() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + set_thrower(lock, ctx); + let thrown: String = ctx + .eval( + lock, + &describe_thrown("thrower.failTunneled('jsg.DOMException(IndexSizeError): boom')"), + ) + .unwrap(); + assert_eq!(thrown, "DOMException/IndexSizeError/boom/1/true"); + + let thrown: String = ctx + .eval( + lock, + &describe_thrown( + "thrower.failTunneled('jsg-internal.DOMException(OperationError): secret')", + ), + ) + .unwrap(); + assert_eq!( + thrown, + "DOMException/OperationError/internal error; reference = 0123456789abcdefghijklmn/0/true" + ); + Ok(()) + }); +} diff --git a/src/rust/jsg-test/tests/mod.rs b/src/rust/jsg-test/tests/mod.rs index 63ed0dbf5d7..e09b309401a 100644 --- a/src/rust/jsg-test/tests/mod.rs +++ b/src/rust/jsg-test/tests/mod.rs @@ -9,6 +9,7 @@ mod builtin_module; mod coercion_safety; mod collections_gc; mod date_bigint; +mod dom_exception; mod eval; mod fast_api; mod function; diff --git a/src/rust/jsg-test/tests/resource_callback.rs b/src/rust/jsg-test/tests/resource_callback.rs index 2b8ed398cdf..3402d7eacc9 100644 --- a/src/rust/jsg-test/tests/resource_callback.rs +++ b/src/rust/jsg-test/tests/resource_callback.rs @@ -658,6 +658,26 @@ impl CallingResource { Err(jsg::Error::new_type_error("replaced")) } + /// Calls `callback`, and fails with a new error that is thrown as a `DOMException` whether + /// or not it threw: a `DOMAbortError`, or for `internal`, a tunneled internal one. + #[jsg_method] + pub fn call_then_fail_with_dom_exception<'s>( + &self, + lock: &mut jsg::Lock<'s>, + callback: v8::Local<'s, v8::Value>, + internal: bool, + ) -> Result<(), jsg::Error> { + let _ = Self::function(callback)?.call::>(lock, None, &[]); + if internal { + let err: jsg::Error = + crate::ffi::throw_kj_exception("jsg-internal.DOMException(OperationError): secret") + .unwrap_err() + .into(); + return Err(err); + } + Err(jsg::Error::new_dom_abort_error("replaced")) + } + /// Calls `callback` in `try_catch`, keeping the error the call failed with after /// `try_catch` caught its exception. #[jsg_method] @@ -838,6 +858,34 @@ fn error_does_not_replace_termination() { }); } +/// Nor is an error that would be thrown as a `DOMException`, whose creation would clear the +/// termination. +#[test] +fn dom_exception_does_not_replace_termination() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + calling_resource(lock, ctx); + for internal in [false, true] { + let result = ctx.eval::( + lock, + &format!( + "globalThis.after = false; + try {{ obj.callThenFailWithDomException({TERMINATE}, {internal}); }} + catch (e) {{ globalThis.caught = true; }} + globalThis.after = true;" + ), + ); + assert!(result.is_err(), "the termination was caught: {result:?}"); + assert!(!ctx.eval::(lock, "globalThis.after").unwrap()); + assert!( + !ctx.eval::(lock, "globalThis.caught === true") + .unwrap() + ); + } + Ok(()) + }); +} + /// An error that replaces a pending exception is thrown in its place, as when C++ JSG code /// throws while a `JsExceptionThrown` unwinds. #[test] diff --git a/src/rust/jsg/BUILD.bazel b/src/rust/jsg/BUILD.bazel index 150feab7506..7d53c5313e8 100644 --- a/src/rust/jsg/BUILD.bazel +++ b/src/rust/jsg/BUILD.bazel @@ -75,6 +75,7 @@ wd_cc_library( ":v8.rs@cxx", "//src/rust/cxx/kj-rs", "//src/workerd/jsg:jsg-core", + "//src/workerd/util:sentry", "@workerd-v8//:v8", ], ) diff --git a/src/rust/jsg/README.md b/src/rust/jsg/README.md index d7337f852ce..5ed72468a88 100644 --- a/src/rust/jsg/README.md +++ b/src/rust/jsg/README.md @@ -519,6 +519,8 @@ Converting a value to JavaScript can fail too, as C++ JSG's `wrap()` can throw, Code that gets a pending error must not call into JavaScript, and should return the error. Converting it to another error and throwing that replaces the exception, as when C++ code throws while a `JsExceptionThrown` unwinds; nothing ever replaces a termination, and none of these run while execution is terminating: they fail with a pending error without preparing V8 to run JavaScript, which would clear the termination, even for a value that would convert without running JavaScript. A pending error is only meaningful while its exception is pending: one kept past that, such as a clone stored after `try_catch` caught the exception, is thrown as an internal error when V8 has no exception pending. +For the JavaScript class that an `Err` returned from a callback is thrown as, see [Throwing errors](#throwing-errors). + To handle an exception instead, run the code in `Lock::try_catch`, which catches as C++ `jsg::Lock::tryCatch()` does: the exception, or an error the closure returns, as a value; a termination keeps propagating. ```rust @@ -834,6 +836,31 @@ impl WebSocket { Constants are set on both the constructor and prototype as read-only, non-configurable properties per Web IDL. The name is used as-is (no camelCase conversion). Only numeric types are supported (`i8`..`i64`, `u8`..`u64`, `f32`, `f64`). +## Throwing errors + +A method, property or constructor that returns `Err(jsg::Error)` throws it. The error's +`ExceptionType` picks the class: `Error`, `TypeError`, `RangeError`, `ReferenceError` and +`SyntaxError` are the JS built-ins, and the `DOM`-prefixed types (`DOMSyntaxError`, +`DOMAbortError`, `DOMQuotaExceededError`, ...) throw a `DOMException` with the unprefixed +name, as the C++ `JSG_REQUIRE(..., DOMSyntaxError, ...)` error types do: + +```rust +jsg_require!(valid, DOMSyntaxError, "Invalid base64"); +// JS: DOMException { name: 'SyntaxError', code: 12, message: 'Invalid base64' } +``` + +`Error::new_dom_exception(name, message)` creates an error for any `DOMException` name, as C++ +`jsg::Lock::domException()` does; a name without a `DOM`-prefixed type, such as the legacy +`IndexSizeError`, has the type `Error`, and `Error::dom_exception_name()` returns it. `FromJS` +for `jsg::Error` reads a caught native error's `name` and `message`, and the name and message a +`DOMException` was created with, which it carries in V8 private properties (see +`newDOMException()` in `src/workerd/jsg/jsg.h`). An internal error from a tunneled +`jsg-internal.DOMException()` is thrown as a `DOMException` with that name and the +redacted message, as in C++. + +Nothing is thrown while execution is terminating, and an error that stands for a pending +exception throws nothing either; see [Errors from JavaScript](#errors-from-javascript). + ## Nested Types To expose other resource types' constructors on a resource class (equivalent to `JSG_NESTED_TYPE` and `JSG_NESTED_TYPE_NAMED` in C++), list them in `nested_types(...)` on the `#[jsg_resource]` impl block: diff --git a/src/rust/jsg/fast_api.rs b/src/rust/jsg/fast_api.rs index 0c5e806fc62..f5f0aa3017b 100644 --- a/src/rust/jsg/fast_api.rs +++ b/src/rust/jsg/fast_api.rs @@ -272,6 +272,9 @@ unsafe fn run( /// (see [`Error::is_pending`]). The throw functions throw nothing while execution is /// terminating. unsafe fn throw(options: *mut ffi::FastApiCallbackOptions, err: &Error) -> T { + // A fast callback fails only in its receiver and argument checks, with a `TypeError`; the + // shim cannot create a `DOMException`. + debug_assert!(err.dom_exception_name().is_none(), "{err}"); // SAFETY: `options` comes from V8 (caller contract). unsafe { if err.is_internal { diff --git a/src/rust/jsg/ffi.c++ b/src/rust/jsg/ffi.c++ index e5a68a7742b..f4229f3862e 100644 --- a/src/rust/jsg/ffi.c++ +++ b/src/rust/jsg/ffi.c++ @@ -11,6 +11,7 @@ #include #include #include +#include #include @@ -804,6 +805,17 @@ Local wrappable_attach_opaque_wrapper(Isolate* isolate, kj::Rc wrappa // Unwrappers +namespace { +::rust::String rustStringFromV8(Isolate* isolate, v8::Local string) { + v8::String::ValueView view(isolate, string); + if (!view.is_one_byte()) { + // lossy(): lone surrogates become U+FFFD instead of throwing (which aborts). + return ::rust::String::lossy(reinterpret_cast(view.data16()), view.length()); + } + return ::rust::String::latin1(reinterpret_cast(view.data8()), view.length()); +} +} // namespace + bool unwrap_string(Isolate* isolate, Local value, ::rust::String& result) { auto v8Value = local_from_ffi(kj::mv(value)); // Fast path: a string primitive needs no coercion and can't throw. (String wrapper objects are @@ -814,13 +826,7 @@ bool unwrap_string(Isolate* isolate, Local value, ::rust::String& result) { } else if (!v8Value->ToString(isolate->GetCurrentContext()).ToLocal(&v8Str)) { return false; } - v8::String::ValueView view(isolate, v8Str); - if (!view.is_one_byte()) { - // lossy(): lone surrogates become U+FFFD instead of throwing (which aborts). - result = ::rust::String::lossy(reinterpret_cast(view.data16()), view.length()); - } else { - result = ::rust::String::latin1(reinterpret_cast(view.data8()), view.length()); - } + result = rustStringFromV8(isolate, v8Str); return true; } @@ -1515,8 +1521,6 @@ Local exceptionCreateFromMessage(ExceptionType exception_type, v8::Local(::workerd::jsg::newDOMException(isolate, + isolate->GetCurrentContext(), + newUtf8String(isolate, kj::arrayPtr(name.data(), name.size()), v8::NewStringType::kNormal), + newUtf8String( + isolate, kj::arrayPtr(message.data(), message.size()), v8::NewStringType::kNormal)))); +} + +bool local_try_unwrap_dom_exception( + Isolate* isolate, Local value, ::rust::String& name, ::rust::String& message) { + KJ_IF_SOME(fields, + ::workerd::jsg::tryGetDOMExceptionFields(isolate, local_from_ffi(kj::mv(value)))) { + name = rustStringFromV8(isolate, fields.name); + message = rustStringFromV8(isolate, fields.message); + return true; + } + return false; +} + namespace { // Mirrors makeInternalError() from util.c++, which it calls: generates a unique error ID, logs // the internal message (with ID) to KJ_LOG(ERROR) for Sentry, and returns a generic @@ -1599,6 +1622,15 @@ void isolate_throw_internal_error(Isolate* isolate, ::rust::Str internalMessage) isolate->ThrowException(createInternalError(isolate, internalMessage)); } +::rust::String log_internal_error(::rust::Str internalMessage) { + // Logs as makeInternalError() does, for an internal error that is thrown as a DOMException. + auto message = kj::heapString(internalMessage.data(), internalMessage.size()); + auto id = ::workerd::makeInternalErrorId(); + KJ_LOG(ERROR, message, id); + auto redacted = kj::str("internal error; reference = ", id); + return ::rust::String(redacted.begin(), redacted.size()); +} + void isolate_terminate_execution(Isolate* isolate) { ::workerd::jsg::IsolateBase::from(isolate).terminateExecution(); } diff --git a/src/rust/jsg/ffi.h b/src/rust/jsg/ffi.h index bc7b8c96c6e..8536fca55be 100644 --- a/src/rust/jsg/ffi.h +++ b/src/rust/jsg/ffi.h @@ -485,11 +485,19 @@ bool local_serialize(Isolate* isolate, MaybeLocal value_deserialize(Isolate* isolate, ::rust::Slice data); // Errors -// Builds an exception whose message is raw bytes interpreted as UTF-8. Invalid -// UTF-8 sequences are replaced with U+FFFD by v8::String::NewFromUtf8 rather -// than rejected, so this accepts arbitrary byte input (e.g. filesystem paths). +// Builds a JS built-in error whose message is raw bytes interpreted as UTF-8. +// Invalid UTF-8 sequences are replaced with U+FFFD rather than rejected, so this +// accepts arbitrary byte input (e.g. filesystem paths). Rust creates the +// DOM-prefixed exception types with dom_exception_create() instead. Local exception_create_from_bytes( Isolate* isolate, ExceptionType exception_type, const uint8_t* data, int32_t length); +// Creates a DOMException without running JavaScript (jsg::newDOMException()), or returns none +// with an exception pending. +MaybeLocal dom_exception_create(Isolate* isolate, ::rust::Str name, ::rust::Str message); +// Stores the name and message of a DOMException, by its brand (jsg::tryGetDOMExceptionFields()), +// and returns true; returns false for any other value. Runs no JavaScript. +bool local_try_unwrap_dom_exception( + Isolate* isolate, Local value, ::rust::String& name, ::rust::String& message); // The exception `isolate_throw_internal_error` throws, without throwing it: logs // `internalMessage` with a new reference ID and returns a generic error carrying only the ID. Local exception_create_internal(Isolate* isolate, ::rust::Str internalMessage); @@ -510,6 +518,7 @@ void isolate_try_catch( void isolate_throw_exception(Isolate* isolate, Local exception); void isolate_throw_error(Isolate* isolate, ::rust::Str message); void isolate_throw_internal_error(Isolate* isolate, ::rust::Str internalMessage); +::rust::String log_internal_error(::rust::Str internalMessage); void isolate_terminate_execution(Isolate* isolate); bool isolate_is_execution_terminating(Isolate* isolate); bool isolate_has_pending_exception(Isolate* isolate); diff --git a/src/rust/jsg/lib-test.rs b/src/rust/jsg/lib-test.rs index 19e571269ba..6ba45e3eba0 100644 --- a/src/rust/jsg/lib-test.rs +++ b/src/rust/jsg/lib-test.rs @@ -43,8 +43,8 @@ fn broken_prefix() { #[test] fn remote_prefix() { - let err = from_description("remote.jsg.AbortError: boom"); - assert_eq!(err.name, ExceptionType::AbortError); + let err = from_description("remote.jsg.DOMException(AbortError): boom"); + assert_eq!(err.name, ExceptionType::DOMAbortError); assert_eq!(err.message, "boom"); assert!(!err.is_internal); } @@ -60,7 +60,7 @@ fn remote_exception_prefix() { #[test] fn dom_exception() { let err = from_description("jsg.DOMException(AbortError): boom"); - assert_eq!(err.name, ExceptionType::AbortError); + assert_eq!(err.name, ExceptionType::DOMAbortError); assert_eq!(err.message, "boom"); assert!(!err.is_internal); } @@ -68,7 +68,7 @@ fn dom_exception() { #[test] fn dom_exception_with_expected_prefix() { let err = from_description("expected someCondition; jsg.DOMException(AbortError): boom"); - assert_eq!(err.name, ExceptionType::AbortError); + assert_eq!(err.name, ExceptionType::DOMAbortError); assert_eq!(err.message, "boom"); assert!(!err.is_internal); } @@ -87,7 +87,7 @@ fn jsg_internal_prefix_preserves_type_but_is_redacted() { #[test] fn jsg_internal_dom_exception_is_redacted() { let err = from_description("jsg-internal.DOMException(OperationError): boom"); - assert_eq!(err.name, ExceptionType::OperationError); + assert_eq!(err.name, ExceptionType::DOMOperationError); assert_eq!(err.message, "boom"); assert!(err.is_internal); } @@ -108,3 +108,75 @@ fn expected_prefix_without_tunneling_tag_is_redacted() { assert_eq!(err.message, "expected someCondition; some message"); assert!(err.is_internal); } + +#[test] +fn dom_syntax_error_is_distinct_from_js_syntax_error() { + let dom = from_description("jsg.DOMException(SyntaxError): boom"); + assert_eq!(dom.name, ExceptionType::DOMSyntaxError); + let js = from_description("jsg.SyntaxError: boom"); + assert_eq!(js.name, ExceptionType::SyntaxError); +} + +#[test] +fn dom_exception_without_dom_type_keeps_its_name() { + let err = from_description("jsg.DOMException(IndexSizeError): boom"); + assert_eq!(err.name, ExceptionType::Error); + assert_eq!(err.dom_exception_name(), Some("IndexSizeError")); + assert_eq!(err.message, "boom"); + assert!(!err.is_internal); +} + +/// Every `ExceptionType` variant, found through the variant name the cxx shared +/// enum's `Debug` prints (`ExceptionType()` for values that are not one). +/// This depends on that `Debug` format; it lets the test below check +/// `ExceptionType::DOM_EXCEPTION_NAMES` against the enum without listing the +/// variants a second time. +fn all_exception_types() -> Vec<(ExceptionType, String)> { + (0..=u8::MAX) + .map(|repr| ExceptionType { repr }) + .map(|ty| (ty, format!("{ty:?}"))) + .filter(|(_, variant)| !variant.starts_with("ExceptionType(")) + .collect() +} + +#[test] +fn dom_prefixed_variants_are_exactly_the_dom_exception_types() { + let types = all_exception_types(); + assert_eq!(types.len(), 18); + for (ty, variant) in types { + assert_eq!( + ty.dom_exception_name(), + variant.strip_prefix("DOM"), + "{variant}" + ); + } +} + +#[test] +fn every_type_round_trips_through_a_tunneled_description() { + for (ty, variant) in all_exception_types() { + let err = from_description(&format!("jsg.{ty}: boom")); + assert_eq!(err.name, ty, "{variant}"); + assert_eq!(err.message, "boom", "{variant}"); + } +} + +#[test] +fn dom_exception_names_round_trip() { + for (ty, name) in ExceptionType::DOM_EXCEPTION_NAMES { + assert_eq!(ty.dom_exception_name(), Some(name)); + assert_eq!(ExceptionType::from_dom_exception_name(name), Some(ty)); + assert_eq!(ty.to_string(), format!("DOMException({name})")); + } + assert_eq!(ExceptionType::SyntaxError.dom_exception_name(), None); + assert_eq!(ExceptionType::from_dom_exception_name("TypeError"), None); +} + +#[test] +fn dom_names_are_not_js_error_names() { + assert_eq!(ExceptionType::from("AbortError"), ExceptionType::Error); + assert_eq!( + ExceptionType::from("SyntaxError"), + ExceptionType::SyntaxError + ); +} diff --git a/src/rust/jsg/lib.rs b/src/rust/jsg/lib.rs index b5014272c90..7a898b8b1f0 100644 --- a/src/rust/jsg/lib.rs +++ b/src/rust/jsg/lib.rs @@ -131,29 +131,61 @@ mod ffi { pub type Result = std::result::Result; +/// Maps a JS error class name (`TypeError`, `SyntaxError`, ...) to its type. +/// Unrecognized names, including `DOMException` names, map to `Error`; use +/// [`ExceptionType::from_dom_exception_name`] for those. impl From<&str> for ExceptionType { fn from(value: &str) -> Self { match value { - "OperationError" => Self::OperationError, - "DataError" => Self::DataError, - "DataCloneError" => Self::DataCloneError, - "InvalidAccessError" => Self::InvalidAccessError, - "InvalidStateError" => Self::InvalidStateError, - "InvalidCharacterError" => Self::InvalidCharacterError, - "NotSupportedError" => Self::NotSupportedError, - "SyntaxError" => Self::SyntaxError, - "TimeoutError" => Self::TimeoutError, - "TypeMismatchError" => Self::TypeMismatchError, - "AbortError" => Self::AbortError, - "NotFoundError" => Self::NotFoundError, "TypeError" => Self::TypeError, "RangeError" => Self::RangeError, "ReferenceError" => Self::ReferenceError, + "SyntaxError" => Self::SyntaxError, _ => Self::Error, } } } +impl ExceptionType { + /// The DOM-prefixed types with the `DOMException` name each is thrown with. + const DOM_EXCEPTION_NAMES: [(Self, &'static str); 13] = [ + (Self::DOMOperationError, "OperationError"), + (Self::DOMDataError, "DataError"), + (Self::DOMDataCloneError, "DataCloneError"), + (Self::DOMInvalidAccessError, "InvalidAccessError"), + (Self::DOMInvalidStateError, "InvalidStateError"), + (Self::DOMInvalidCharacterError, "InvalidCharacterError"), + (Self::DOMNotSupportedError, "NotSupportedError"), + (Self::DOMSyntaxError, "SyntaxError"), + (Self::DOMTimeoutError, "TimeoutError"), + (Self::DOMTypeMismatchError, "TypeMismatchError"), + (Self::DOMQuotaExceededError, "QuotaExceededError"), + (Self::DOMAbortError, "AbortError"), + (Self::DOMNotFoundError, "NotFoundError"), + ]; + + /// Returns the `DOMException` name this type is thrown with, or `None` for + /// a JS error class. + #[must_use] + pub fn dom_exception_name(self) -> Option<&'static str> { + Self::DOM_EXCEPTION_NAMES + .iter() + .find(|(ty, _)| *ty == self) + .map(|(_, name)| *name) + } + + /// Maps a `DOMException` name to its DOM-prefixed type, or `None` for a + /// name without one (such as the legacy `IndexSizeError` or a name user + /// code chose); see [`Error::new_dom_exception`] for those. + #[must_use] + pub fn from_dom_exception_name(name: &str) -> Option { + Self::DOM_EXCEPTION_NAMES + .iter() + .find(|(_, dom_name)| *dom_name == name) + .map(|(ty, _)| *ty) + } +} + #[derive(Debug, Clone)] pub struct Error { pub name: ExceptionType, @@ -172,11 +204,18 @@ pub struct Error { /// Whether this error stands for an exception or termination that is pending in V8; see /// [`Error::is_pending`]. pending: bool, + /// The name of a `DOMException` that no DOM-prefixed [`ExceptionType`] stands for, such as + /// the legacy `IndexSizeError` or a name that user code chose. `name` is then + /// [`ExceptionType::Error`]. See [`Error::dom_exception_name`]. + other_dom_exception_name: Option>, } impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}: {}", self.name, self.message) + match &self.other_dom_exception_name { + Some(name) => write!(f, "DOMException({name}): {}", self.message), + None => write!(f, "{}: {}", self.name, self.message), + } } } @@ -192,6 +231,7 @@ macro_rules! impl_error_constructors { message: message.into(), is_internal: false, pending: false, + other_dom_exception_name: None, } } )* @@ -200,22 +240,24 @@ macro_rules! impl_error_constructors { } impl_error_constructors! { - OperationError => new_operation_error, - DataError => new_data_error, - DataCloneError => new_data_clone_error, - InvalidAccessError => new_invalid_access_error, - InvalidStateError => new_invalid_state_error, - InvalidCharacterError => new_invalid_character_error, - NotSupportedError => new_not_supported_error, - SyntaxError => new_syntax_error, - TimeoutError => new_timeout_error, - TypeMismatchError => new_type_mismatch_error, - AbortError => new_abort_error, - NotFoundError => new_not_found_error, + DOMOperationError => new_dom_operation_error, + DOMDataError => new_dom_data_error, + DOMDataCloneError => new_dom_data_clone_error, + DOMInvalidAccessError => new_dom_invalid_access_error, + DOMInvalidStateError => new_dom_invalid_state_error, + DOMInvalidCharacterError => new_dom_invalid_character_error, + DOMNotSupportedError => new_dom_not_supported_error, + DOMSyntaxError => new_dom_syntax_error, + DOMTimeoutError => new_dom_timeout_error, + DOMTypeMismatchError => new_dom_type_mismatch_error, + DOMQuotaExceededError => new_dom_quota_exceeded_error, + DOMAbortError => new_dom_abort_error, + DOMNotFoundError => new_dom_not_found_error, TypeError => new_type_error, Error => new_error, RangeError => new_range_error, ReferenceError => new_reference_error, + SyntaxError => new_syntax_error, } impl<'s> FromJS<'s> for Error { @@ -223,12 +265,15 @@ impl<'s> FromJS<'s> for Error { /// Creates an Error from a V8 value (typically an exception). /// - /// If the value is a native error, extracts the name and message properties. - /// Otherwise, converts the value to a string for the message. + /// A native error contributes its `name` and `message` properties. A + /// `DOMException` contributes the name and message it was created with, + /// as [`Error::new_dom_exception`] takes them. Any other value is rejected + /// with a `TypeError`. /// /// Reading `name` and `message` can run JavaScript (a getter, a Proxy trap, or `toString`). /// If it throws, or execution is terminating, this fails with an error for which /// [`Error::is_pending`] is true. + #[expect(unsafe_code, reason = "unwraps a DOMException through the FFI")] fn from_js( lock: &mut Lock<'s>, value: v8::Local<'s, v8::Value>, @@ -250,15 +295,34 @@ impl<'s> FromJS<'s> for Error { String::from_js(lock, message)? }; - Ok(Self { + return Ok(Self { name: name.map_or(ExceptionType::Error, |n| ExceptionType::from(n.as_str())), message, is_internal: false, pending: false, - }) - } else { - Err(Self::new_type_error("Unknown error")) + other_dom_exception_name: None, + }); } + + // A DOMException is not a native error, though its prototype chain includes + // Error.prototype; it is recognized by its brand. + let mut dom_name = String::new(); + let mut dom_message = String::new(); + // SAFETY: Lock guarantees the isolate is valid and locked, and `value` + // belongs to it. + let is_dom_exception = unsafe { + v8::ffi::local_try_unwrap_dom_exception( + lock.isolate().as_ffi(), + value.into_ffi(), + &mut dom_name, + &mut dom_message, + ) + }?; + if is_dom_exception { + return Ok(Self::new_dom_exception(&dom_name, dom_message)); + } + + Err(Self::new_type_error("Unknown error")) } } @@ -269,6 +333,7 @@ impl Error { message: message.to_owned(), is_internal: false, pending: false, + other_dom_exception_name: None, } } @@ -286,9 +351,35 @@ impl Error { message: message.into(), is_internal: false, pending: false, + other_dom_exception_name: None, } } + /// Creates an `Error` that is thrown as a `DOMException` with `name`, as C++ + /// `jsg::Lock::domException()` does. A name with a DOM-prefixed [`ExceptionType`] gets that + /// type (`"AbortError"` gets [`ExceptionType::DOMAbortError`]); any other name, such as the + /// legacy `"IndexSizeError"`, gets [`ExceptionType::Error`] and is kept for + /// [`Error::dom_exception_name`]. + pub fn new_dom_exception(name: &str, message: impl Into) -> Self { + let message = message.into(); + match ExceptionType::from_dom_exception_name(name) { + Some(ty) => Self::from_type(ty, message), + None => Self { + other_dom_exception_name: Some(name.into()), + ..Self::from_type(ExceptionType::Error, message) + }, + } + } + + /// Returns the name of the `DOMException` this error is thrown as, or `None` if it is thrown + /// as a JS built-in error. + #[must_use] + pub fn dom_exception_name(&self) -> Option<&str> { + self.name + .dom_exception_name() + .or(self.other_dom_exception_name.as_deref()) + } + /// Creates an `Error` for a call into JavaScript that threw, or that execution terminating /// interrupted, leaving the exception pending in V8; see [`Error::is_pending`]. pub(crate) fn pending() -> Self { @@ -319,26 +410,44 @@ impl Error { /// This always builds the exception from `self.message` verbatim: it does /// not check `is_internal` / redact anything. Prefer `Lock::throw_exception()`, /// which does perform that check before falling back to this method. + /// + /// # Errors + /// + /// Creating a `DOMException` can fail with an exception pending, such as a stack overflow. + /// Then this returns an error for which [`Error::is_pending`] is true. It also returns such + /// a pending error, and creates nothing, while execution is terminating, because capturing + /// the `DOMException`'s stack trace then fails a V8 debug check. #[expect( unsafe_code, reason = "creates the V8 exception object through the FFI" )] - pub fn to_local<'s>(&self, lock: &mut Lock<'s>) -> v8::Local<'s, v8::Value> { + pub fn to_local<'s>(&self, lock: &mut Lock<'s>) -> Result> { let isolate = lock.isolate(); + if let Some(name) = self.dom_exception_name() { + // SAFETY: isolate is valid and locked (guaranteed by caller). + if unsafe { v8::ffi::isolate_is_execution_terminating(isolate.as_ffi()) } { + return Err(Self::pending()); + } + // SAFETY: isolate is valid and locked (guaranteed by caller). + let exception = + unsafe { v8::ffi::dom_exception_create(isolate.as_ffi(), name, &self.message) }?; + // SAFETY: the exception was created in the caller's handle scope. + return unsafe { v8::Local::from_ffi_maybe(exception) }.ok_or_else(Self::pending); + } let message = self.message.as_bytes(); // Strings longer than i32::MAX bytes are truncated to satisfy V8's // int-typed length parameter; V8 rejects strings that large anyway. let len = i32::try_from(message.len()).unwrap_or(i32::MAX); // SAFETY: isolate is valid and locked (guaranteed by caller); message // pointer and len describe a valid byte slice. - unsafe { + Ok(unsafe { v8::Local::from_ffi(v8::ffi::exception_create_from_bytes( isolate.as_ffi(), self.name, message.as_ptr(), len, )) - } + }) } } @@ -440,10 +549,8 @@ impl Error { && let Some((name, message)) = rest.split_once("): ") { return Some(Self { - name: ExceptionType::from(name), - message: message.to_owned(), is_internal, - pending: false, + ..Self::new_dom_exception(name, message) }); } @@ -453,6 +560,7 @@ impl Error { message: message.to_owned(), is_internal, pending: false, + other_dom_exception_name: None, }) } @@ -468,6 +576,7 @@ impl Error { message: message.into(), is_internal: true, pending: false, + other_dom_exception_name: None, } } } @@ -845,7 +954,11 @@ impl<'s> Lock<'s> { /// exception pending, it is thrown as an internal error. Otherwise, if /// `err.is_internal` is set, the message is redacted (via /// `throw_internal_error()`) rather than thrown verbatim, matching how - /// C++ `decodeTunneledException()` handles `isInternal` KJ exceptions. + /// C++ `decodeTunneledException()` handles `isInternal` KJ exceptions; an + /// internal error with a `DOMException` name is thrown as a `DOMException` + /// with the redacted message, as in C++. If creating the exception fails + /// with an exception pending in V8, that exception propagates; any other + /// failure is thrown as an internal error. pub fn throw_exception(&mut self, err: &Error) { if err.is_pending() { let isolate = self.isolate().as_ffi(); @@ -865,7 +978,12 @@ impl<'s> Lock<'s> { return; } if err.is_internal { - self.throw_internal_error(&err.message); + if let Some(name) = err.dom_exception_name() { + let message = v8::ffi::log_internal_error(&err.message); + self.throw_exception(&Error::new_dom_exception(name, message)); + } else { + self.throw_internal_error(&err.message); + } return; } if self.is_execution_terminating() { @@ -873,12 +991,14 @@ impl<'s> Lock<'s> { // execution is terminating fails a V8 debug check. return; } - // SAFETY: isolate is valid and locked (guaranteed by Lock). - unsafe { - v8::ffi::isolate_throw_exception( - self.isolate().as_ffi(), - err.to_local(self).into_ffi(), - ); + match err.to_local(self) { + // SAFETY: isolate is valid and locked (guaranteed by Lock). + Ok(exception) => unsafe { + v8::ffi::isolate_throw_exception(self.isolate().as_ffi(), exception.into_ffi()); + }, + // Creating the exception threw, and what it threw propagates instead. + Err(failure) if failure.is_pending() => {} + Err(failure) => self.throw_internal_error(&failure.message), } } @@ -998,6 +1118,10 @@ impl<'s> Lock<'s> { return Err(Error::pending()); } if err.is_internal { + if let Some(name) = err.dom_exception_name() { + let message = v8::ffi::log_internal_error(&err.message); + return Error::new_dom_exception(name, message).to_local(self); + } // SAFETY: isolate is valid and locked (guaranteed by Lock); the exception lives in // the lock's scope. return Ok(unsafe { @@ -1007,7 +1131,7 @@ impl<'s> Lock<'s> { )) }); } - Ok(err.to_local(self)) + err.to_local(self) } /// Throws an internal error as a V8 exception, matching `makeInternalError()` in C++. diff --git a/src/rust/jsg/macros.rs b/src/rust/jsg/macros.rs index 0b6609e19e8..8250a15bd05 100644 --- a/src/rust/jsg/macros.rs +++ b/src/rust/jsg/macros.rs @@ -47,11 +47,10 @@ macro_rules! jsg_traced { /// - `condition` — Any expression that evaluates to `bool`. When `true`, the macro is a no-op. /// When `false`, an error is returned. /// - `ExceptionVariant` — One of the [`ExceptionType`](crate::v8::ffi::ExceptionType) variants -/// that determines the JavaScript error class thrown to user code. Available variants: -/// `TypeError`, `RangeError`, `ReferenceError`, `SyntaxError`, `Error`, -/// `OperationError`, `DataError`, `DataCloneError`, `InvalidAccessError`, -/// `InvalidStateError`, `InvalidCharacterError`, `NotSupportedError`, -/// `TimeoutError`, `TypeMismatchError`, `AbortError`, `NotFoundError`. +/// that determines the JavaScript error class thrown to user code: one of the JS +/// error classes `TypeError`, `RangeError`, `ReferenceError`, `SyntaxError` and +/// `Error`, or a `DOMException` type such as `DOMSyntaxError` or `DOMAbortError`, +/// named as in C++ `JSG_REQUIRE`. /// - `"message"` / `"format string", args...` — A message string, optionally with `format!`-style /// arguments. Unlike the C++ `JSG_REQUIRE` which concatenates via `kj::str()`, this macro uses /// Rust's standard `format!()` for string interpolation. @@ -113,11 +112,10 @@ macro_rules! jsg_require { /// # Parameters /// /// - `ExceptionVariant` — One of the [`ExceptionType`](crate::v8::ffi::ExceptionType) variants -/// that determines the JavaScript error class thrown to user code. Available variants: -/// `TypeError`, `RangeError`, `ReferenceError`, `SyntaxError`, `Error`, -/// `OperationError`, `DataError`, `DataCloneError`, `InvalidAccessError`, -/// `InvalidStateError`, `InvalidCharacterError`, `NotSupportedError`, -/// `TimeoutError`, `TypeMismatchError`, `AbortError`, `NotFoundError`. +/// that determines the JavaScript error class thrown to user code: one of the JS +/// error classes `TypeError`, `RangeError`, `ReferenceError`, `SyntaxError` and +/// `Error`, or a `DOMException` type such as `DOMSyntaxError` or `DOMAbortError`, +/// named as in C++ `JSG_REQUIRE`. /// - `"message"` / `"format string", args...` — A message string, optionally with `format!`-style /// arguments. Unlike the C++ `JSG_FAIL_REQUIRE` which concatenates via `kj::str()`, this macro /// uses Rust's standard `format!()` for string interpolation. @@ -133,7 +131,7 @@ macro_rules! jsg_require { /// use jsg::{jsg_fail_require, Result}; /// /// fn unsupported_algorithm(name: &str) -> Result<()> { -/// jsg_fail_require!(NotSupportedError, "algorithm '{}' is not supported", name); +/// jsg_fail_require!(DOMNotSupportedError, "algorithm '{}' is not supported", name); /// } /// /// fn validate_input(mode: &str) -> Result { diff --git a/src/rust/jsg/ser.c++ b/src/rust/jsg/ser.c++ index b4ce1f489b8..b765e26399f 100644 --- a/src/rust/jsg/ser.c++ +++ b/src/rust/jsg/ser.c++ @@ -116,12 +116,15 @@ class Serializer final: public v8::ValueSerializer::Delegate { } bool HasCustomHostObject(v8::Isolate*) override { - // V8 already treats objects with internal fields, such as API objects, as host objects. - return errorsAreHostObjects; + // V8 treats only objects with internal fields, such as API objects, as host objects. + return true; } v8::Maybe IsHostObject(v8::Isolate*, v8::Local object) override { - return v8::Just(object->InternalFieldCount() > 0 || object->IsNativeError()); + // A DOMException is written by its brand, which jsg::writeApiObject() checks. + return v8::Just(object->InternalFieldCount() > 0 || + ::workerd::jsg::tryGetDOMExceptionFields(isolate, object) != kj::none || + (errorsAreHostObjects && object->IsNativeError())); } v8::Maybe WriteHostObject(v8::Isolate*, v8::Local object) override { diff --git a/src/rust/jsg/v8.rs b/src/rust/jsg/v8.rs index 86f57f72a20..77da47d7e10 100644 --- a/src/rust/jsg/v8.rs +++ b/src/rust/jsg/v8.rs @@ -114,24 +114,33 @@ pub mod ffi { Uninitialized = 1, } + /// The JavaScript error class a [`crate::Error`] is thrown as. + /// + /// The `DOM`-prefixed variants are thrown as a `DOMException` with the + /// unprefixed name, mirroring the C++ `JSG_REQUIRE(..., DOMSyntaxError, ...)` + /// error types (`src/workerd/jsg/exception.h`). The others are the JS + /// built-in error classes, so `SyntaxError` and `DOMSyntaxError` are + /// distinct. #[derive(Debug, PartialEq, Eq, Copy, Clone)] pub enum ExceptionType { - OperationError, - DataError, - DataCloneError, - InvalidAccessError, - InvalidStateError, - InvalidCharacterError, - NotSupportedError, - SyntaxError, - TimeoutError, - TypeMismatchError, - AbortError, - NotFoundError, + DOMOperationError, + DOMDataError, + DOMDataCloneError, + DOMInvalidAccessError, + DOMInvalidStateError, + DOMInvalidCharacterError, + DOMNotSupportedError, + DOMSyntaxError, + DOMTimeoutError, + DOMTypeMismatchError, + DOMQuotaExceededError, + DOMAbortError, + DOMNotFoundError, TypeError, Error, RangeError, ReferenceError, + SyntaxError, } /// Mirrors `v8::Promise::PromiseState`; `ffi.c++` asserts the values match. @@ -607,6 +616,9 @@ pub mod ffi { options: *mut FastApiCallbackOptions, receiver: Local, ) -> TraitObjectPtr; + /// Throws a JS built-in error; `exception_type` must not be DOM-prefixed. + /// Fast callbacks fail only in their receiver and argument checks, with a + /// `TypeError`. pub unsafe fn fast_api_throw_exception( options: *mut FastApiCallbackOptions, exception_type: ExceptionType, @@ -632,12 +644,30 @@ pub mod ffi { pub unsafe fn value_deserialize(isolate: *mut Isolate, data: &[u8]) -> Result; // Errors + /// Creates a JS built-in error; `exception_type` must not be DOM-prefixed. pub unsafe fn exception_create_from_bytes( isolate: *mut Isolate, exception_type: ExceptionType, data: *const u8, length: i32, ) -> Local; + /// Creates a `DOMException` with the given name, running no JavaScript, or returns + /// none with an exception pending. Fallible: the context must have a DOMException + /// prototype, which every JSG context has. + pub unsafe fn dom_exception_create( + isolate: *mut Isolate, + name: &str, + message: &str, + ) -> Result; + /// If `value` is a `DOMException`, stores the name and message it was created with + /// and returns `true`; otherwise leaves them untouched and returns `false`. Runs no + /// JavaScript. Fallible: it constructs `rust::String`s. + pub unsafe fn local_try_unwrap_dom_exception( + isolate: *mut Isolate, + value: Local, + name: &mut String, + message: &mut String, + ) -> Result; pub unsafe fn exception_create_internal( isolate: *mut Isolate, internal_message: &str, @@ -655,6 +685,9 @@ pub mod ffi { pub unsafe fn isolate_throw_exception(isolate: *mut Isolate, exception: Local); pub unsafe fn isolate_throw_error(isolate: *mut Isolate, message: &str); pub unsafe fn isolate_throw_internal_error(isolate: *mut Isolate, internal_message: &str); + /// Logs `internal_message` as `isolate_throw_internal_error` does, and returns the + /// redacted message, `internal error; reference = `, to throw instead. + pub fn log_internal_error(internal_message: &str) -> String; pub unsafe fn isolate_terminate_execution(isolate: *mut Isolate); pub unsafe fn isolate_is_execution_terminating(isolate: *mut Isolate) -> bool; pub unsafe fn isolate_has_pending_exception(isolate: *mut Isolate) -> bool; @@ -829,24 +862,19 @@ pub mod ffi { } } +/// Formats the type the way a tunneled KJ exception description names it: +/// `TypeError` for a JS error class, `DOMException(SyntaxError)` for a +/// `DOMException`. impl std::fmt::Display for ffi::ExceptionType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(name) = self.dom_exception_name() { + return write!(f, "DOMException({name})"); + } let name = match *self { - Self::OperationError => "OperationError", - Self::DataError => "DataError", - Self::DataCloneError => "DataCloneError", - Self::InvalidAccessError => "InvalidAccessError", - Self::InvalidStateError => "InvalidStateError", - Self::InvalidCharacterError => "InvalidCharacterError", - Self::NotSupportedError => "NotSupportedError", - Self::SyntaxError => "SyntaxError", - Self::TimeoutError => "TimeoutError", - Self::TypeMismatchError => "TypeMismatchError", - Self::AbortError => "AbortError", - Self::NotFoundError => "NotFoundError", Self::TypeError => "TypeError", Self::RangeError => "RangeError", Self::ReferenceError => "ReferenceError", + Self::SyntaxError => "SyntaxError", _ => "Error", }; write!(f, "{name}") diff --git a/src/workerd/api/eventsource.c++ b/src/workerd/api/eventsource.c++ index 1774ef49f22..c5e6351441e 100644 --- a/src/workerd/api/eventsource.c++ +++ b/src/workerd/api/eventsource.c++ @@ -385,9 +385,7 @@ void EventSource::start(jsg::Lock& js) { auto fetcher = i.options.fetcher.map([](jsg::Ref& f) { return f.addRef(); }); static constexpr auto handleError = [](auto& js, auto& self, kj::String message) { - auto ex = js.domException(kj::str("AbortError"), kj::mv(message)); - auto handle = KJ_ASSERT_NONNULL(ex.tryGetHandle(js)); - self->notifyError(js, jsg::JsValue(handle)); + self->notifyError(js, js.domException(kj::str("AbortError"), kj::mv(message))); return js.resolvedPromise(); }; diff --git a/src/workerd/api/filesystem-bootstrap.c++ b/src/workerd/api/filesystem-bootstrap.c++ index 789fb4a4dcf..30f0cc4bbc1 100644 --- a/src/workerd/api/filesystem-bootstrap.c++ +++ b/src/workerd/api/filesystem-bootstrap.c++ @@ -18,8 +18,6 @@ jsg::JsValue createFileSystemWriteContext( auto& contextHandler = KJ_ASSERT_NONNULL(js.tryGetTypeHandler>(), "FileSystemWriteContextHandle is missing from the isolate type list"); - auto& deHandler = KJ_ASSERT_NONNULL(js.tryGetTypeHandler>(), - "DOMException is missing from the isolate type list"); // The receiver reaches here as an ordinary argument because the TypeScript // createWritable() is a plain function rather than a JSG method, so nothing has @@ -30,10 +28,10 @@ jsg::JsValue createFileSystemWriteContext( auto opened = FileSystemWriteContextHandle::open( js, kj::mv(file), keepExistingData == KeepExistingData::YES); - KJ_IF_SOME(ex, opened.tryGet>()) { + KJ_IF_SOME(ex, opened.tryGet()) { // The C++ createWritable() reports these by rejecting; the TypeScript one // turns a throw into the same rejection, so the observable result matches. - js.throwException(jsg::JsValue(deHandler.wrap(js, kj::mv(ex)))); + js.throwException(ex); } return jsg::JsValue( diff --git a/src/workerd/api/filesystem.c++ b/src/workerd/api/filesystem.c++ index f16a2274a74..ec06aca504c 100644 --- a/src/workerd/api/filesystem.c++ +++ b/src/workerd/api/filesystem.c++ @@ -2065,7 +2065,7 @@ constexpr bool isValidFileName(kj::StringPtr name) { name.find("\\"_kj) == kj::none; } -jsg::Ref fsErrorToDomException(jsg::Lock& js, workerd::FsError error) { +jsg::JsObject fsErrorToDomException(jsg::Lock& js, workerd::FsError error) { switch (error) { case workerd::FsError::NOT_DIRECTORY: { return js.domException(kj::str("NotSupportedError"), kj::str("Not a directory")); @@ -2122,8 +2122,7 @@ FileSystemHandle::FileSystemHandle( locator(kj::mv(locator)), name(kj::mv(name)) {} -jsg::Promise FileSystemHandle::getUniqueId( - jsg::Lock& js, const jsg::TypeHandler>& deHandler) { +jsg::Promise FileSystemHandle::getUniqueId(jsg::Lock& js) { KJ_IF_SOME(item, vfs.resolve(js, getLocator(), {})) { KJ_SWITCH_ONEOF(item) { KJ_CASE_ONEOF(file, kj::Rc) { @@ -2136,14 +2135,13 @@ jsg::Promise FileSystemHandle::getUniqueId( return js.resolvedPromise(link->getUniqueId(js)); } KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise( - deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } KJ_UNREACHABLE; } auto ex = js.domException(kj::str("NotFoundError"), kj::str("The entry was not found.")); - return js.rejectedPromise(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } jsg::Promise FileSystemHandle::isSameEntry(jsg::Lock& js, jsg::Ref other) { @@ -2158,14 +2156,12 @@ jsg::Promise FileSystemHandle::isSameEntry(jsg::Lock& js, jsg::Ref FileSystemHandle::remove(jsg::Lock& js, - jsg::Optional options, - const jsg::TypeHandler>& deHandler) { +jsg::Promise FileSystemHandle::remove(jsg::Lock& js, jsg::Optional options) { if (!canBeModifiedCurrently(js)) { auto ex = js.domException(kj::str("NoModificationAllowedError"), kj::str("Cannot remove a handle that is not writable or not a directory.")); - return js.rejectedPromise(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } auto relative = getLocator().getRelative(jsg::Url::RelativeOption::STRIP_TAILING_SLASHES); @@ -2178,39 +2174,37 @@ jsg::Promise FileSystemHandle::remove(jsg::Lock& js, kj::Path path({name}); if (parentDir->stat(js, path) == kj::none) { auto ex = js.domException(kj::str("NotFoundError"), kj::str("The entry was not found.")); - return js.rejectedPromise(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } KJ_SWITCH_ONEOF(parentDir->remove(js, path, {.recursive = recursive})) { KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } KJ_CASE_ONEOF(removed, bool) { if (!removed) { auto ex = js.domException(kj::str("NotFoundError"), kj::str("The entry was not found.")); - return js.rejectedPromise(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } return js.resolvedPromise(); } } } KJ_CASE_ONEOF(_, kj::Rc) { - return js.rejectedPromise( - deHandler.wrap(js, fsErrorToDomException(js, workerd::FsError::NOT_DIRECTORY))); + return js.rejectedPromise(fsErrorToDomException(js, workerd::FsError::NOT_DIRECTORY)); } KJ_CASE_ONEOF(_, kj::Rc) { - return js.rejectedPromise( - deHandler.wrap(js, fsErrorToDomException(js, workerd::FsError::NOT_DIRECTORY))); + return js.rejectedPromise(fsErrorToDomException(js, workerd::FsError::NOT_DIRECTORY)); } KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } KJ_UNREACHABLE; } auto ex = js.domException(kj::str("NotFoundError"), kj::str("The entry was not found.")); - return js.rejectedPromise(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } bool FileSystemHandle::canBeModifiedCurrently(jsg::Lock& js) const { @@ -2223,8 +2217,7 @@ bool FileSystemHandle::canBeModifiedCurrently(jsg::Lock& js) const { return !getVfs().isLocked(js, getLocator()); } -jsg::Promise> StorageManager::getDirectory( - jsg::Lock& js, const jsg::TypeHandler>& exception) { +jsg::Promise> StorageManager::getDirectory(jsg::Lock& js) { auto& vfs = workerd::VirtualFileSystem::current(js); return js.resolvedPromise(js.alloc( vfs, KJ_ASSERT_NONNULL(jsg::Url::tryParse("file:///"_kj)), jsg::USVString())); @@ -2234,10 +2227,8 @@ FileSystemDirectoryHandle::FileSystemDirectoryHandle( const workerd::VirtualFileSystem& vfs, jsg::Url locator, jsg::USVString name) : FileSystemHandle(vfs, kj::mv(locator), kj::mv(name)) {} -jsg::Promise> FileSystemDirectoryHandle::getFileHandle(jsg::Lock& js, - jsg::USVString name, - jsg::Optional options, - const jsg::TypeHandler>& exception) { +jsg::Promise> FileSystemDirectoryHandle::getFileHandle( + jsg::Lock& js, jsg::USVString name, jsg::Optional options) { if (!isValidFileName(name)) { return js.rejectedPromise>(js.typeError("Invalid file name")); } @@ -2249,8 +2240,7 @@ jsg::Promise> FileSystemDirectoryHandle::getFileH KJ_IF_SOME(existing, getVfs().resolve(js, getLocator(), {})) { KJ_SWITCH_ONEOF(existing) { KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise>( - exception.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise>(fsErrorToDomException(js, err)); } KJ_CASE_ONEOF(dir, kj::Rc) { auto locator = KJ_ASSERT_NONNULL(getLocator().tryResolve(name)); @@ -2268,46 +2258,41 @@ jsg::Promise> FileSystemDirectoryHandle::getFileH KJ_CASE_ONEOF(dir, kj::Rc) { auto ex = js.domException( kj::str("TypeMismatchError"), kj::str("File name is a directory")); - return js.rejectedPromise>( - exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } KJ_CASE_ONEOF(link, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Not a file")); - return js.rejectedPromise>( - exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } KJ_CASE_ONEOF(err, workerd::FsError) { return js.rejectedPromise>( - exception.wrap(js, fsErrorToDomException(js, err))); + fsErrorToDomException(js, err)); } } KJ_UNREACHABLE; } auto ex = js.domException(kj::str("NotFoundError"), kj::str("Not found")); - return js.rejectedPromise>(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } KJ_CASE_ONEOF(file, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Not a directory")); - return js.rejectedPromise>(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } KJ_CASE_ONEOF(link, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Not a directory")); - return js.rejectedPromise>(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } } KJ_UNREACHABLE; } auto ex = js.domException(kj::str("NotFoundError"), kj::str("Directory not found")); - return js.rejectedPromise>(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } jsg::Promise> FileSystemDirectoryHandle::getDirectoryHandle( - jsg::Lock& js, - jsg::USVString name, - jsg::Optional options, - const jsg::TypeHandler>& exception) { + jsg::Lock& js, jsg::USVString name, jsg::Optional options) { if (!isValidFileName(name)) { return js.rejectedPromise>( js.typeError("Invalid directory name")); @@ -2322,7 +2307,7 @@ jsg::Promise> FileSystemDirectoryHandle::get KJ_SWITCH_ONEOF(existing) { KJ_CASE_ONEOF(err, workerd::FsError) { return js.rejectedPromise>( - exception.wrap(js, fsErrorToDomException(js, err))); + fsErrorToDomException(js, err)); } KJ_CASE_ONEOF(dir, kj::Rc) { auto locator = KJ_ASSERT_NONNULL(getLocator().tryResolve(name)); @@ -2341,17 +2326,15 @@ jsg::Promise> FileSystemDirectoryHandle::get KJ_CASE_ONEOF(dir, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("File name is a file")); - return js.rejectedPromise>( - exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } KJ_CASE_ONEOF(link, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Not a directory")); - return js.rejectedPromise>( - exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } KJ_CASE_ONEOF(err, workerd::FsError) { return js.rejectedPromise>( - exception.wrap(js, fsErrorToDomException(js, err))); + fsErrorToDomException(js, err)); } } KJ_UNREACHABLE; @@ -2359,31 +2342,26 @@ jsg::Promise> FileSystemDirectoryHandle::get // Could not open or create the directory. auto ex = js.domException(kj::str("NotFoundError"), kj::str("Directory not opened or created")); - return js.rejectedPromise>( - exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } KJ_CASE_ONEOF(file, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Not a directory")); - return js.rejectedPromise>( - exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } KJ_CASE_ONEOF(link, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Not a directory")); - return js.rejectedPromise>( - exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } } KJ_UNREACHABLE; } auto ex = js.domException(kj::str("NotFoundError"), kj::str("Directory not found")); - return js.rejectedPromise>(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } -jsg::Promise FileSystemDirectoryHandle::removeEntry(jsg::Lock& js, - jsg::USVString name, - jsg::Optional options, - const jsg::TypeHandler>& exception) { +jsg::Promise FileSystemDirectoryHandle::removeEntry( + jsg::Lock& js, jsg::USVString name, jsg::Optional options) { if (!isValidFileName(name)) { return js.rejectedPromise(js.typeError("Invalid name")); } @@ -2392,7 +2370,7 @@ jsg::Promise FileSystemDirectoryHandle::removeEntry(jsg::Lock& js, KJ_IF_SOME(existing, getVfs().resolve(js, getLocator(), {})) { KJ_SWITCH_ONEOF(existing) { KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise(exception.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } KJ_CASE_ONEOF(dir, kj::Rc) { kj::Path item({name}); @@ -2401,7 +2379,7 @@ jsg::Promise FileSystemDirectoryHandle::removeEntry(jsg::Lock& js, // If the file is locked, we cannot remove it. auto ex = js.domException(kj::str("NoModificationAllowedError"), kj::str("Cannot remove an entry that is currently locked.")); - return js.rejectedPromise(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } KJ_SWITCH_ONEOF(dir->remove(js, item, @@ -2414,28 +2392,28 @@ jsg::Promise FileSystemDirectoryHandle::removeEntry(jsg::Lock& js, } // If the entry was not found, we throw a NotFoundError. auto ex = js.domException(kj::str("NotFoundError"), kj::str("File not found")); - return js.rejectedPromise(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } KJ_CASE_ONEOF(error, workerd::FsError) { - return js.rejectedPromise(exception.wrap(js, fsErrorToDomException(js, error))); + return js.rejectedPromise(fsErrorToDomException(js, error)); } } KJ_UNREACHABLE; } KJ_CASE_ONEOF(file, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Not a directory")); - return js.rejectedPromise(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } KJ_CASE_ONEOF(link, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Not a directory")); - return js.rejectedPromise(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } } KJ_UNREACHABLE; } auto ex = js.domException(kj::str("NotFoundError"), kj::str("Not found")); - return js.rejectedPromise(exception.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } jsg::Promise> FileSystemDirectoryHandle::resolve( @@ -2528,8 +2506,7 @@ jsg::Ref FileSystemDirectoryHandle::en // we still return an iterator here but it needs to throw a NotFoundError when next // is actually called. auto ex = js.domException(kj::str("NotFoundError"), kj::str("Not found")); - auto handle = jsg::JsValue(KJ_ASSERT_NONNULL(ex.tryGetHandle(js))); - return js.alloc(IteratorState(jsg::JsRef(js, handle))); + return js.alloc(IteratorState(jsg::JsRef(js, jsg::JsValue(ex)))); } jsg::Ref FileSystemDirectoryHandle::keys(jsg::Lock& js) { @@ -2556,8 +2533,7 @@ jsg::Ref FileSystemDirectoryHandle::keys // we still return an iterator here but it needs to throw a NotFoundError when next // is actually called. auto ex = js.domException(kj::str("NotFoundError"), kj::str("Not found")); - auto handle = jsg::JsValue(KJ_ASSERT_NONNULL(ex.tryGetHandle(js))); - return js.alloc(IteratorState(jsg::JsRef(js, handle))); + return js.alloc(IteratorState(jsg::JsRef(js, jsg::JsValue(ex)))); } jsg::Ref FileSystemDirectoryHandle::values( @@ -2585,20 +2561,18 @@ jsg::Ref FileSystemDirectoryHandle::va // we still return an iterator here but it needs to throw a NotFoundError when next // is actually called. auto ex = js.domException(kj::str("NotFoundError"), kj::str("Not found")); - auto handle = jsg::JsValue(KJ_ASSERT_NONNULL(ex.tryGetHandle(js))); - return js.alloc(IteratorState(jsg::JsRef(js, handle))); + return js.alloc(IteratorState(jsg::JsRef(js, jsg::JsValue(ex)))); } void FileSystemDirectoryHandle::forEach(jsg::Lock& js, jsg::Function, jsg::Ref)> callback, - jsg::Optional thisArg, - const jsg::TypeHandler>& exception) { + jsg::Optional thisArg) { KJ_IF_SOME(existing, resolveDirectoryHandle(js, getVfs(), getLocator())) { KJ_SWITCH_ONEOF(existing) { KJ_CASE_ONEOF(err, workerd::FsError) { - js.throwException(js.v8Ref(exception.wrap(js, fsErrorToDomException(js, err)))); + js.throwException(fsErrorToDomException(js, err)); } KJ_CASE_ONEOF(dir, kj::Rc) { auto receiver = js.v8Undefined(); @@ -2632,8 +2606,7 @@ FileSystemFileHandle::FileSystemFileHandle( const workerd::VirtualFileSystem& vfs, jsg::Url locator, jsg::USVString name) : FileSystemHandle(vfs, kj::mv(locator), kj::mv(name)) {} -jsg::Promise> FileSystemFileHandle::getFile( - jsg::Lock& js, const jsg::TypeHandler>& deHandler) { +jsg::Promise> FileSystemFileHandle::getFile(jsg::Lock& js) { // TODO(node-fs): Currently this copies the file data into the new File object. // Alternatively, File/Blob can be modified to allow it to be backed by a // workerd::File such that it does not need to create a separate in-memory @@ -2643,8 +2616,7 @@ jsg::Promise> FileSystemFileHandle::getFile( KJ_IF_SOME(item, getVfs().resolve(js, getLocator(), {})) { KJ_SWITCH_ONEOF(item) { KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise>( - deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise>(fsErrorToDomException(js, err)); } KJ_CASE_ONEOF(file, kj::Rc) { auto stat = file->stat(js); @@ -2655,19 +2627,18 @@ jsg::Promise> FileSystemFileHandle::getFile( (stat.lastModified - kj::UNIX_EPOCH) / kj::MILLISECONDS)); } KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise>( - deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise>(fsErrorToDomException(js, err)); } } KJ_UNREACHABLE; } KJ_CASE_ONEOF(dir, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Is a directory")); - return js.rejectedPromise>(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } KJ_CASE_ONEOF(link, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Is a symbolic link")); - return js.rejectedPromise>(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } } KJ_UNREACHABLE; @@ -2675,22 +2646,20 @@ jsg::Promise> FileSystemFileHandle::getFile( // If the file does not exist, we reject the promise with a NotFoundError. auto ex = js.domException(kj::str("NotFoundError"), kj::str("Not found")); - return js.rejectedPromise>(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise>(kj::mv(ex)); } jsg::Promise> FileSystemFileHandle::createWritable( jsg::Lock& js, jsg::Optional options, - const jsg::TypeHandler>& deHandler, const jsg::TypeHandler& dataHandler) { auto opts = options.orDefault(FileSystemCreateWritableOptions{}); bool keepExistingData = opts.keepExistingData.orDefault(false); auto opened = FileSystemWriteContextHandle::open(js, JSG_THIS, keepExistingData); - KJ_IF_SOME(ex, opened.tryGet>()) { - return js.rejectedPromise>( - deHandler.wrap(js, kj::mv(ex))); + KJ_IF_SOME(ex, opened.tryGet()) { + return js.rejectedPromise>(kj::mv(ex)); } auto context = kj::mv(opened.get>()); @@ -2707,11 +2676,11 @@ jsg::Promise> FileSystemFileHandle::creat // Each closure holds its own ref to the context. These are separate jsg::Refs from the // one the stream holds, so each has its own tracing-parent slot; they stay strong and // untraced, while the stream's is what the GC traces. - sink.write = [context = context.addRef(), &deHandler, &dataHandler]( + sink.write = [context = context.addRef(), &dataHandler]( jsg::Lock& js, v8::Local chunk, auto c) mutable { return js.tryCatch([&] { KJ_IF_SOME(unwrapped, dataHandler.tryUnwrap(js, chunk)) { - return context->write(js, kj::mv(unwrapped), deHandler); + return context->write(js, kj::mv(unwrapped)); } return js.rejectedPromise( js.typeError("WritableStream received a value that is not writable")); @@ -2721,8 +2690,7 @@ jsg::Promise> FileSystemFileHandle::creat context->discard(js); return js.resolvedPromise(); }; - sink.close = [context = context.addRef(), &deHandler]( - jsg::Lock& js) mutable { return context->commit(js, deHandler); }; + sink.close = [context = context.addRef()](jsg::Lock& js) mutable { return context->commit(js); }; stream->getController().setup(js, kj::mv(sink), kj::none); return js.resolvedPromise(kj::mv(stream)); @@ -2737,8 +2705,7 @@ FileSystemWriteContextHandle::FileSystemWriteContextHandle(jsg::Lock& js, file(kj::mv(file)), lock(vfs.lock(js, this->file->getLocator())) {} -kj::OneOf, jsg::Ref> -FileSystemWriteContextHandle::open( +kj::OneOf, jsg::JsObject> FileSystemWriteContextHandle::open( jsg::Lock& js, jsg::Ref file, bool keepExistingData) { auto& vfs = file->getVfs(); @@ -2792,8 +2759,7 @@ void FileSystemWriteContextHandle::discard(jsg::Lock& js) { clear(); } -jsg::Promise FileSystemWriteContextHandle::commit( - jsg::Lock& js, const jsg::TypeHandler>& deHandler) { +jsg::Promise FileSystemWriteContextHandle::commit(jsg::Lock& js) { KJ_DEFER(clear()); return js.tryCatch([&] { KJ_IF_SOME(inner, temp) { @@ -2808,28 +2774,28 @@ jsg::Promise FileSystemWriteContextHandle::commit( })) { KJ_SWITCH_ONEOF(existing) { KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } KJ_CASE_ONEOF(dir, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Is a directory")); - return js.rejectedPromise(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } KJ_CASE_ONEOF(target, kj::Rc) { KJ_IF_SOME(err, target->replace(js, inner.addRef())) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } return js.resolvedPromise(); } KJ_CASE_ONEOF(link, kj::Rc) { auto ex = js.domException(kj::str("TypeMismatchError"), kj::str("Is a symbolic link")); - return js.rejectedPromise(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } } KJ_UNREACHABLE; } auto ex = js.domException(kj::str("InvalidStateError"), kj::str("Failed to open or create file")); - return js.rejectedPromise(deHandler.wrap(js, kj::mv(ex))); + return js.rejectedPromise(kj::mv(ex)); } return js.resolvedPromise(); }, [&](jsg::Value exception) { return js.rejectedPromise(kj::mv(exception)); }); @@ -2840,30 +2806,23 @@ FileSystemWritableFileStream::FileSystemWritableFileStream( : WritableStream(kj::mv(controller)), context(kj::mv(context)) {} -jsg::Promise FileSystemWritableFileStream::write(jsg::Lock& js, - FileSystemWritableData data, - const jsg::TypeHandler>& deHandler) { +jsg::Promise FileSystemWritableFileStream::write(jsg::Lock& js, FileSystemWritableData data) { JSG_REQUIRE(!getController().isLockedToWriter(), TypeError, "Cannot write to a stream that is locked to a reader"); auto writer = getWriter(js); KJ_DEFER(writer->releaseLock(js)); - return context->write(js, kj::mv(data), deHandler); + return context->write(js, kj::mv(data)); } -jsg::Promise FileSystemWritableFileStream::seek(jsg::Lock& js, - uint32_t position, - const jsg::TypeHandler>& deHandler) { - return context->seek(js, position, deHandler); +jsg::Promise FileSystemWritableFileStream::seek(jsg::Lock& js, uint32_t position) { + return context->seek(js, position); } -jsg::Promise FileSystemWritableFileStream::truncate( - jsg::Lock& js, uint32_t size, const jsg::TypeHandler>& deHandler) { - return context->truncate(js, size, deHandler); +jsg::Promise FileSystemWritableFileStream::truncate(jsg::Lock& js, uint32_t size) { + return context->truncate(js, size); } -jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, - FileSystemWritableData data, - const jsg::TypeHandler>& deHandler) { +jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, FileSystemWritableData data) { KJ_IF_SOME(inner, temp) { return js.tryCatch([&] { KJ_SWITCH_ONEOF(data) { @@ -2873,7 +2832,7 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, position += written; } KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } } @@ -2884,7 +2843,7 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, position += written; } KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } } @@ -2894,7 +2853,7 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, position += written; } KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } } @@ -2907,7 +2866,7 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, // the end is implementation-defined (File::write may resize or throw). // Growing zero-fills, which is what the spec requires for a gap. KJ_IF_SOME(err, inner->resize(js, pos)) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } offset = pos; @@ -2924,8 +2883,7 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, return js.resolvedPromise(); } KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise( - deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } KJ_UNREACHABLE; @@ -2938,8 +2896,7 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, return js.resolvedPromise(); } KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise( - deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } KJ_UNREACHABLE; @@ -2951,8 +2908,7 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, return js.resolvedPromise(); } KJ_CASE_ONEOF(err, workerd::FsError) { - return js.rejectedPromise( - deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } } @@ -2964,24 +2920,22 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, } } - return js.rejectedPromise(deHandler.wrap(js, - js.domException(kj::str("SyntaxError"), - kj::str("write() requires a non-null data parameter")))); + return js.rejectedPromise(js.domException( + kj::str("SyntaxError"), kj::str("write() requires a non-null data parameter"))); } else if (params.type == "seek"_kj) { uint32_t pos; KJ_IF_SOME(s, params.position) { pos = s; } else { - return js.rejectedPromise(deHandler.wrap(js, - js.domException( - kj::str("SyntaxError"), kj::str("seek() requires a position parameter")))); + return js.rejectedPromise(js.domException( + kj::str("SyntaxError"), kj::str("seek() requires a position parameter"))); } position = pos; auto stat = inner->stat(js); if (position > stat.size) { KJ_IF_SOME(err, inner->resize(js, position)) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } } else if (params.type == "truncate"_kj) { @@ -2989,12 +2943,11 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, KJ_IF_SOME(s, params.size) { size = s; } else { - return js.rejectedPromise(deHandler.wrap(js, - js.domException( - kj::str("SyntaxError"), kj::str("truncate() requires a size parameter")))); + return js.rejectedPromise(js.domException( + kj::str("SyntaxError"), kj::str("truncate() requires a size parameter"))); } KJ_IF_SOME(err, inner->resize(js, size)) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } auto stat = inner->stat(js); if (position > stat.size) { @@ -3014,14 +2967,12 @@ jsg::Promise FileSystemWriteContextHandle::write(jsg::Lock& js, return js.rejectedPromise(js.typeError("write() after closed")); } -jsg::Promise FileSystemWriteContextHandle::seek(jsg::Lock& js, - uint32_t newPosition, - const jsg::TypeHandler>& deHandler) { +jsg::Promise FileSystemWriteContextHandle::seek(jsg::Lock& js, uint32_t newPosition) { KJ_IF_SOME(inner, temp) { auto stat = inner->stat(js); if (newPosition > stat.size) { KJ_IF_SOME(err, inner->resize(js, newPosition)) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } } position = newPosition; @@ -3031,11 +2982,10 @@ jsg::Promise FileSystemWriteContextHandle::seek(jsg::Lock& js, return js.rejectedPromise(js.typeError("seek() after closed")); } -jsg::Promise FileSystemWriteContextHandle::truncate( - jsg::Lock& js, uint32_t size, const jsg::TypeHandler>& deHandler) { +jsg::Promise FileSystemWriteContextHandle::truncate(jsg::Lock& js, uint32_t size) { KJ_IF_SOME(inner, temp) { KJ_IF_SOME(err, inner->resize(js, size)) { - return js.rejectedPromise(deHandler.wrap(js, fsErrorToDomException(js, err))); + return js.rejectedPromise(fsErrorToDomException(js, err)); } auto stat = inner->stat(js); if (position > stat.size) { diff --git a/src/workerd/api/filesystem.h b/src/workerd/api/filesystem.h index 5cde464f192..7796051ba89 100644 --- a/src/workerd/api/filesystem.h +++ b/src/workerd/api/filesystem.h @@ -268,11 +268,10 @@ class FileSystemHandle: public jsg::Object { JSG_STRUCT(recursive); }; jsg::Promise remove(jsg::Lock& js, - jsg::Optional options, - const jsg::TypeHandler>& deHandler); + jsg::Optional options); jsg::Promise getUniqueId( - jsg::Lock& js, const jsg::TypeHandler>& deHandler); + jsg::Lock& js); JSG_RESOURCE_TYPE(FileSystemHandle) { JSG_READONLY_PROTOTYPE_PROPERTY(kind, getKind); @@ -326,11 +325,10 @@ class FileSystemFileHandle final: public FileSystemHandle { }; jsg::Promise> getFile( - jsg::Lock& js, const jsg::TypeHandler>& deHandler); + jsg::Lock& js); jsg::Promise> createWritable(jsg::Lock& js, jsg::Optional options, - const jsg::TypeHandler>& deHandler, const jsg::TypeHandler& dataHandler); JSG_RESOURCE_TYPE(FileSystemFileHandle) { @@ -423,18 +421,15 @@ class FileSystemDirectoryHandle final: public FileSystemHandle { jsg::Promise> getFileHandle(jsg::Lock& js, jsg::USVString name, - jsg::Optional options, - const jsg::TypeHandler>& exception); + jsg::Optional options); jsg::Promise> getDirectoryHandle(jsg::Lock& js, jsg::USVString name, - jsg::Optional options, - const jsg::TypeHandler>& exception); + jsg::Optional options); jsg::Promise removeEntry(jsg::Lock& js, jsg::USVString name, - jsg::Optional options, - const jsg::TypeHandler>& exception); + jsg::Optional options); // TODO(node-fs): We are not currently implementing the resolve() method // as defined in the spec. @@ -464,8 +459,7 @@ class FileSystemDirectoryHandle final: public FileSystemHandle { jsg::Function, jsg::Ref)> callback, - jsg::Optional thisArg, - const jsg::TypeHandler>& exception); + jsg::Optional thisArg); JSG_RESOURCE_TYPE(FileSystemDirectoryHandle) { JSG_INHERIT(FileSystemHandle); @@ -555,22 +549,20 @@ class FileSystemWriteContextHandle final: public jsg::Object { // it, because the two callers need it in different forms: the C++ // createWritable() wraps it in a rejected promise, while the bootstrap factory // backing the TypeScript implementation throws it. - static kj::OneOf, jsg::Ref> open( + static kj::OneOf, jsg::JsObject> open( jsg::Lock& js, jsg::Ref file, bool keepExistingData); jsg::Promise write(jsg::Lock& js, - FileSystemWritableData data, - const jsg::TypeHandler>& deHandler); + FileSystemWritableData data); jsg::Promise seek(jsg::Lock& js, - uint32_t newPosition, - const jsg::TypeHandler>& deHandler); + uint32_t newPosition); jsg::Promise truncate( - jsg::Lock& js, uint32_t size, const jsg::TypeHandler>& deHandler); + jsg::Lock& js, uint32_t size); // The underlying sink's close algorithm: replaces the original file's contents // with the temporary's, then releases the state. jsg::Promise commit( - jsg::Lock& js, const jsg::TypeHandler>& deHandler); + jsg::Lock& js); // The underlying sink's abort algorithm: drops the written data, leaving the // original file intact. @@ -624,13 +616,11 @@ class FileSystemWritableFileStream final: public WritableStream { using WriteParams = FileSystemFileWriteParams; jsg::Promise write(jsg::Lock& js, - FileSystemWritableData data, - const jsg::TypeHandler>& deHandler); + FileSystemWritableData data); jsg::Promise seek(jsg::Lock& js, - uint32_t position, - const jsg::TypeHandler>& deHandler); + uint32_t position); jsg::Promise truncate( - jsg::Lock& js, uint32_t size, const jsg::TypeHandler>& deHandler); + jsg::Lock& js, uint32_t size); JSG_RESOURCE_TYPE(FileSystemWritableFileStream) { JSG_INHERIT(WritableStream); @@ -650,7 +640,7 @@ class FileSystemWritableFileStream final: public WritableStream { class StorageManager final: public jsg::Object { public: jsg::Promise> getDirectory( - jsg::Lock& js, const jsg::TypeHandler>& exception); + jsg::Lock& js); JSG_RESOURCE_TYPE(StorageManager) { JSG_METHOD(getDirectory); diff --git a/src/workerd/api/memory-cache.c++ b/src/workerd/api/memory-cache.c++ index a3e577d2cbf..ead109b3d74 100644 --- a/src/workerd/api/memory-cache.c++ +++ b/src/workerd/api/memory-cache.c++ @@ -301,8 +301,12 @@ static kj::Array hackySerialize(jsg::Lock& js, jsg::JsRef& args) { }); } +// Gives a DOMException being constructed by the TypeScript DOMException its brand and its own +// `code`, `name` and `message` properties (jsg::initDOMException()). The caller passes the +// object it is constructing and the converted strings. +static void InitDOMException(const v8::FunctionCallbackInfo& args) { + auto isolate = args.GetIsolate(); + jsg::liftKj(isolate, [&] { + KJ_ASSERT(args[0]->IsObject() && args[1]->IsString() && args[2]->IsString()); + // A failure leaves its exception pending, which propagates to the constructor's caller. + (void)jsg::initDOMException(isolate, isolate->GetCurrentContext(), args[0].As(), + args[1].As(), args[2].As()); + }); +} + +// Returns whether a value is a DOMException, by its brand. +static void IsDOMException(const v8::FunctionCallbackInfo& args) { + args.GetReturnValue().Set(jsg::tryGetDOMExceptionFields(args.GetIsolate(), args[0]) != kj::none); +} + +// Hands native code the prototype of the TypeScript DOMException, for the DOMExceptions it +// creates (jsg::setDOMExceptionPrototype()). +static void SetDOMExceptionPrototype(const v8::FunctionCallbackInfo& args) { + auto isolate = args.GetIsolate(); + jsg::liftKj(isolate, [&] { + KJ_ASSERT(args[0]->IsObject()); + jsg::setDOMExceptionPrototype(isolate->GetCurrentContext(), args[0].As()); + }); +} + static const v8::CFunction fast_mark_promise_handled_ = v8::CFunction::Make(MarkPromiseHandledFastApi); @@ -244,6 +272,10 @@ v8::Local getMethod(jsg::Lock& js, v8::FunctionCallback callback) { return makeMethod(js, callback, v8::SideEffectType::kHasSideEffect); } +v8::Local getMethodNoSideEffect(jsg::Lock& js, v8::FunctionCallback callback) { + return makeMethod(js, callback, v8::SideEffectType::kHasNoSideEffect); +} + // Creates an object with methods for performing fast type checks on JS values. // Because we are not fully bootstrapped at this point, we don't want to rely // on jsg::Object and the type wrapper system, etc. Instead, just use a plain @@ -265,6 +297,9 @@ jsg::JsRef createUtilsObject(jsg::Lock& js) { "createFileSystemWriteContext", "addAbortAlgorithm", "utf8Length", + "initDOMException", + "isDOMException", + "setDOMExceptionPrototype", }; auto tmpl = v8::DictionaryTemplate::New(js.v8Isolate, names); v8::MaybeLocal values[] = { @@ -279,6 +314,9 @@ jsg::JsRef createUtilsObject(jsg::Lock& js) { getMethod(js, CreateFileSystemWriteContext), getMethod(js, AddAbortAlgorithm), getMethod(js, Utf8Length), + getMethod(js, InitDOMException), + getMethodNoSideEffect(js, IsDOMException), + getMethod(js, SetDOMExceptionPrototype), }; static_assert(kj::arrayPtr(names).size() == kj::arrayPtr(values).size()); diff --git a/src/workerd/api/streams/standard.c++ b/src/workerd/api/streams/standard.c++ index f3b40cb49e3..80eb27de9ff 100644 --- a/src/workerd/api/streams/standard.c++ +++ b/src/workerd/api/streams/standard.c++ @@ -1408,9 +1408,8 @@ jsg::Promise WritableImpl::abort( // when no reason is provided, but the stored error should remain as the original reason. auto signalReason = [&]() -> jsg::JsValue { if (reason.isUndefined() && FeatureFlags::get(js).getPedanticWpt()) { - auto ex = js.domException( + return js.domException( kj::str("AbortError"), kj::str("This writable stream has been aborted."), kj::none); - return jsg::JsValue(KJ_ASSERT_NONNULL(ex.tryGetHandle(js))); } return reason; }(); diff --git a/src/workerd/api/tests/BUILD.bazel b/src/workerd/api/tests/BUILD.bazel index 27c17237d72..c56b276eb46 100644 --- a/src/workerd/api/tests/BUILD.bazel +++ b/src/workerd/api/tests/BUILD.bazel @@ -802,6 +802,11 @@ wd_test( data = ["global-scope-test.js"], ) +wd_test( + src = "dom-exception-test.wd-test", + data = ["dom-exception-test.js"], +) + wd_test( src = "queue-microtask-gc-test.wd-test", data = ["queue-microtask-gc-test.js"], diff --git a/src/workerd/api/tests/dom-exception-test.js b/src/workerd/api/tests/dom-exception-test.js new file mode 100644 index 00000000000..69700eca136 --- /dev/null +++ b/src/workerd/api/tests/dom-exception-test.js @@ -0,0 +1,237 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +// The global DOMException is a TypeScript class (src/per_isolate/dom-exception.ts). These tests +// pin the shape it shares with the C++ class it replaced, and check that the DOMExceptions the +// runtime creates are instances of it. + +import { deepStrictEqual, ok, strictEqual, throws } from 'node:assert'; +import { types } from 'node:util'; + +const describe = (object, key) => { + const { value, get, set, ...attributes } = Object.getOwnPropertyDescriptor( + object, + key + ); + return { ...attributes, accessor: get !== undefined && set !== undefined }; +}; + +const DATA = { + writable: false, + enumerable: true, + configurable: true, + accessor: false, +}; + +const CODES = [ + 'INDEX_SIZE_ERR', + 'DOMSTRING_SIZE_ERR', + 'HIERARCHY_REQUEST_ERR', + 'WRONG_DOCUMENT_ERR', + 'INVALID_CHARACTER_ERR', + 'NO_DATA_ALLOWED_ERR', + 'NO_MODIFICATION_ALLOWED_ERR', + 'NOT_FOUND_ERR', + 'NOT_SUPPORTED_ERR', + 'INUSE_ATTRIBUTE_ERR', + 'INVALID_STATE_ERR', + 'SYNTAX_ERR', + 'INVALID_MODIFICATION_ERR', + 'NAMESPACE_ERR', + 'INVALID_ACCESS_ERR', + 'VALIDATION_ERR', + 'TYPE_MISMATCH_ERR', + 'SECURITY_ERR', + 'NETWORK_ERR', + 'ABORT_ERR', + 'URL_MISMATCH_ERR', + 'QUOTA_EXCEEDED_ERR', + 'TIMEOUT_ERR', + 'INVALID_NODE_TYPE_ERR', + 'DATA_CLONE_ERR', +]; + +function checkInstance(e, name, message, code) { + ok(e instanceof DOMException); + ok(e instanceof Error); + ok(!types.isNativeError(e)); + deepStrictEqual(Reflect.ownKeys(e), ['code', 'name', 'message', 'stack']); + deepStrictEqual(describe(e, 'code'), DATA); + deepStrictEqual(describe(e, 'name'), DATA); + deepStrictEqual(describe(e, 'message'), DATA); + strictEqual(describe(e, 'stack').enumerable, true); + strictEqual(e.name, name); + strictEqual(e.message, message); + strictEqual(e.code, code); + // DOMException.prototype has a Symbol.toStringTag under the set_tostring_tag flag. + strictEqual( + Object.prototype.toString.call(e), + Symbol.toStringTag in DOMException.prototype + ? '[object DOMException]' + : '[object Object]' + ); +} + +export const constructed = { + test() { + const e = new DOMException('msg', 'AbortError'); + checkInstance(e, 'AbortError', 'msg', 20); + strictEqual(describe(e, 'stack').accessor, true); + // The stack starts with the caller's frame, not the constructor's. + const [header, frame] = e.stack.split('\n'); + strictEqual(header, 'AbortError: msg'); + ok(frame.includes('Object.test'), frame); + + checkInstance(new DOMException(), 'Error', '', 0); + checkInstance(new DOMException(null, undefined), 'Error', 'null', 0); + checkInstance(new DOMException(undefined, null), 'null', '', 0); + checkInstance( + new DOMException( + { toString: () => 'm' }, + { toString: () => 'DataError' } + ), + 'DataError', + 'm', + 0 + ); + throws(() => new DOMException(Symbol()), TypeError); + throws(() => DOMException('m'), { + name: 'TypeError', + message: + "Failed to construct 'DOMException': Please use the 'new' operator, this object constructor cannot be called as a function.", + }); + + throws(() => { + e.name = 'x'; + }, TypeError); + strictEqual( + JSON.stringify(e).startsWith('{"code":20,"name":"AbortError"'), + true + ); + }, +}; + +export const subclassed = { + test() { + class Sub extends DOMException { + constructor() { + super('s', 'NotFoundError'); + this.extra = 1; + } + } + const s = new Sub(); + ok(s instanceof Sub); + ok(s instanceof DOMException); + deepStrictEqual(Reflect.ownKeys(s), [ + 'code', + 'name', + 'message', + 'stack', + 'extra', + ]); + strictEqual(s.code, 8); + ok(s.stack.split('\n')[1].includes('new Sub'), s.stack); + }, +}; + +export const classShape = { + test() { + // The TypeScript class, whose source shows; the C++ class's showed [native code]. + ok( + !Function.prototype.toString.call(DOMException).includes('[native code]') + ); + strictEqual(DOMException.name, 'DOMException'); + strictEqual(DOMException.length, 0); + strictEqual(Object.getPrototypeOf(DOMException), Function.prototype); + strictEqual(Object.getPrototypeOf(DOMException.prototype), Error.prototype); + deepStrictEqual(Reflect.ownKeys(DOMException), [ + 'length', + 'name', + 'prototype', + ...CODES, + ]); + deepStrictEqual(Reflect.ownKeys(DOMException.prototype).slice(0, 26), [ + ...CODES, + 'constructor', + ]); + CODES.forEach((name, i) => { + strictEqual(DOMException[name], i + 1); + strictEqual(DOMException.prototype[name], i + 1); + }); + strictEqual(DOMException.prototype.constructor, DOMException); + ok(Symbol.for('cloudflare:internal-class') in DOMException.prototype); + + // The global lives on the global object's prototype, as JSG puts it. + strictEqual( + Object.getOwnPropertyDescriptor(globalThis, 'DOMException'), + undefined + ); + deepStrictEqual( + describe(Object.getPrototypeOf(globalThis), 'DOMException'), + { writable: true, enumerable: true, configurable: true, accessor: false } + ); + }, +}; + +export const createdByTheRuntime = { + test() { + let e; + try { + atob('*'); + } catch (err) { + e = err; + } + checkInstance(e, 'InvalidCharacterError', e.message, 5); + + try { + structuredClone(() => {}); + } catch (err) { + e = err; + } + checkInstance(e, 'DataCloneError', e.message, 25); + + const reason = AbortSignal.abort().reason; + ok(reason instanceof DOMException); + strictEqual(reason.name, 'AbortError'); + }, +}; + +export const userCodeCannotRedirectTheRuntime = { + test() { + const original = DOMException; + const originalPrototype = DOMException.prototype; + try { + globalThis.DOMException = class Fake {}; + original.prototype = {}; + let e; + try { + atob('*'); + } catch (err) { + e = err; + } + strictEqual(Object.getPrototypeOf(e), originalPrototype); + } finally { + original.prototype = originalPrototype; + globalThis.DOMException = original; + } + }, +}; + +export const cloned = { + test() { + const e = new DOMException('cl', 'DataCloneError'); + const c = structuredClone(e); + checkInstance(c, 'DataCloneError', 'cl', 25); + strictEqual(c.stack, e.stack); + + // The clone has the name and message the DOMException was created with. + const overridden = new DOMException('m', 'DataError'); + Object.defineProperty(overridden, 'name', { value: 'TypeError' }); + strictEqual(structuredClone(overridden).name, 'DataError'); + + // A plain object that looks like one is cloned as a plain object. + const plain = structuredClone({ name: 'AbortError', message: 'm' }); + ok(!(plain instanceof DOMException)); + }, +}; diff --git a/src/workerd/api/tests/dom-exception-test.wd-test b/src/workerd/api/tests/dom-exception-test.wd-test new file mode 100644 index 00000000000..0d738ae6e49 --- /dev/null +++ b/src/workerd/api/tests/dom-exception-test.wd-test @@ -0,0 +1,14 @@ +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [ + ( name = "dom-exception-test", + worker = ( + modules = [ + (name = "worker", esModule = embed "dom-exception-test.js") + ], + compatibilityFlags = ["nodejs_compat"] + ) + ), + ], +); diff --git a/src/workerd/jsg/dom-exception-test.c++ b/src/workerd/jsg/dom-exception-test.c++ index ea7bb4db775..fb7c8d7ebd8 100644 --- a/src/workerd/jsg/dom-exception-test.c++ +++ b/src/workerd/jsg/dom-exception-test.c++ @@ -51,16 +51,51 @@ KJ_TEST("jsg::Lock domException") { JSG_WITHIN_CONTEXT_SCOPE(lock, lock.template newContext().getHandle(lock.v8Isolate), [&](jsg::Lock& js) { - Ref exception = js.domException(kj::str("foo"), kj::str("bar")); + auto exception = js.domException(kj::str("foo"), kj::str("bar")); - // The DOMException object should have the JS handle created already. - auto handle = JsObject(KJ_ASSERT_NONNULL(exception.tryGetHandle(js))); + KJ_ASSERT(exception.has(js, "stack"_kj)); + KJ_ASSERT(exception.get(js, "name"_kj).toString(js) == "foo"_kj); + KJ_ASSERT(exception.get(js, "message"_kj).toString(js) == "bar"_kj); + KJ_ASSERT(exception.get(js, "code"_kj).toString(js) == "0"_kj); + auto constructor = + KJ_ASSERT_NONNULL(js.global().get(js, "DOMException"_kj).tryCast()); + KJ_ASSERT(exception.getPrototype(js) == constructor.get(js, "prototype"_kj)); + }); + }); +} + +KJ_TEST("DOMExceptions are recognized by their brand") { + Evaluator e(v8System); + e.getIsolate().runInLockScope([&](DOMExceptionIsolate::Lock& lock) { + JSG_WITHIN_CONTEXT_SCOPE(lock, + lock.template newContext().getHandle(lock.v8Isolate), + [&](jsg::Lock& js) { + auto fieldsOf = [&](kj::StringPtr code) -> kj::Maybe { + auto script = jsg::check(v8::Script::Compile(js.v8Context(), js.str(code))); + auto value = jsg::check(script->Run(js.v8Context())); + return tryGetDOMExceptionFields(js.v8Isolate, value).map([&](DOMExceptionFields fields) { + return kj::str( + JsString(fields.name).toString(js), "/", JsString(fields.message).toString(js)); + }); + }; - // And it will have the stack. - KJ_ASSERT(handle.has(js, "stack"_kj)); + KJ_EXPECT(KJ_ASSERT_NONNULL(fieldsOf("new DOMException('m', 'AbortError')"_kj)) == + "AbortError/m"_kj); + KJ_EXPECT(KJ_ASSERT_NONNULL(fieldsOf("new (class extends DOMException {})('m')"_kj)) == + "Error/m"_kj); + // The brand, not the JavaScript properties, which can be redefined. + KJ_EXPECT(KJ_ASSERT_NONNULL(fieldsOf( + "const e = new DOMException('m', 'DataError');" + "Object.defineProperty(e, 'name', { value: 'X' }); e"_kj)) == "DataError/m"_kj); + // Only an object's own brand counts. + KJ_EXPECT(fieldsOf("Object.create(new DOMException('m'))"_kj) == kj::none); + KJ_EXPECT(fieldsOf("({ name: 'AbortError', message: 'm' })"_kj) == kj::none); + KJ_EXPECT(fieldsOf("new Error('m')"_kj) == kj::none); - KJ_ASSERT(exception->getName() == "foo"_kj); - KJ_ASSERT(exception->getMessage() == "bar"_kj); + auto created = js.domException(kj::str("NotFoundError"), kj::str("gone")); + auto fields = KJ_ASSERT_NONNULL(tryGetDOMExceptionFields(js.v8Isolate, created)); + KJ_EXPECT(JsString(fields.name).toString(js) == "NotFoundError"_kj); + KJ_EXPECT(JsString(fields.message).toString(js) == "gone"_kj); }); }); } diff --git a/src/workerd/jsg/dom-exception.c++ b/src/workerd/jsg/dom-exception.c++ index 0471d7114c8..9cf339f80e1 100644 --- a/src/workerd/jsg/dom-exception.c++ +++ b/src/workerd/jsg/dom-exception.c++ @@ -11,10 +11,154 @@ #include -#include - namespace workerd::jsg { +namespace { + +v8::Local namePrivate(v8::Isolate* isolate) { + return v8::Private::ForApi(isolate, + v8::String::NewFromUtf8Literal( + isolate, "workerd:DOMException.name", v8::NewStringType::kInternalized)); +} + +v8::Local messagePrivate(v8::Isolate* isolate) { + return v8::Private::ForApi(isolate, + v8::String::NewFromUtf8Literal( + isolate, "workerd:DOMException.message", v8::NewStringType::kInternalized)); +} + +int getCode(v8::Isolate* isolate, v8::Local name) { + // Every name with a code is ASCII and shorter than this. + char buffer[32]; + if (name->Length() >= sizeof(buffer)) return 0; + size_t length = name->WriteUtf8V2(isolate, buffer, sizeof(buffer)); + return getDOMExceptionCode(kj::arrayPtr(buffer, length)); +} + +v8::Maybe defineReadOnly(v8::Isolate* isolate, + v8::Local context, + v8::Local object, + v8::Local key, + v8::Local value) { + bool defined; + if (!object->DefineOwnProperty(context, key, value, v8::PropertyAttribute::ReadOnly) + .To(&defined)) { + return v8::Nothing(); + } + KJ_ASSERT(defined, "could not define a DOMException property"); + return v8::JustVoid(); +} + +} // namespace + +int getDOMExceptionCode(kj::ArrayPtr name) { + static constexpr struct { + kj::StringPtr name; + int code; + } LEGACY_CODES[] = { +#define TABLE_ENTRY(name, code, friendlyName) {friendlyName##_kj, code}, + JSG_DOM_EXCEPTION_FOR_EACH_ERROR_NAME(TABLE_ENTRY) +#undef TABLE_ENTRY + }; + for (auto& entry: LEGACY_CODES) { + if (entry.name.asArray() == name) return entry.code; + } + return 0; +} + +v8::Maybe initDOMException(v8::Isolate* isolate, + v8::Local context, + v8::Local object, + v8::Local name, + v8::Local message) { + // The order of `code`, `name` and `message` is the order in which they have always been own + // properties of a DOMException, from JSG's lazy instance properties. + auto code = v8::Integer::New(isolate, getCode(isolate, name)); + if (defineReadOnly(isolate, context, object, + v8::String::NewFromUtf8Literal(isolate, "code", v8::NewStringType::kInternalized), code) + .IsNothing() || + defineReadOnly(isolate, context, object, + v8::String::NewFromUtf8Literal(isolate, "name", v8::NewStringType::kInternalized), name) + .IsNothing() || + defineReadOnly(isolate, context, object, + v8::String::NewFromUtf8Literal(isolate, "message", v8::NewStringType::kInternalized), + message) + .IsNothing()) { + return v8::Nothing(); + } + if (object->SetPrivate(context, namePrivate(isolate), name).IsNothing() || + object->SetPrivate(context, messagePrivate(isolate), message).IsNothing()) { + return v8::Nothing(); + } + return v8::JustVoid(); +} + +v8::MaybeLocal newDOMException(v8::Isolate* isolate, + v8::Local context, + v8::Local name, + v8::Local message, + v8::MaybeLocal stack) { + auto prototype = getContextDataSlot(context, ContextDataSlot::DOM_EXCEPTION_PROTOTYPE); + KJ_ASSERT(prototype->IsValue() && prototype.As()->IsObject(), + "the context has no DOMException prototype"); + auto object = v8::Object::New(isolate, prototype.As(), nullptr, nullptr, 0); + if (initDOMException(isolate, context, object, name, message).IsNothing()) return {}; + + auto stackName = + v8::String::NewFromUtf8Literal(isolate, "stack", v8::NewStringType::kInternalized); + v8::Local stackValue; + if (stack.ToLocal(&stackValue)) { + // Writable and enumerable, but, as JSG has always defined it, not configurable. + v8::PropertyDescriptor prop(stackValue, true); + prop.set_enumerable(true); + if (object->DefineProperty(context, stackName, prop).IsNothing()) return {}; + } else { + if (v8::Exception::CaptureStackTrace(context, object).IsNothing()) return {}; + // Error properties are not enumerable, but a DOMException's always have been. + v8::PropertyDescriptor prop; + prop.set_enumerable(true); + if (object->DefineProperty(context, stackName, prop).IsNothing()) return {}; + } + return object; +} + +kj::Maybe tryGetDOMExceptionFields( + v8::Isolate* isolate, v8::Local value) { + if (!value->IsObject()) return kj::none; + auto object = value.As(); + auto context = isolate->GetCurrentContext(); + // A private property is never looked up on the prototype chain, and reading one runs no + // JavaScript, not even for a Proxy. + v8::Local name; + v8::Local message; + if (!object->GetPrivate(context, namePrivate(isolate)).ToLocal(&name) || !name->IsString() || + !object->GetPrivate(context, messagePrivate(isolate)).ToLocal(&message) || + !message->IsString()) { + return kj::none; + } + return DOMExceptionFields{name.As(), message.As()}; +} + +void setDOMExceptionPrototype(v8::Local context, v8::Local prototype) { + setContextDataSlot(context, ContextDataSlot::DOM_EXCEPTION_PROTOTYPE, prototype); +} + +void serializeDOMException( + Lock& js, Serializer& serializer, v8::Local object, DOMExceptionFields fields) { + serializer.writeLengthDelimited(JsString(fields.name).toString(js)); + serializer.writeLengthDelimited(JsString(fields.message).toString(js)); + + // It's a bit unfortunate that the stack here ends up also including the name and message + // so we end up duplicating some of the information here, but that's OK. It's better to + // keep this implementation simple rather than to implement any kind of deduplication. + auto stack = JsObject(object).get(js, "stack"_kj); + if (stack.isUndefined()) { + serializer.writeLengthDelimited(""_kj); + } else { + serializer.writeLengthDelimited(stack.toString(js)); + } +} + Ref DOMException::constructor(const v8::FunctionCallbackInfo& args, Optional message, Optional name) { @@ -37,8 +181,13 @@ Ref DOMException::constructor(const v8::FunctionCallbackInfo stackName = js.str("stack"_kjc); jsg::check(args.This()->DefineProperty(js.v8Context(), stackName, prop)); - return js.alloc( - kj::mv(errMessage), kj::mv(name).orDefault([] { return kj::str("Error"); })); + auto errName = kj::mv(name).orDefault([] { return kj::str("Error"); }); + // The name and message are also this class's lazy instance properties, so only the brand is set. + auto isolate = js.v8Isolate; + jsg::check(args.This()->SetPrivate(js.v8Context(), namePrivate(isolate), js.str(errName))); + jsg::check(args.This()->SetPrivate(js.v8Context(), messagePrivate(isolate), js.str(errMessage))); + + return js.alloc(kj::mv(errMessage), kj::mv(errName)); } kj::StringPtr DOMException::getName() const { @@ -50,38 +199,16 @@ kj::StringPtr DOMException::getMessage() const { } int DOMException::getCode() const { - static const std::map legacyCodes{ -#define MAP_ENTRY(name, code, friendlyName) {friendlyName, code}, - JSG_DOM_EXCEPTION_FOR_EACH_ERROR_NAME(MAP_ENTRY) -#undef MAP_ENTRY - }; - auto code = legacyCodes.find(name); - if (code != legacyCodes.end()) { - return code->second; - } - return 0; + return getDOMExceptionCode(name.asArray()); } void DOMException::serialize(jsg::Lock& js, jsg::Serializer& serializer) { - serializer.writeLengthDelimited(name); - serializer.writeLengthDelimited(message); - - // It's a bit unfortunate that the stack here ends up also including the name and message - // so we end up duplicating some of the information here, but that's OK. It's better to - // keep this implementation simple rather than to implement any kind of deduplication. - KJ_IF_SOME(stack, this->stack.get(js, "stack")) { - serializer.writeLengthDelimited(stack); - } else { - // This branch shouldn't really be taken in the typical case. It's only here - // to handle the case where the stack property could not be unwrapped for some - // reason. We don't need to treat it as an error case, just set the stack to - // the empty string and move on. - serializer.writeLengthDelimited(""_kj); - } + // Serializer::writeApiObject() writes every DOMException itself, by its brand, without calling + // this. + KJ_UNREACHABLE; } -jsg::Ref DOMException::deserialize( - jsg::Lock& js, uint tag, jsg::Deserializer& deserializer) { +jsg::JsObject DOMException::deserialize(jsg::Lock& js, uint tag, jsg::Deserializer& deserializer) { switch (tag) { case SERIALIZATION_TAG_V2: { kj::String name = deserializer.readLengthDelimitedString(); diff --git a/src/workerd/jsg/dom-exception.h b/src/workerd/jsg/dom-exception.h index e2fce8e79d0..0ac21100aee 100644 --- a/src/workerd/jsg/dom-exception.h +++ b/src/workerd/jsg/dom-exception.h @@ -30,6 +30,14 @@ namespace workerd::jsg { class Serializer; class Deserializer; +// The functions that create and recognize DOMExceptions are declared in jsg.h. + +// In a Worker, the per-isolate bootstrap replaces this class as the global DOMException with a +// TypeScript class of the same shape (src/per_isolate/dom-exception.ts), and native code creates +// instances of that. This class remains the DOMException of isolates that run no bootstrap, such +// as those of JSG's and Rust JSG's tests, and supplies the deserializer for its serialization +// tags. +// // JSG allows DOMExceptions to be tunneled through kj::Exceptions (see exceptionToJs() for // details). While this feature is activated conditionally at run-time, and thus does not depend // on any specific concrete C++ type, JSG needs to be able to unit test the tunneled exception @@ -82,7 +90,6 @@ class DOMException: public Object { set stack(value: any); }); } - JSG_REFLECTION(stack); #undef JSG_DOM_EXCEPTION_CONSTANT_CXX #undef JSG_DOM_EXCEPTION_CONSTANT_JS @@ -101,13 +108,11 @@ class DOMException: public Object { JSG_SERIALIZABLE(SERIALIZATION_TAG_V2, SERIALIZATION_TAG); void serialize(jsg::Lock& js, jsg::Serializer& serializer); - static jsg::Ref deserialize( - jsg::Lock& js, uint tag, jsg::Deserializer& deserializer); + static jsg::JsObject deserialize(jsg::Lock& js, uint tag, jsg::Deserializer& deserializer); private: kj::String message; kj::String name; - PropertyReflection stack; }; } // namespace workerd::jsg diff --git a/src/workerd/jsg/jsg.h b/src/workerd/jsg/jsg.h index 99831fa9ae8..899fa5d2ab5 100644 --- a/src/workerd/jsg/jsg.h +++ b/src/workerd/jsg/jsg.h @@ -2633,6 +2633,57 @@ class JsRef; class DOMException; class ExternalMemoryAdjustment; +class Serializer; + +// A DOMException carries its name and message in V8 private properties, which JavaScript cannot +// read or change. They brand it: native code recognizes a DOMException by them, and reads the +// name and message it was created with, without running JavaScript. Both the C++ class +// (dom-exception.h) and the TypeScript class that the per-isolate bootstrap installs as the +// global DOMException brand their instances this way, so the functions below work for either. +// +// These functions use only V8, not jsg::Lock, so that Rust JSG's shims can call them. + +struct DOMExceptionFields { + v8::Local name; + v8::Local message; +}; + +// Returns the legacy `code` of a DOMException named `name`, or 0 for a name without one. +int getDOMExceptionCode(kj::ArrayPtr name); + +// Gives `object` the brand of a DOMException named `name` with `message`, and its own read-only +// `code`, `name` and `message` properties, in that order. Captures no stack. Runs no JavaScript. +// Returns nothing if defining a property failed, with an exception pending. +v8::Maybe initDOMException(v8::Isolate* isolate, + v8::Local context, + v8::Local object, + v8::Local name, + v8::Local message); + +// Creates a DOMException without running JavaScript, inheriting from the DOMException prototype +// of `context` (see setDOMExceptionPrototype()). Its own `stack` property is `stack` if given, +// otherwise the current stack trace. Returns nothing with an exception pending if creating it +// failed. +v8::MaybeLocal newDOMException(v8::Isolate* isolate, + v8::Local context, + v8::Local name, + v8::Local message, + v8::MaybeLocal stack = {}); + +// Returns the name and message of `value` if it is a DOMException, kj::none otherwise. Runs no +// JavaScript. Only the object's own brand counts, not one on its prototype chain. +kj::Maybe tryGetDOMExceptionFields( + v8::Isolate* isolate, v8::Local value); + +// Sets the prototype that newDOMException() gives the DOMExceptions it creates in `context`. +// JSG sets this class's prototype when it creates a context, and the per-isolate bootstrap +// replaces it with the TypeScript class's. +void setDOMExceptionPrototype(v8::Local context, v8::Local prototype); + +// Writes `object`, a DOMException with `fields`, as DOMException::SERIALIZATION_TAG_V2 (without +// the tag). Reads its `stack` property, which can run JavaScript. +void serializeDOMException( + Lock& js, Serializer& serializer, v8::Local object, DOMExceptionFields fields); // Used to save a reference to an isolate that is responsible for external memory usage. // getAdjustment() can be invoked at any time to create a new RAII adjustment object @@ -3187,7 +3238,8 @@ class Lock { } } - virtual Ref domException( + // Creates a DOMException with the given name and message. Runs no JavaScript. + virtual JsObject domException( kj::String name, kj::String message, kj::Maybe stackValue = kj::none) = 0; // Get the prototype object for the given C++ type (which must be a JSG_RESOURCE_TYPE). diff --git a/src/workerd/jsg/ser.c++ b/src/workerd/jsg/ser.c++ index 71e45b21c72..572c72539ae 100644 --- a/src/workerd/jsg/ser.c++ +++ b/src/workerd/jsg/ser.c++ @@ -194,8 +194,7 @@ void Serializer::throwDataCloneErrorForObject(jsg::Lock& js, v8::LocalGetConstructorName(), "\". This type does " "not support serialization."); - auto exception = js.domException(kj::str("DataCloneError"), kj::mv(message)); - js.throwException(jsg::JsValue(KJ_ASSERT_NONNULL(exception.tryGetHandle(js)))); + js.throwException(js.domException(kj::str("DataCloneError"), kj::mv(message))); } void Serializer::ThrowDataCloneError(v8::Local message) { @@ -207,18 +206,15 @@ void Serializer::ThrowDataCloneError(v8::Local message) { } bool Serializer::HasCustomHostObject(v8::Isolate* isolate) { - // By default, V8 will call WriteHostObject() for objects that have internal fields. We only need - // to override IsHostObject() if we want to treat pure-JS objects differently, which we do if - // treatClassInstancesAsPlainObjects is false, or if treatErrorsAsHostObjects is true. - return !treatClassInstancesAsPlainObjects || treatErrorsAsHostObjects; + // By default, V8 will call WriteHostObject() only for objects that have internal fields. A + // DOMException is a host object whether or not it has them. + return true; } v8::Maybe Serializer::IsHostObject(v8::Isolate* isolate, v8::Local object) { - // This is only called if HasCustomHostObject() returned true. - KJ_ASSERT(!treatClassInstancesAsPlainObjects || treatErrorsAsHostObjects); - - // Any object with internal fields is DEFINITELY a host object! - if (object->InternalFieldCount() > 0) { + // Any object with internal fields is DEFINITELY a host object, and so is a DOMException, which + // writeApiObject() writes by its brand. + if (object->InternalFieldCount() > 0 || tryGetDOMExceptionFields(isolate, object) != kj::none) { return v8::Just(true); } @@ -303,6 +299,14 @@ v8::Maybe Serializer::WriteHostObject(v8::Isolate* isolate, v8::Local object) { + KJ_IF_SOME(fields, tryGetDOMExceptionFields(js.v8Isolate, object)) { + // Whichever class created it, a DOMException is written as the C++ class's serializer always + // wrote it, so that both read it back. + writeRawUint32(DOMException::SERIALIZATION_TAG_V2); + serializeDOMException(js, *this, object, fields); + return; + } + if (object->InternalFieldCount() != Wrappable::INTERNAL_FIELD_COUNT || !Wrappable::isWorkerdApiObject(object)) { KJ_IF_SOME(eh, externalHandler) { @@ -675,8 +679,7 @@ v8::MaybeLocal readApiObject( v8::MaybeLocal newDataCloneError(v8::Isolate* isolate, v8::Local message) { auto& js = Lock::from(isolate); try { - auto exception = js.domException(kj::str("DataCloneError"), kj::str(message)); - return KJ_ASSERT_NONNULL(exception.tryGetHandle(js)); + return v8::Local(js.domException(kj::str("DataCloneError"), kj::str(message))); } catch (JsExceptionThrown&) { // Creating the DOMException threw, most likely because execution is terminating; that // exception stays pending. diff --git a/src/workerd/jsg/setup.h b/src/workerd/jsg/setup.h index 3bf47005015..74b501a5328 100644 --- a/src/workerd/jsg/setup.h +++ b/src/workerd/jsg/setup.h @@ -1068,31 +1068,15 @@ class Isolate: public IsolateBase { *this, context, handle, jsg::TypeErrorContext::other()); } - Ref domException( + JsObject domException( kj::String name, kj::String message, kj::Maybe maybeStack) override { - return withinHandleScope([&] { - v8::Local tmpl = jsgIsolate.getWrapperByContext(*this)->getTemplate( - v8Isolate, static_cast(nullptr)); - KJ_DASSERT(!tmpl.IsEmpty()); - v8::Local obj = check(tmpl->InstanceTemplate()->NewInstance(v8Context())); - v8::Local stackName = str("stack"_kjc); - - KJ_IF_SOME(stack, maybeStack) { - v8::PropertyDescriptor prop(str(stack), true); - prop.set_enumerable(true); - jsg::check(obj->DefineProperty(v8Context(), stackName, prop)); - } else { - v8::Exception::CaptureStackTrace(v8Context(), obj); - v8::PropertyDescriptor prop; - prop.set_enumerable(true); - jsg::check(obj->DefineProperty(v8Context(), stackName, prop)); + return JsObject(withinHandleScope([&]() -> v8::Local { + v8::MaybeLocal stack; + KJ_IF_SOME(s, maybeStack) { + stack = v8::Local(str(s)); } - - auto de = alloc(kj::mv(message), kj::mv(name)); - de.attachWrapper(v8Isolate, obj, TypeWrapper::template wrappableTag()); - - return kj::mv(de); - }); + return check(newDOMException(v8Isolate, v8Context(), str(name), str(message), stack)); + })); } // Returns the constructor function for a given type declared as JSG_RESOURCE_TYPE. @@ -1169,8 +1153,17 @@ class Isolate: public IsolateBase { // a Ref. auto context = wrapper->newContext(*this, options, jsgIsolate.getObserver(), static_cast(nullptr), kj::fwd(args)...); + auto handle = context.getHandle(v8Isolate); jsg::setAlignedPointerInEmbedderData( - context.getHandle(v8Isolate), jsg::ContextPointerSlot::EXTENDED_CONTEXT_WRAPPER, wrapper); + handle, jsg::ContextPointerSlot::EXTENDED_CONTEXT_WRAPPER, wrapper); + withinHandleScope([&] { + // Reading the function's `prototype` data property runs no JavaScript. + auto constructor = + check(wrapper->getTemplate(v8Isolate, static_cast(nullptr)) + ->GetFunction(handle)); + auto prototype = check(constructor->Get(handle, strIntern("prototype"_kj))); + setDOMExceptionPrototype(handle, prototype.template As()); + }); return context; } @@ -1204,11 +1197,9 @@ class Isolate: public IsolateBase { void reportError(const JsValue& value) override { auto& js = Lock::from(v8Isolate); - KJ_IF_SOME(domException, - jsgIsolate.getWrapperByContext(*this)->tryUnwrap( - js, v8Context(), value, static_cast(nullptr), kj::none)) { - auto desc = - kj::str("DOMException(", domException.getName(), "): ", domException.getMessage()); + KJ_IF_SOME(domException, tryGetDOMExceptionFields(v8Isolate, value)) { + auto desc = kj::str("DOMException(", JsString(domException.name).toString(js), + "): ", JsString(domException.message).toString(js)); jsgIsolate.reportError(*this, kj::mv(desc), value, JsMessage::create(*this, value)); } else { jsgIsolate.reportError( diff --git a/src/workerd/jsg/util.c++ b/src/workerd/jsg/util.c++ index 90dea9cb18c..df153d37690 100644 --- a/src/workerd/jsg/util.c++ +++ b/src/workerd/jsg/util.c++ @@ -271,8 +271,7 @@ DecodedException decodeTunneledException( auto& js = Lock::from(isolate); auto errorName = kj::str(errorType.first(closeParen)); auto message = appMessage(errorType.slice(1 + closeParen)); - auto exception = js.domException(kj::mv(errorName), kj::mv(message)); - result.handle = KJ_ASSERT_NONNULL(exception.tryGetHandle(js)); + result.handle = js.domException(kj::mv(errorName), kj::mv(message)); addAdditionalInfo(); return result; } diff --git a/src/workerd/jsg/value.h b/src/workerd/jsg/value.h index ab174964a7d..e1bc97dcd90 100644 --- a/src/workerd/jsg/value.h +++ b/src/workerd/jsg/value.h @@ -1473,11 +1473,10 @@ class ExceptionWrapper { return kj::Exception::Type::FAILED; }(); - KJ_IF_SOME(domException, - wrapper.tryUnwrap( - js, context, handle, static_cast(nullptr), parentObject)) { + KJ_IF_SOME(domException, tryGetDOMExceptionFields(js.v8Isolate, handle)) { return KJ_EXCEPTION(FAILED, - kj::str("jsg.DOMException(", domException.getName(), "): ", domException.getMessage())); + kj::str("jsg.DOMException(", JsString(domException.name).toString(js), + "): ", JsString(domException.message).toString(js))); } else { static const constexpr kj::StringPtr PREFIXES[] = { diff --git a/src/workerd/jsg/wrappable.h b/src/workerd/jsg/wrappable.h index c67262920c1..a2210f5b22e 100644 --- a/src/workerd/jsg/wrappable.h +++ b/src/workerd/jsg/wrappable.h @@ -81,6 +81,9 @@ enum class ContextDataSlot : int { CONSOLE_ORIGINAL_INFO, CONSOLE_ORIGINAL_LOG, CONSOLE_ORIGINAL_WARN, + // The prototype of the DOMExceptions that native code creates; see setDOMExceptionPrototype() + // in dom-exception.h. Rust JSG reads it too (src/rust/jsg/ffi.c++). + DOM_EXCEPTION_PROTOTYPE, }; // Returns the slot holding the saved original of the i'th decorated console method, counting