Repository: denoland/deno_core
Pull request #767 (2024-06-06 15:42:41)

Add non-panic constructor for runtimes
Added a new constructor variant that does not panic
Existing constructor can then simply take in the returned error and panic


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 2674abc..09f022e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2648,9 +2648,9 @@ dependencies = [
 
 [[package]]
 name = "v8"
-version = "0.92.0"
+version = "0.93.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "234589219e37a7496cbce73d971586db8369871be2420372c45a579b6a919b15"
+checksum = "abc3ff12243d345cc697c151b29fbb8ad1f898e3f8b7aa1386701ff9f2c7cbbf"
 dependencies = [
  "bitflags",
  "fslock",
@@ -2758,15 +2758,14 @@ dependencies = [
 
 [[package]]
 name = "which"
-version = "5.0.0"
+version = "6.0.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9bf3ea8596f3a0dd5980b46430f2058dfe2c36a27ccfbb1845d6fbfcd9ba6e14"
+checksum = "8211e4f58a2b2805adfbefbc07bab82958fc91e3836339b1ab7ae32465dce0d7"
 dependencies = [
  "either",
  "home",
- "once_cell",
  "rustix",
- "windows-sys 0.48.0",
+ "winsafe",
 ]
 
 [[package]]
@@ -2948,6 +2947,12 @@ dependencies = [
  "memchr",
 ]
 
+[[package]]
+name = "winsafe"
+version = "0.0.19"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904"
+
 [[package]]
 name = "wyz"
 version = "0.5.1"
diff --git a/Cargo.toml b/Cargo.toml
index fd79656..2caa451 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -24,7 +24,7 @@ deno_ops = { version = "0.160.0", path = "./ops" }
 serde_v8 = { version = "0.193.0", path = "./serde_v8" }
 deno_core_testing = { path = "./testing" }
 
-v8 = { version = "0.92.0", default-features = false }
+v8 = { version = "0.93.0", default-features = false }
 deno_ast = { version = "=0.35.3", features = ["transpiling"] }
 deno_unsync = "0.3.2"
 deno_core_icudata = "0.0.73"
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index 8768c3f..ede85ec 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -605,13 +605,9 @@ impl JsRuntime {
   }
 
   /// Only constructor, configuration is done through `options`.
-  pub fn new(mut options: RuntimeOptions) -> JsRuntime {
-    setup::init_v8(
-      options.v8_platform.take(),
-      cfg!(test),
-      options.unsafe_expose_natives_and_gc(),
-    );
-    match JsRuntime::new_inner(options, false) {
+  /// Panics if the runtime cannot be initialized.
+  pub fn new(options: RuntimeOptions) -> JsRuntime {
+    match Self::try_new(options) {
       Ok(runtime) => runtime,
       Err(err) => {
         panic!("Failed to initialize a JsRuntime: {:?}", err);
@@ -619,6 +615,17 @@ impl JsRuntime {
     }
   }
 
+  /// Only constructor, configuration is done through `options`.
+  /// Returns an error if the runtime cannot be initialized.
+  pub fn try_new(mut options: RuntimeOptions) -> Result<JsRuntime, Error> {
+    setup::init_v8(
+      options.v8_platform.take(),
+      cfg!(test),
+      options.unsafe_expose_natives_and_gc(),
+    );
+    JsRuntime::new_inner(options, false)
+  }
+
   pub(crate) fn state_from(isolate: &v8::Isolate) -> Rc<JsRuntimeState> {
     let state_ptr = isolate.get_data(STATE_DATA_OFFSET);
     let state_rc =
@@ -1884,20 +1891,28 @@ fn create_context<'a>(
 }
 
 impl JsRuntimeForSnapshot {
-  pub fn new(mut options: RuntimeOptions) -> JsRuntimeForSnapshot {
+  /// Create a new runtime, panicking if the process fails.
+  pub fn new(options: RuntimeOptions) -> JsRuntimeForSnapshot {
+    match Self::try_new(options) {
+      Ok(runtime) => runtime,
+      Err(err) => {
+        panic!("Failed to initialize JsRuntime for snapshotting: {:?}", err);
+      }
+    }
+  }
+
+  /// Try to create a new runtime, returning an error if the process fails.
+  pub fn try_new(
+    mut options: RuntimeOptions,
+  ) -> Result<JsRuntimeForSnapshot, Error> {
     setup::init_v8(
       options.v8_platform.take(),
       true,
       options.unsafe_expose_natives_and_gc(),
     );
 
-    let runtime = match JsRuntime::new_inner(options, true) {
-      Ok(r) => r,
-      Err(err) => {
-        panic!("Failed to initialize JsRuntime for snapshotting: {:?}", err);
-      }
-    };
-    JsRuntimeForSnapshot(runtime)
+    let runtime = JsRuntime::new_inner(options, true)?;
+    Ok(JsRuntimeForSnapshot(runtime))
   }
 
   /// Takes a snapshot and consumes the runtime.


Test patch:
diff --git a/core/runtime/tests/misc.rs b/core/runtime/tests/misc.rs
index ba94cc2..1ba8ff8 100644
--- a/core/runtime/tests/misc.rs
+++ b/core/runtime/tests/misc.rs
@@ -1243,8 +1243,9 @@ async fn global_template_middleware() {
     _key: v8::Local<'s, v8::Name>,
     _args: v8::PropertyCallbackArguments<'s>,
     _rv: v8::ReturnValue,
-  ) {
-    unsafe { CALLS.push("descriptor".to_string()) }
+  ) -> v8::Intercepted {
+    unsafe { CALLS.push("descriptor".to_string()) };
+    v8::Intercepted::No
   }
 
   pub fn setter<'s>(
@@ -1253,10 +1254,11 @@ async fn global_template_middleware() {
     _value: v8::Local<'s, v8::Value>,
     _args: v8::PropertyCallbackArguments<'s>,
     _rv: v8::ReturnValue,
-  ) {
+  ) -> v8::Intercepted {
     unsafe {
       CALLS.push("setter".to_string());
-    }
+    };
+    v8::Intercepted::No
   }
 
   fn definer<'s>(
@@ -1265,10 +1267,11 @@ async fn global_template_middleware() {
     _descriptor: &v8::PropertyDescriptor,
     _args: v8::PropertyCallbackArguments<'s>,
     _rv: v8::ReturnValue,
-  ) {
+  ) -> v8::Intercepted {
     unsafe {
       CALLS.push("definer".to_string());
-    }
+    };
+    v8::Intercepted::No
   }
 
   pub fn gt_middleware<'s>(