[piscisaureus]

Rust JSG's v8 module takes handles and returns handles. Converting
between a Rust value and a JavaScript value is the job of FromJS and
ToJS, not of a v8 method. A v8 method returns a Rust value only
where V8 itself returns a C scalar, from a typed handle (value,
length, value_of, is_*) or from one of V8's coercions of any
value (int32_value, uint32_value, integer_value), or copies
memory out (to_vec, to_string), and takes raw Rust data only where
V8 takes C data: constructor input, indices, times and &str property
keys. The one method that breaks the rule is Local<Function>::call,
which converts its result with FromJS; two thirds of its callers ask
for Local<Value>, that is, for no conversion.

A survey of the module also found a bug. Three C++ shims are declared
infallible but can throw a KJ exception: local_new_string,
exception_create_from_bytes and isolate_throw_error. All three
build their string through newUtf8String, which fails a KJ_REQUIRE
for input longer than v8::String::kMaxLength. An exception that
crosses a cxx bridge function not declared Result becomes a Rust
panic, so a string longer than V8's limit (about 512 MiB) returns
None from v8::String::new_from_str but panics through
"…".to_js(lock), although to_js returns a Result.

Commits, each of which builds and passes the tests on its own:

• Make Rust JSG's string and error shims unable to throw
• Return a handle from Rust JSG's Function::call
• Document the conventions of Rust JSG's v8 module

Strings

• ToJS for &str and String goes through
  v8::String::new_from_str and fails with
  Error::invalid_string_length(), a RangeError with V8's own
  message, "Invalid string length". The error is not pending and
  nothing has entered V8. The local_new_string shim is removed.
• A #[jsg_struct] converts its fields with ToJS (since Remove ToLocalValue from Rust JSG #8014), so
  an overlong String field gives the same RangeError.
• The v8::String constructors decide in Rust whether the input fits
  MAX_LENGTH, so None means exactly that, and no length above
  i32::MAX reaches V8's int parameter. V8's NewFromUtf8 measures
  its input against kMaxLength in bytes too, so the Rust check
  refuses exactly what V8 refuses. new_from_utf8_literal keeps its
  panic, as V8's NewFromUtf8Literal fails a static_assert.

Error messages

• An error message longer than the limit is cut to fit rather than
  refused. error_message_for_v8 cuts it in Rust at a UTF-8 boundary;
  exception_create_from_bytes takes a slice, clamps as a last
  defence and uses ToLocalChecked(), so it is total. A message that
  long is a bug or embeds user data, and its start is what a reader
  wants. Returning a Result would make Lock::error,
  Lock::throw_exception and the fast API error path fallible for a
  case no caller can act on, and a fixed replacement message would
  lose the information.
• isolate_throw_error is removed. Lock::throw_error creates the
  error with Lock::error and throws it through
  isolate_throw_exception, after the same termination check as
  before.
• newUtf8String returns a MaybeLocal and never throws; each caller
  decides. makeInternedStr keeps its KJ_REQUIRE: the property
  shims that intern a &str key and object_new_with_data_properties
  (used by #[jsg_struct] and, since Add jsg::Dict to Rust JSG #7998, jsg::Dict) are declared
  fallible, and resource template names come from Rust source.

Function::call

call returns Result<Local<Value>, Error>. A caller that wants a
Rust value converts the handle itself:

let result = f.call(lock, None, &[])?;
let n = f64::from_js(lock, result)?;

Every caller, the doc examples in lib.rs, v8.rs and the README,
the compile tests and the benchmark fixture are converted. With #8054
(no jsg::Number) and #8072 (integer FromJS/ToJS), the converted
tests read their results as u32, i32 or f64. src/rust/api,
node-exceptions, i18n and the server crate do not call it.

Conventions

The README gets a section "Conventions of the v8 module", and
src/rust/AGENTS.md a matching bullet: when a method returns or takes
a Rust value, when it returns Result or Option, that no method has
an infallible signature over a C++ shim that can throw, and the
as_*/to_*/value/*_value naming. It covers the APIs from
#8054 and #8072: Local<Number>::value and the other typed-handle
reads, Number::new and Integer::new as constructors taking C data,
and the int32_value/uint32_value/integer_value coercions, which
can run JavaScript and so return Result.

Tests

v8_length and utf8_prefix are unit-tested at the boundary and with
a small limit, since a real string of MAX_LENGTH + 1 bytes costs
512 MiB. The harness tests do use one, allocated zeroed so that it
costs no physical memory until read, which refusing it by length never
does: the constructors return None, to_js returns the RangeError,
and a method returning the string throws RangeError: Invalid string length to JavaScript. Lock::throw_error, Lock::range_error and
Error::invalid_string_length are checked from JavaScript. Removing
the length check, the UTF-8 back-off, the message cut, the error type
or the throw in throw_error each fails a test.

Related

• Remove ToLocalValue from Rust JSG #8014 removed ToLocalValue, so this change no longer has to make
  ToLocalValue for &str panic in Rust; that part is gone.
• Check for a pending exception in every Rust JSG shim that enters V8 #8009 adds a pending-exception guard to every shim; it touches
  ToJS for strings and the error shims, and this change is written
  to fit either order.

Not done here

• Renames that the conventions suggest: Local<String>::to_string (a
  lossy copy to a Rust String) and Value::type_of, which returns a
  Rust String rather than a handle.
• v8::String::MAX_LENGTH is a copy of kMaxLength, checked against
  a hard-coded number. The C++ clamp and V8's own check keep a drift
  from crashing anything, but the RangeError threshold would be off.
• A smaller cap for error messages than V8's string limit.

[clanky-the-clanker]

✅ Approved — No findings. The earlier README note has been fixed, and the rest of the change has no correctness, security or performance problems.

_{The decision is advisory: this comment never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🔒 Security: ✅ LGTM

No security issues. String::v8_length keeps lengths above MAX_LENGTH or i32::MAX from reaching V8's int parameter. Building an error cannot throw. The throw paths check for termination first.

---

📖 AGENTS.md: ✅ LGTM

The src/rust/AGENTS.md bullet is accurate and matches the README conventions section.

---

📋 Code Quality: ✅ LGTM

No code changed since the last review, which found no code-quality problems. utf8_prefix stays within bounds. The error-message cut and the C++ clamp use the same limit. The termination checks are still there. No references to the removed shims remain.

---

⚡ Performance: ✅ LGTM

The length checks are O(1), and the UTF-8 back-off moves at most three bytes. Function::call now skips a conversion that most callers did not need.

---

📝 Documentation: ✅ LGTM

src/rust/jsg/README.md:397-399 now limits the None statement to the constructors that take runtime input. The conventions section names String::new_from_utf8_literal as the exception that panics.

---

_{Reviewed commit: fd894c50}

[ask-bonk]

Review: 1 finding (1 warning; 1 note).

Updates Rust JSG string/error handling and changes Function::call to return V8 handles.

Other findings

• [SUGGESTION] src/rust/jsg/README.md:533-537: This new convention presents over-limit input as the Option case for all String constructors, but public String::new_from_utf8_literal() asserts and panics for an over-limit &'static str. Document that exception here and in the matching new src/rust/AGENTS.md convention so callers do not rely on None for that constructor.

  - A method that can run JavaScript, or that needs a context, returns a `Result`; its error is
    usually pending (see [Errors from JavaScript](#errors-from-javascript)). It returns an `Option`
    only for an absent value, as `int64_value` does for a `BigInt` that does not fit, or when V8
    refuses with nothing pending, as the ordinary `String` constructors do for input longer than
    `String::MAX_LENGTH`. `String::new_from_utf8_literal` instead panics for an over-limit literal,
    matching V8's compile-time assertion. A method never has an infallible signature over a C++ shim
    that can throw: the shim is declared `Result` in the bridge, or Rust decides before calling it,
    as `String::new_from_utf8` checks the length.
  

Specialist findings not posted (2)

• tests#1 [WARNING] src/rust/jsg/lib.rs:769: The new overlong-error-message behavior is only unit-tested as error_message_for_v8; none of the JSG tests invokes Lock::error (or Error::to_local) with an over-limit message. lock_errors exer… — The bound and UTF-8 truncation helper is directly unit-tested at the real limit; exercising Lock::error with it would require materializing a roughly 512 MiB V8 error message and is not necessary to e…
• docs#1 [INFO] src/rust/jsg/lib.rs:323: Error::to_local() no longer builds self.message verbatim: it now routes through create_error_from_bytes(), which truncates messages at V8's string limit. The new sentence below acknowledges the … — The sentence immediately following the verbatim wording explicitly documents the length-truncation exception, so this is not a distinct actionable documentation defect.

---

_{Reviewed commit: a8b1367b · github run}

[piscisaureus]

Addressed the README suggestion: the conventions section and the matching src/rust/AGENTS.md bullet now name String::new_from_utf8_literal as the exception that panics for an overlong literal, as V8's NewFromUtf8Literal fails a static_assert. After the rebase onto main the section also covers Local<Number>::value and the int32_value/uint32_value/integer_value coercions, which return Result.