Repository: denoland/deno_core (rust)

Issue:
Deno.core.close(null) coerces to null to 0
```ts
  assertThrows(() => {
    Deno.close((null as unknown) as number);
  }, TypeError);
```

fails because instead of throwing `TypeError` the `rid` inside `op_close` is actually 0.

Pull request:
fix(ops): don't coerce non-number v8 types to 0
Closes #63 


Patch:
diff --git a/core/runtime/ops.rs b/core/runtime/ops.rs
index b8e945c..f39f3a7 100644
--- a/core/runtime/ops.rs
+++ b/core/runtime/ops.rs
@@ -228,86 +228,68 @@ pub fn map_async_op_fallible<R: 'static, E: Into<Error> + 'static>(
   None
 }
 
-macro_rules! try_number {
+macro_rules! try_number_some {
   ($n:ident $type:ident $is:ident) => {
     if $n.$is() {
       // SAFETY: v8 handles can be transmuted
       let n: &v8::$type = unsafe { std::mem::transmute($n) };
-      return n.value() as _;
+      return Some(n.value() as _);
     }
   };
 }
 
-pub fn to_u32(number: &v8::Value) -> u32 {
-  try_number!(number Uint32 is_uint32);
-  try_number!(number Int32 is_int32);
-  try_number!(number Number is_number);
-  if number.is_big_int() {
-    // SAFETY: v8 handles can be transmuted
-    let n: &v8::BigInt = unsafe { std::mem::transmute(number) };
-    return n.u64_value().0 as _;
-  }
-  0
+macro_rules! try_bignum {
+  ($n:ident $method:ident) => {
+    if $n.is_big_int() {
+      // SAFETY: v8 handles can be transmuted
+      let $n: &v8::BigInt = unsafe { std::mem::transmute($n) };
+      return Some($n.$method().0 as _);
+    }
+  };
 }
 
-pub fn to_i32(number: &v8::Value) -> i32 {
-  try_number!(number Uint32 is_uint32);
-  try_number!(number Int32 is_int32);
-  try_number!(number Number is_number);
-  if number.is_big_int() {
-    // SAFETY: v8 handles can be transmuted
-    let n: &v8::BigInt = unsafe { std::mem::transmute(number) };
-    return n.i64_value().0 as _;
-  }
-  0
+pub fn to_u32_option(number: &v8::Value) -> Option<i32> {
+  try_number_some!(number Integer is_uint32);
+  try_number_some!(number Int32 is_int32);
+  try_number_some!(number Number is_number);
+  try_bignum!(number u64_value);
+  None
 }
 
-pub fn to_u64(number: &v8::Value) -> u32 {
-  try_number!(number Uint32 is_uint32);
-  try_number!(number Int32 is_int32);
-  try_number!(number Number is_number);
-  if number.is_big_int() {
-    // SAFETY: v8 handles can be transmuted
-    let n: &v8::BigInt = unsafe { std::mem::transmute(number) };
-    return n.u64_value().0 as _;
-  }
-  0
+pub fn to_i32_option(number: &v8::Value) -> Option<i32> {
+  try_number_some!(number Uint32 is_uint32);
+  try_number_some!(number Int32 is_int32);
+  try_number_some!(number Number is_number);
+  try_bignum!(number i64_value);
+  None
 }
 
-pub fn to_i64(number: &v8::Value) -> i32 {
-  try_number!(number Uint32 is_uint32);
-  try_number!(number Int32 is_int32);
-  try_number!(number Number is_number);
-  if number.is_big_int() {
-    // SAFETY: v8 handles can be transmuted
-    let n: &v8::BigInt = unsafe { std::mem::transmute(number) };
-    return n.i64_value().0 as _;
-  }
-  0
+pub fn to_u64_option(number: &v8::Value) -> Option<u64> {
+  try_number_some!(number Integer is_uint32);
+  try_number_some!(number Int32 is_int32);
+  try_number_some!(number Number is_number);
+  try_bignum!(number u64_value);
+  None
 }
 
-pub fn to_f32(number: &v8::Value) -> f32 {
-  try_number!(number Uint32 is_uint32);
-  try_number!(number Int32 is_int32);
-  try_number!(number Number is_number);
-  if number.is_big_int() {
-    // SAFETY: v8 handles can be transmuted
-    let n: &v8::BigInt = unsafe { std::mem::transmute(number) };
-    return n.i64_value().0 as _;
-  }
-  0.0
+pub fn to_i64_option(number: &v8::Value) -> Option<i64> {
+  try_number_some!(number Integer is_uint32);
+  try_number_some!(number Int32 is_int32);
+  try_number_some!(number Number is_number);
+  try_bignum!(number u64_value);
+  None
 }
 
-pub fn to_f64(number: &v8::Value) -> f64 {
-  try_number!(number Uint32 is_uint32);
-  try_number!(number Int32 is_int32);
-  try_number!(number Number is_number);
-  if number.is_big_int() {
-    // SAFETY: v8 handles can be transmuted
-    let n: &v8::BigInt = unsafe { std::mem::transmute(number) };
-    return n.i64_value().0 as _;
-  }
-  0.0
+pub fn to_f32_option(number: &v8::Value) -> Option<f32> {
+  try_number_some!(number Number is_number);
+  try_bignum!(number i64_value);
+  None
+}
+
+pub fn to_f64_option(number: &v8::Value) -> Option<f64> {
+  try_number_some!(number Number is_number);
+  try_bignum!(number i64_value);
+  None
 }
 
 /// Expands `inbuf` to `outbuf`, assuming that `outbuf` has at least 2x `input_length`.
diff --git a/ops/op2/dispatch_async.rs b/ops/op2/dispatch_async.rs
index d8d4913..dc202e4 100644
--- a/ops/op2/dispatch_async.rs
+++ b/ops/op2/dispatch_async.rs
@@ -100,9 +100,7 @@ pub(crate) fn generate_dispatch_async(
   }
 
   output.extend(gs_quote!(generator_state(promise_id, fn_args, result, opctx, scope, deno_core) => {
-    // TODO(mmastrac): We are extending the eager polling behaviour temporarily to op2, but I would like to get
-    // rid of it as soon as we can
-    let #promise_id = #deno_core::_ops::to_i32(&#fn_args.get(0));
+    let #promise_id = #deno_core::_ops::to_i32_option(&#fn_args.get(0)).unwrap_or_default();
     if let Some(#result) = #deno_core::_ops::#mapper(#opctx, #promise_id, #result, |#scope, #result| {
       #return_value
     }) {
diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
index 1c9747c..3cb2bf0 100644
--- a/ops/op2/dispatch_slow.rs
+++ b/ops/op2/dispatch_slow.rs
@@ -210,37 +210,25 @@ pub fn from_arg(
     Arg::Numeric(NumericArg::u8)
     | Arg::Numeric(NumericArg::u16)
     | Arg::Numeric(NumericArg::u32) => {
-      quote! {
-        let #arg_ident = #deno_core::_ops::to_u32(&#arg_ident) as _;
-      }
+      from_arg_numeric(generator_state, &arg_ident, "u32")?
     }
     Arg::Numeric(NumericArg::i8)
     | Arg::Numeric(NumericArg::i16)
     | Arg::Numeric(NumericArg::i32)
     | Arg::Numeric(NumericArg::__SMI__) => {
-      quote! {
-        let #arg_ident = #deno_core::_ops::to_i32(&#arg_ident) as _;
-      }
+      from_arg_numeric(generator_state, &arg_ident, "i32")?
     }
     Arg::Numeric(NumericArg::u64) | Arg::Numeric(NumericArg::usize) => {
-      quote! {
-        let #arg_ident = #deno_core::_ops::to_u64(&#arg_ident) as _;
-      }
+      from_arg_numeric(generator_state, &arg_ident, "u64")?
     }
     Arg::Numeric(NumericArg::i64) | Arg::Numeric(NumericArg::isize) => {
-      quote! {
-        let #arg_ident = #deno_core::_ops::to_i64(&#arg_ident) as _;
-      }
+      from_arg_numeric(generator_state, &arg_ident, "i64")?
     }
     Arg::Numeric(NumericArg::f32) => {
-      quote! {
-        let #arg_ident = #deno_core::_ops::to_f32(&#arg_ident) as _;
-      }
+      from_arg_numeric(generator_state, &arg_ident, "f32")?
     }
     Arg::Numeric(NumericArg::f64) => {
-      quote! {
-        let #arg_ident = #deno_core::_ops::to_f64(&#arg_ident) as _;
-      }
+      from_arg_numeric(generator_state, &arg_ident, "f64")?
     }
     Arg::OptionNumeric(numeric) => {
       let some = from_arg(generator_state, index, &Arg::Numeric(*numeric))?;
@@ -399,6 +387,22 @@ pub fn from_arg(
   Ok(res)
 }
 
+pub fn from_arg_numeric(
+  generator_state: &mut GeneratorState,
+  arg_ident: &Ident,
+  numeric: &str,
+) -> Result<TokenStream, V8MappingError> {
+  let exception =
+    throw_type_error(generator_state, format!("expected {numeric}"))?;
+  let convert = format_ident!("to_{numeric}_option");
+  Ok(gs_quote!(generator_state(deno_core) => (
+    let Some(#arg_ident) = #deno_core::_ops::#convert(&#arg_ident) else {
+      #exception
+    };
+    let #arg_ident = #arg_ident as _;
+  )))
+}
+
 pub fn from_arg_buffer(
   generator_state: &mut GeneratorState,
   arg_ident: &Ident,


Test patch:
diff --git a/ops/op2/test_cases/async/async_arg_return.out b/ops/op2/test_cases/async/async_arg_return.out
index d835e00..b091ba1 100644
--- a/ops/op2/test_cases/async/async_arg_return.out
+++ b/ops/op2/test_cases/async/async_arg_return.out
@@ -36,10 +36,23 @@ impl op_async {
         };
         let result = {
             let arg0 = args.get(1usize as i32);
-            let arg0 = deno_core::_ops::to_i32(&arg0) as _;
+            let Some(arg0) = deno_core::_ops::to_i32_option(&arg0) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected i32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg0 = arg0 as _;
             Self::call(arg0)
         };
-        let promise_id = deno_core::_ops::to_i32(&args.get(0));
+        let promise_id = deno_core::_ops::to_i32_option(&args.get(0))
+            .unwrap_or_default();
         if let Some(result)
             = deno_core::_ops::map_async_op_infallible(
                 opctx,
diff --git a/ops/op2/test_cases/async/async_arg_return_result.out b/ops/op2/test_cases/async/async_arg_return_result.out
index 66313c1..97ef5c8 100644
--- a/ops/op2/test_cases/async/async_arg_return_result.out
+++ b/ops/op2/test_cases/async/async_arg_return_result.out
@@ -36,10 +36,23 @@ impl op_async {
         };
         let result = {
             let arg0 = args.get(1usize as i32);
-            let arg0 = deno_core::_ops::to_i32(&arg0) as _;
+            let Some(arg0) = deno_core::_ops::to_i32_option(&arg0) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected i32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg0 = arg0 as _;
             Self::call(arg0)
         };
-        let promise_id = deno_core::_ops::to_i32(&args.get(0));
+        let promise_id = deno_core::_ops::to_i32_option(&args.get(0))
+            .unwrap_or_default();
         if let Some(result)
             = deno_core::_ops::map_async_op_fallible(
                 opctx,
diff --git a/ops/op2/test_cases/async/async_opstate.out b/ops/op2/test_cases/async/async_opstate.out
index 8f62c10..d4cb2b8 100644
--- a/ops/op2/test_cases/async/async_opstate.out
+++ b/ops/op2/test_cases/async/async_opstate.out
@@ -39,7 +39,8 @@ impl op_async_opstate {
             let arg0 = opstate.clone();
             Self::call(arg0)
         };
-        let promise_id = deno_core::_ops::to_i32(&args.get(0));
+        let promise_id = deno_core::_ops::to_i32_option(&args.get(0))
+            .unwrap_or_default();
         if let Some(result)
             = deno_core::_ops::map_async_op_fallible(
                 opctx,
diff --git a/ops/op2/test_cases/async/async_result.out b/ops/op2/test_cases/async/async_result.out
index 07e4dae..a9ac49a 100644
--- a/ops/op2/test_cases/async/async_result.out
+++ b/ops/op2/test_cases/async/async_result.out
@@ -35,7 +35,8 @@ impl op_async {
                 as *const deno_core::_ops::OpCtx)
         };
         let result = { Self::call() };
-        let promise_id = deno_core::_ops::to_i32(&args.get(0));
+        let promise_id = deno_core::_ops::to_i32_option(&args.get(0))
+            .unwrap_or_default();
         if let Some(result)
             = deno_core::_ops::map_async_op_fallible(
                 opctx,
diff --git a/ops/op2/test_cases/async/async_result_impl.out b/ops/op2/test_cases/async/async_result_impl.out
index fff4c2b..107db01 100644
--- a/ops/op2/test_cases/async/async_result_impl.out
+++ b/ops/op2/test_cases/async/async_result_impl.out
@@ -36,7 +36,19 @@ impl op_async_result_impl {
         };
         let result = {
             let arg0 = args.get(1usize as i32);
-            let arg0 = deno_core::_ops::to_i32(&arg0) as _;
+            let Some(arg0) = deno_core::_ops::to_i32_option(&arg0) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected i32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg0 = arg0 as _;
             Self::call(arg0)
         };
         let result = match result {
@@ -53,7 +65,8 @@ impl op_async_result_impl {
                 return;
             }
         };
-        let promise_id = deno_core::_ops::to_i32(&args.get(0));
+        let promise_id = deno_core::_ops::to_i32_option(&args.get(0))
+            .unwrap_or_default();
         if let Some(result)
             = deno_core::_ops::map_async_op_fallible(
                 opctx,
diff --git a/ops/op2/test_cases/async/async_void.out b/ops/op2/test_cases/async/async_void.out
index 86c7b37..670042e 100644
--- a/ops/op2/test_cases/async/async_void.out
+++ b/ops/op2/test_cases/async/async_void.out
@@ -35,7 +35,8 @@ impl op_async {
                 as *const deno_core::_ops::OpCtx)
         };
         let result = { Self::call() };
-        let promise_id = deno_core::_ops::to_i32(&args.get(0));
+        let promise_id = deno_core::_ops::to_i32_option(&args.get(0))
+            .unwrap_or_default();
         if let Some(result)
             = deno_core::_ops::map_async_op_infallible(
                 opctx,
diff --git a/ops/op2/test_cases/sync/add.out b/ops/op2/test_cases/sync/add.out
index ed92a38..48ef215 100644
--- a/ops/op2/test_cases/sync/add.out
+++ b/ops/op2/test_cases/sync/add.out
@@ -51,9 +51,33 @@ impl op_add {
         });
         let result = {
             let arg0 = args.get(0usize as i32);
-            let arg0 = deno_core::_ops::to_u32(&arg0) as _;
+            let Some(arg0) = deno_core::_ops::to_u32_option(&arg0) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected u32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg0 = arg0 as _;
             let arg1 = args.get(1usize as i32);
-            let arg1 = deno_core::_ops::to_u32(&arg1) as _;
+            let Some(arg1) = deno_core::_ops::to_u32_option(&arg1) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected u32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg1 = arg1 as _;
             Self::call(arg0, arg1)
         };
         rv.set_uint32(result as u32);
diff --git a/ops/op2/test_cases/sync/add_options.out b/ops/op2/test_cases/sync/add_options.out
index d2cf0b3..56134d4 100644
--- a/ops/op2/test_cases/sync/add_options.out
+++ b/ops/op2/test_cases/sync/add_options.out
@@ -31,12 +31,36 @@ impl op_test_add_option {
         });
         let result = {
             let arg0 = args.get(0usize as i32);
-            let arg0 = deno_core::_ops::to_u32(&arg0) as _;
+            let Some(arg0) = deno_core::_ops::to_u32_option(&arg0) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected u32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg0 = arg0 as _;
             let arg1 = args.get(1usize as i32);
             let arg1 = if arg1.is_null_or_undefined() {
                 None
             } else {
-                let arg1 = deno_core::_ops::to_u32(&arg1) as _;
+                let Some(arg1) = deno_core::_ops::to_u32_option(&arg1) else {
+                let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+                let msg = deno_core::v8::String::new_from_one_byte(
+                        &mut scope,
+                        "expected u32".as_bytes(),
+                        deno_core::v8::NewStringType::Normal,
+                    )
+                    .unwrap();
+                let exc = deno_core::v8::Exception::error(&mut scope, msg);
+                scope.throw_exception(exc);
+                return;
+            };
+                let arg1 = arg1 as _;
                 Some(arg1)
             };
             Self::call(arg0, arg1)
diff --git a/ops/op2/test_cases/sync/nofast.out b/ops/op2/test_cases/sync/nofast.out
index 5d72643..9ff984b 100644
--- a/ops/op2/test_cases/sync/nofast.out
+++ b/ops/op2/test_cases/sync/nofast.out
@@ -31,9 +31,33 @@ impl op_nofast {
         });
         let result = {
             let arg0 = args.get(0usize as i32);
-            let arg0 = deno_core::_ops::to_u32(&arg0) as _;
+            let Some(arg0) = deno_core::_ops::to_u32_option(&arg0) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected u32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg0 = arg0 as _;
             let arg1 = args.get(1usize as i32);
-            let arg1 = deno_core::_ops::to_u32(&arg1) as _;
+            let Some(arg1) = deno_core::_ops::to_u32_option(&arg1) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected u32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg1 = arg1 as _;
             Self::call(arg0, arg1)
         };
         rv.set_uint32(result as u32);
diff --git a/ops/op2/test_cases/sync/smi.out b/ops/op2/test_cases/sync/smi.out
index c279710..387cc65 100644
--- a/ops/op2/test_cases/sync/smi.out
+++ b/ops/op2/test_cases/sync/smi.out
@@ -51,9 +51,33 @@ impl op_add {
         });
         let result = {
             let arg0 = args.get(0usize as i32);
-            let arg0 = deno_core::_ops::to_i32(&arg0) as _;
+            let Some(arg0) = deno_core::_ops::to_i32_option(&arg0) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected i32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg0 = arg0 as _;
             let arg1 = args.get(1usize as i32);
-            let arg1 = deno_core::_ops::to_u32(&arg1) as _;
+            let Some(arg1) = deno_core::_ops::to_u32_option(&arg1) else {
+            let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    &mut scope,
+                    "expected u32".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(&mut scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            let arg1 = arg1 as _;
             Self::call(arg0, arg1)
         };
         rv.set_uint32(result as u32);