fix(core): construct registered op errors natively

[nathanwhit]

Summary

• construct registered op errors through the native V8 path in both slow and fast dispatch
• preserve JavaScript builders for builder-only and constructor-dependent error classes
• define error fields as own data properties and keep constructor-registry lookups deterministic

Synchronous op errors previously used different construction paths depending on dispatch. The slow path always called buildCustomError, so a registered class could invoke its constructor or prototype accessors while the op callback was still active. Registered ordinary error classes now use the same native construction path in both cases, while custom builders continue to handle objects that require constructor-specific state, including WebGPU errors.

The current V8 API no longer exposes the deprecated fast-callback fallback flag, so the fast path remains on native construction rather than attempting to repeat an op through the slow callback.

Testing

• ./tools/format.js --check
• cargo test -p deno_core --lib
• cargo test -p deno_ops
• cargo test -p unit_tests --test unit webgpu_test -- --nocapture
• cargo clippy -p deno_core -p deno_ops --all-targets -- -D warnings
• cargo build --bin deno

[bartlomieju]

Nice cleanup unifying the two dispatch paths. Everything below is latent against the current in-tree error classes rather than an active bug, but a few are worth addressing. Left inline notes; the test-coverage one and the registration-time-snapshot suggestion are the two I'd most want to see resolved.

[bartlomieju]

This test op never exercises the fast-call path the fix is about. #[buffer(detach)] hits dispatch_fast.rs's Arg::Buffer(_, BufferMode::Detach, _) => return Ok(None), so no fast callback is generated and ops.op_registered_err(new Uint8Array()) in error_builder_test.js routes through the slow path (try_throw_js_error_class).

The GHSA-p4r3-6jgx-4cj5 scenario is specifically a buffer-carrying fast op throwing while buildCustomError runs attacker JS — a detach-mode arg can't reproduce it. So the fast path is only covered by the Rust unit test calling throw_js_error_class directly, never through real fast dispatch with a live buffer arg. A regression that broke only the fast path would leave this JS test green. Consider a regular (non-detach) buffer arg so the op is actually fast-call eligible.

[bartlomieju]

This switches the clobber check from prototype-chain to own-only, which diverges from the buildCustomError path you're claiming to mirror. buildCustomError used if (!(key in error)), skipping a key defined anywhere on the class prototype chain; has_own_property only looks at the exception itself.

So a registered class that exposes e.g. code via a prototype getter would previously keep that getter authoritative, but now an op-supplied code additional property is defined as an own data property that shadows it. Latent today (no in-tree registered class does this), but it's a silent behavioral difference between the native and builder paths worth a comment at least.

[bartlomieju]

Two notes here:

1. Behavioral change worth documenting. Registered-class constructors no longer run on the slow path — pre-PR it went through to_v8_error -> buildCustomError -> new ErrorClass(msg). In-tree classes only do super(msg) + this.name so shapes match, but a deno_core embedder whose registered-class constructor computes derived own properties or has side effects will now see those dropped on op errors. Might be worth a line in the PR body since it's an observable change to the public op-error contract.

2. Altitude/binary size. This if !try_throw_js_error_class { to_v8_error; throw } branch is macro-expanded verbatim into every generated slow-path stub (the 11 changed .out fixtures). Moving it behind a single deno_core::error helper (e.g. throw_op_error(scope, &err)) would shrink expanded code across hundreds of ops and make a future fallback-policy change one edit instead of a proc-macro change plus regenerating every fixture.

[bartlomieju]

This re-derives trust from the live JS object on every throw: two get_own_property_descriptor calls, proxy/accessor/own-data-property checks, and several v8::String::new allocations per throw on a hot path.

The entire proxy/accessor defense here and the new immutable ObjectDefineProperty in 00_infra.js exist only because the prototype is looked up at throw time from a mutable object. Since registerErrorClass now installs immutable entries, each class's prototype is fixed at registration — snapshotting it into a Rust-side HashMap<String, v8::Global<v8::Value>> (or a field on ExceptionState) when registerErrorClass runs would reduce throwing to a map lookup and let you delete this whole threat model rather than defend against it per-throw. Worth considering as the deeper fix.

[bartlomieju]

throw_js_error_class (above) and try_throw_js_error_class duplicate the get_class + prototype-lookup + build + throw sequence. The former is behaviorally if !try_throw_js_error_class(scope, error) { build with prototype=None; throw } — expressing it that way keeps fast- and slow-call error shapes from silently diverging when one is edited.

Relatedly, the get_own_property_descriptor -> cast -> has_own_property("value") -> get("value") dance in registered_error_class_prototype appears twice back-to-back; since it encodes the security-critical "never read through an accessor" invariant, a single own_data_property(scope, obj, key) helper would keep both copies from drifting.

[bartlomieju]

Downgrading these to builder-only creates a slow/fast asymmetry that's currently invisible: builder-only classes get the to_v8_error fallback on the slow path, but the fast path (throw_js_error_class) has no fallback and returns a plain Error with the right name string but the base Error prototype, so e instanceof GPUValidationError would be false.

Harmless today — all webgpu ops returning GPUError take a scope/webidl args and are slow-only — but if a fast op ever returns one it fails silently. The selection rule ("classes with private-slot / webidl-brand state must be builder-only") isn't documented anywhere, and everything else defaults to registerErrorClass, so the next person adding a brand-backed error class will hit this at throw time. A short comment here explaining why these three can't use registerErrorClass would help.