[piscisaureus]

This builds on #7997, which made Rust JSG's ToJS fallible and gave
wrap_resource a check for a termination. This change applies that
check to every shim that enters V8 with a context, and to any pending
exception, not only a termination.

What V8 does: an API that takes a v8::Local<v8::Context> must not be
entered while an exception or a termination is pending. How it goes
wrong depends on how the API enters V8 (src/api/api-inl.h):

• PrepareForExecutionScope clears the pending exception on entry, a
  termination included. The exception is lost and JavaScript runs on
  as if nothing had happened. Object::Get, ToString of a
  non-string, ObjectTemplate::NewInstance,
  FunctionTemplate::GetFunction, Function::New, Date::New and the
  promise APIs enter this way.
• EnterV8Scope and EnterV8NoScriptScope don't clear it. They fail
  a debug check, and in a release build run on with the exception
  still pending. Through EnterV8Scope that can run user JavaScript
  while an exception is in flight. Function::Call, Object::Has,
  Object::Set and NumberValue enter this way, and
  ArrayBuffer::Detach through EnterV8NoScriptScope.
• Creating strings, numbers, BigInts, symbols, plain objects and
  arrays, typed arrays, error objects and templates takes no context
  and is fine at any time, also while execution is terminating.

Before this change, call_into_js() checked
isolate_is_execution_terminating from Rust before each shim. That
cost an extra FFI call per operation, missed an ordinary pending
exception, and didn't cover the shims that don't go through
call_into_js(). A few operations (Object::set, Array::set,
ArrayBuffer::detach, Utf8Value::new) could not report a pending
exception at all.

• Every C++ shim that passes a context to V8 first checks
  exceptionPending(isolate) (a termination or
  HasPendingException(), which the promise shims already used) and
  fails with an empty result or false without entering V8. Rust
  turns that into a pending jsg::Error, as it does for an exception
  the call itself throws. A shim that skips the check says why in a
  comment (local_type_of). The Rust pre-check in call_into_js()
  and in Function::new is removed.
• Object::set, Array::set, ArrayBuffer::detach and
  Utf8Value::new now return a Result. Set leaves what a setter or
  Proxy trap throws pending, rather than panicking through
  jsg::check, so ffi.c++ no longer uses jsg::check. Array::set
  takes a u32 index, as V8 does. Utf8Value::new converts with
  ToString itself, because v8::String::Utf8Value catches and
  discards what toString throws; a Symbol now fails with V8's
  TypeError instead of reading as an empty string.
  function_template_of and as_local_function return a Result
  too.
• ToJS for Vec<T> builds the array with
  v8::Array::New(isolate, elements, length), as C++ JSG does. That
  takes no context and runs no setter on Array.prototype. A
  #[jsg_struct]'s ToJS and the node exception helpers pass on what
  setting a property throws.
• The README, the AGENTS files, the review checklist and the review
  prompts state the rule as V8 requires it, and tell reviewers not to
  flag creating values while execution is terminating. Comments that
  said creating an error object while terminating fails a debug check
  are corrected.
• Tests in a new pending_exception.rs and in termination.rs,
  coercion_safety.rs, resource_callback.rs, string.rs,
  arrays.rs and jsg_struct.rs: with an exception or a termination
  pending, each guarded operation runs no JavaScript and the exception
  reaches JavaScript unchanged; what a setter or toString throws is
  passed on. Removing any guard except function_new's fails a test.
  function_new is called only after the opaque wrapper, whose shim
  already checks.

Design choice: the check lives in each C++ shim, next to the V8 call
it protects, and it fails whatever the value. Converting a string
primitive or wrapping a resource that already has a wrapper would run
no JavaScript, but these fail too while an exception is pending. One
rule per shim is easier to review than a check that depends on the
value, and these calls have no reason to run while an exception is
propagating.

"Pending" includes an exception that the innermost v8::TryCatch has
caught and not yet reset, because that is what HasPendingException()
reports. So C++ code that catches with its own v8::TryCatch must
reset it before it calls into Rust JSG, or runs JavaScript that might.
jsg::Lock::tryCatch() and JSG_TRY/JSG_CATCH already leave the
TryCatch's scope first. Two places did not, and now reset first:
handleLog(), which kept stringifying console.log() arguments for
the trace after one threw, and IoContext::runImpl(), which logged and
tunneled an exception while its TryCatch still held it. Reading the
exception's properties can run getters. A test in
caught-exception-reset-test calls node:url from both.

Alternatives considered:

• Keep a Rust pre-check in call_into_js(), widened to any pending
  exception. It costs an FFI call per operation, and the shims that
  don't go through call_into_js() stay unguarded.
• Check only for a termination, as Make Rust JSG's ToJS fallible #7997 does for wrap_resource.
  That still lets JavaScript run with an ordinary exception pending,
  which V8 allows in release builds and which can replace or hide the
  first exception.
• Wrap each shim in a v8::TryCatch. That catches what the call
  throws, but does nothing about an exception that is already
  pending, and would hide exceptions that must keep propagating.
• Have a #[jsg_struct]'s ToJS define its fields, as C++ JSG's
  fastJsgStruct path does with a DictionaryTemplate, so that no
  setter runs. Rust JSG's structs follow C++ JSG's slow path, which
  sets fields and omits absent optional ones. Changing that is a
  separate behaviour change.

[piscisaureus]

Follow-up idea, not for this PR: make the rule part of the types instead of a runtime check in each shim.

Today a call that can run JavaScript returns Result<T, jsg::Error>, and nothing stops Rust from calling V8 again after it fails. Instead, such calls could return Result<Local<'s, T>, PendingException<'s>>, where PendingException holds a mutable borrow of the scope until it is consumed or dropped. While it is alive, only what it exposes compiles:

• passing it on to the caller with ?, or rethrowing it;
• handling it through a TryCatch;
• creating values and error objects, which take no context and are safe with an exception pending.

Calling into V8 again after a failure would then be a compile error, not something reviewers have to catch.

The guard from this PR would stay as a backstop. A termination requested from another thread only takes effect at V8's next stack check, so it can start inside any call, and a pending exception can also come from a C++ caller.

This fits best in the rusty_v8-based bindings, where the borrow checker already tracks scopes.

For comparison, rusty_v8 and Deno have neither a guard nor a type like this. Deno checks the TryCatch after each call and relies on V8 clearing a termination at the top level. It has had bugs of this kind: denoland/deno#15012 and denoland/deno_core#450.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 13.79310% with 175 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.94%. Comparing base (1991918) to head (726ff5f).

Files with missing lines	Patch %	Lines
src/rust/jsg/v8.rs	13.33%	65 Missing ⚠️
src/workerd/io/caught-exception-reset-test.js	0.00%	64 Missing ⚠️
src/rust/jsg/ffi.c++	5.12%	34 Missing and 3 partials ⚠️
src/rust/jsg/wrappable.rs	0.00%	5 Missing ⚠️
src/rust/jsg/ffi.h	50.00%	0 Missing and 1 partial ⚠️
src/rust/jsg/resource.rs	0.00%	1 Missing ⚠️
src/rust/node-exceptions/lib.rs	92.30%	1 Missing ⚠️
src/workerd/io/worker.c++	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8009      +/-   ##
==========================================
- Coverage   41.95%   41.94%   -0.02%     
==========================================
  Files        1064     1065       +1     
  Lines      302162   302266     +104     
  Branches    29082    29099      +17     
==========================================
+ Hits       126775   126781       +6     
- Misses     158350   158446      +96     
- Partials    17037    17039       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.