Repository: denoland/deno_core
Pull request #82 (2023-07-24 14:57:01)

fix(realms): Call the right realm's format exception callback for promise rejections
The `JsRuntime::check_promise_rejection` method takes care of failing the event loop run if it detects any rejected promise in any realm. It does so by delegating to `JsRealmInner::check_promise_rejection` for every realm; however, the scope it passes to that method is `runtime.handle_scope()` – that is, the main realm's scope. The `JsRealmInner` method in turn uses this scope, which results in the main realm's format exception callback being called, rather than the actual realm's.

This patch fixes this by changing `JsRealmInner::check_promise_rejection` to take a `&mut v8::Isolate` rather than a `&mut v8::HandleScope`, and to call `Self::handle_scope` to create a scope in the realm, as the rest of `JsRealm` and `JsRealmInner` methods do. It also adds realm-specific tests for the format exception callback.



Patch:
diff --git a/core/runtime/jsrealm.rs b/core/runtime/jsrealm.rs
index 41d8da7..3f467d4 100644
--- a/core/runtime/jsrealm.rs
+++ b/core/runtime/jsrealm.rs
@@ -208,12 +208,13 @@ impl JsRealmInner {
 
   pub(crate) fn check_promise_rejections(
     &self,
-    scope: &mut v8::HandleScope,
+    isolate: &mut v8::Isolate,
   ) -> Result<(), Error> {
     let Some((_, handle)) = self.context_state.borrow_mut().pending_promise_rejections.pop_front() else {
       return Ok(());
     };
 
+    let scope = &mut self.handle_scope(isolate);
     let exception = v8::Local::new(scope, handle);
     let state_rc = JsRuntime::state_from(scope);
     let state = state_rc.borrow();
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index 0ee845b..b125cdc 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -1810,11 +1810,11 @@ impl JsRuntime {
   }
 
   fn check_promise_rejections(&mut self) -> Result<(), Error> {
-    let state = self.inner.state.clone();
-    let scope = &mut self.handle_scope();
-    let state = state.borrow();
+    let state_rc = self.inner.state.clone();
+    let isolate = &mut self.v8_isolate();
+    let state = state_rc.borrow();
     for realm in &state.known_realms {
-      realm.check_promise_rejections(scope)?;
+      realm.check_promise_rejections(isolate)?;
     }
     Ok(())
   }


Test patch:
diff --git a/core/runtime/tests/jsrealm.rs b/core/runtime/tests/jsrealm.rs
index ffd1c85..b517972 100644
--- a/core/runtime/tests/jsrealm.rs
+++ b/core/runtime/tests/jsrealm.rs
@@ -63,6 +63,75 @@ async fn test_set_promise_reject_callback_realms() {
   }
 }
 
+#[test]
+fn test_set_format_exception_callback_realms() {
+  let mut runtime = JsRuntime::new(RuntimeOptions::default());
+  let main_realm = runtime.main_realm();
+  let realm1 = runtime.create_realm(Default::default()).unwrap();
+  let realm2 = runtime.create_realm(Default::default()).unwrap();
+
+  let realm_expectations = &[
+    (&main_realm, "main_realm"),
+    (&realm1, "realm1"),
+    (&realm2, "realm2"),
+  ];
+
+  // Set up format exception callbacks.
+  for (realm, realm_name) in realm_expectations {
+    realm
+      .execute_script(
+        runtime.v8_isolate(),
+        "",
+        format!(
+          r#"
+            Deno.core.ops.op_set_format_exception_callback((error) => {{
+              return `{realm_name} / ${{error}}`;
+            }});
+          "#
+        )
+        .into(),
+      )
+      .unwrap();
+  }
+
+  for (realm, realm_name) in realm_expectations {
+    // Immediate exceptions
+    {
+      let result = realm.execute_script(
+        runtime.v8_isolate(),
+        "",
+        format!("throw new Error('{realm_name}');").into(),
+      );
+      assert!(result.is_err());
+
+      let error = result.unwrap_err().downcast::<error::JsError>().unwrap();
+      assert_eq!(
+        error.exception_message,
+        format!("{realm_name} / Error: {realm_name}")
+      );
+    }
+
+    // Promise rejections
+    {
+      realm
+        .execute_script(
+          runtime.v8_isolate(),
+          "",
+          format!("Promise.reject(new Error('{realm_name}'));").into(),
+        )
+        .unwrap();
+
+      let result = futures::executor::block_on(runtime.run_event_loop(false));
+      assert!(result.is_err());
+      let error = result.unwrap_err().downcast::<error::JsError>().unwrap();
+      assert_eq!(
+        error.exception_message,
+        format!("Uncaught (in promise) {realm_name} / Error: {realm_name}")
+      );
+    }
+  }
+}
+
 #[test]
 fn js_realm_simple() {
   let mut runtime = JsRuntime::new(Default::default());