isolate2: Catch v8 op panics in tests to improve debugging (#34991)

GitOrigin-RevId: a1a99d3b7152b771fcc58eb864311df21f833d4f

Author: goffrie

crates/isolate/src/isolate2/callback_context.rs

@@ -154,7 +154,16 @@ impl<'callback, 'scope: 'callback> CallbackContext<'callback, 'scope> {
         rv: v8::ReturnValue,
     ) {
         let mut ctx = CallbackContext::new(scope);
-        if let Err(e) = run_op(&mut ctx, args, rv) {
+        #[cfg(test)]
+        let r = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            run_op(&mut ctx, args, rv)
+        })) {
+            Ok(r) => r,
+            Err(e) => Err(crate::test_helpers::PanicError::new(e).into()),
+        };
+        #[cfg(not(test))]
+        let r = run_op(&mut ctx, args, rv);
+        if let Err(e) = r {
             ctx.handle_syscall_or_op_error(e);
         }
     }
@@ -165,7 +174,16 @@ impl<'callback, 'scope: 'callback> CallbackContext<'callback, 'scope> {
         rv: v8::ReturnValue,
     ) {
         let mut ctx = CallbackContext::new(scope);
-        if let Err(e) = start_async_op(&mut ctx, args, rv) {
+        #[cfg(test)]
+        let r = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            start_async_op(&mut ctx, args, rv)
+        })) {
+            Ok(r) => r,
+            Err(e) => Err(crate::test_helpers::PanicError::new(e).into()),
+        };
+        #[cfg(not(test))]
+        let r = start_async_op(&mut ctx, args, rv);
+        if let Err(e) = r {
             ctx.handle_syscall_or_op_error(e);
         }
     }

crates/isolate/src/isolate2/entered_context.rs

@@ -135,7 +135,14 @@ impl<'enter, 'scope: 'enter> EnteredContext<'enter, 'scope> {
             if let Some(failure) = context_state.failure.take() {
                 match failure {
                     ContextFailure::UncatchableDeveloperError(js_error) => anyhow::bail!(js_error),
-                    ContextFailure::SystemError(error) => anyhow::bail!(error),
+                    ContextFailure::SystemError(error) => {
+                        #[cfg(test)]
+                        let error = match error.downcast::<crate::test_helpers::PanicError>() {
+                            Ok(panic) => std::panic::resume_unwind(panic.into_inner()),
+                            Err(e) => e,
+                        };
+                        anyhow::bail!(error)
+                    },
                 }
             } else if self.heap_context.oomed() {
                 // TODO: do the rest of the logging that isolate1 does

crates/isolate/src/lib.rs

@@ -1,4 +1,5 @@
 #![feature(const_type_name)]
+#![feature(exclusive_wrapper)]
 #![feature(try_blocks)]
 #![feature(iterator_try_collect)]
 #![feature(type_alias_impl_trait)]

crates/isolate/src/test_helpers.rs

@@ -1,9 +1,11 @@
 use std::{
+    any::Any,
     collections::BTreeMap,
     fs::File,
     io::Read,
     sync::{
         Arc,
+        Exclusive,
         LazyLock,
     },
     time::Duration,
@@ -241,6 +243,29 @@ pub fn test_environment_data<RT: Runtime>(rt: RT) -> anyhow::Result<EnvironmentD
     })
 }
 
+/// Indicates that a V8 op panicked and we should panic the test
+#[derive(thiserror::Error)]
+#[error("panicked")]
+// Wrap the error in `Exclusive` to make this type `Sync` so that it can be
+// wrapped by anyhow
+pub struct PanicError(Exclusive<Box<dyn Any + Send>>);
+
+impl PanicError {
+    pub fn new(panic: Box<dyn Any + Send>) -> Self {
+        Self(Exclusive::new(panic))
+    }
+
+    pub fn into_inner(self) -> Box<dyn Any + Send> {
+        self.0.into_inner()
+    }
+}
+
+impl std::fmt::Debug for PanicError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_tuple("PanicError").finish()
+    }
+}
+
 pub struct UdfTest<RT: Runtime, P: Persistence> {
     pub database: Database<RT>,
     pub isolate: IsolateClient<RT>,