refactor(napi): simplify types (#19179)

This commit removes "Error" and "Result" structs from
"ext/napi". In turn all NAPI functions now return "napi_status"
instead of "napi::Result".

cli/napi/async.rs

@@ -18,7 +18,7 @@ fn napi_create_async_work(
   complete: napi_async_complete_callback,
   data: *mut c_void,
   result: *mut napi_async_work,
-) -> Result {
+) -> napi_status {
   let mut work = AsyncWork {
     data,
     execute,
@@ -26,30 +26,38 @@ fn napi_create_async_work(
   };
   let work_box = Box::new(work);
   *result = transmute::<*mut AsyncWork, _>(Box::into_raw(work_box));
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
 fn napi_cancel_async_work(
   _env: &mut Env,
   _async_work: napi_async_work,
-) -> Result {
+) -> napi_status {
-  Ok(())
+  napi_ok
 }
 
 /// Frees a previously allocated work object.
 #[napi_sym::napi_sym]
-fn napi_delete_async_work(_env: &mut Env, work: napi_async_work) -> Result {
+fn napi_delete_async_work(
+  _env: &mut Env,
+  work: napi_async_work,
+) -> napi_status {
   let work = Box::from_raw(work as *mut AsyncWork);
   drop(work);
 
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
-fn napi_queue_async_work(env_ptr: *mut Env, work: napi_async_work) -> Result {
+fn napi_queue_async_work(
+  env_ptr: *mut Env,
+  work: napi_async_work,
+) -> napi_status {
   let work: &AsyncWork = &*(work as *const AsyncWork);
-  let env: &mut Env = env_ptr.as_mut().ok_or(Error::InvalidArg)?;
+  let Some(env) = env_ptr.as_mut() else {
+    return napi_invalid_arg;
+  };
 
   let fut = Box::new(move || {
     (work.execute)(env_ptr as napi_env, work.data);
@@ -58,7 +66,7 @@ fn napi_queue_async_work(env_ptr: *mut Env, work: napi_async_work) -> Result {
   });
   env.add_async_work(fut);
 
-  Ok(())
+  napi_ok
 }
 
 // TODO: Custom async operations.
@@ -69,11 +77,11 @@ fn napi_async_init(
   _async_resource: napi_value,
   _async_resource_name: napi_value,
   _result: *mut *mut (),
-) -> Result {
+) -> napi_status {
   todo!()
 }
 
 #[napi_sym::napi_sym]
-fn napi_async_destroy(_env: *mut Env, _async_context: *mut ()) -> Result {
+fn napi_async_destroy(_env: *mut Env, _async_context: *mut ()) -> napi_status {
   todo!()
 }

cli/napi/env.rs

@@ -41,8 +41,10 @@ pub unsafe extern "C" fn napi_fatal_error(
 // napi-3
 
 #[napi_sym::napi_sym]
-fn napi_fatal_exception(env: *mut Env, value: napi_value) -> Result {
+fn napi_fatal_exception(env: *mut Env, value: napi_value) -> napi_status {
-  let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  let Some(env) = env.as_mut() else {
+    return napi_invalid_arg;
+  };
   let value = transmute::<napi_value, v8::Local<v8::Value>>(value);
   let error = value.to_rust_string_lossy(&mut env.scope());
   panic!("Fatal exception triggered by napi_fatal_exception!\n{error}");
@@ -53,8 +55,10 @@ fn napi_add_env_cleanup_hook(
   env: *mut Env,
   hook: extern "C" fn(*const c_void),
   data: *const c_void,
-) -> Result {
+) -> napi_status {
-  let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  let Some(env) = env.as_mut() else {
+    return napi_invalid_arg;
+  };
 
   {
     let mut env_cleanup_hooks = env.cleanup_hooks.borrow_mut();
@@ -66,7 +70,7 @@ fn napi_add_env_cleanup_hook(
     }
     env_cleanup_hooks.push((hook, data));
   }
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
@@ -74,8 +78,10 @@ fn napi_remove_env_cleanup_hook(
   env: *mut Env,
   hook: extern "C" fn(*const c_void),
   data: *const c_void,
-) -> Result {
+) -> napi_status {
-  let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  let Some(env) = env.as_mut() else {
+    return napi_invalid_arg;
+  };
 
   {
     let mut env_cleanup_hooks = env.cleanup_hooks.borrow_mut();
@@ -91,7 +97,7 @@ fn napi_remove_env_cleanup_hook(
     }
   }
 
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
@@ -100,46 +106,51 @@ fn napi_open_callback_scope(
   _resource_object: napi_value,
   _context: napi_value,
   _result: *mut napi_callback_scope,
-) -> Result {
+) -> napi_status {
   // we open scope automatically when it's needed
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
 fn napi_close_callback_scope(
   _env: *mut Env,
   _scope: napi_callback_scope,
-) -> Result {
+) -> napi_status {
   // we close scope automatically when it's needed
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
 fn node_api_get_module_file_name(
   env: *mut Env,
   result: *mut *const c_char,
-) -> Result {
+) -> napi_status {
-  let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  let Some(env) = env.as_mut() else {
+    return napi_invalid_arg;
+  };
 
   let shared = env.shared();
   *result = shared.filename;
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
-fn napi_module_register(module: *const NapiModule) -> Result {
+fn napi_module_register(module: *const NapiModule) -> napi_status {
   MODULE.with(|cell| {
     let mut slot = cell.borrow_mut();
     slot.replace(module);
   });
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
-fn napi_get_uv_event_loop(_env: *mut Env, uv_loop: *mut *mut ()) -> Result {
+fn napi_get_uv_event_loop(
+  _env: *mut Env,
+  uv_loop: *mut *mut (),
+) -> napi_status {
   // There is no uv_loop in Deno
   *uv_loop = std::ptr::null_mut();
-  Ok(())
+  napi_ok
 }
 
 const NODE_VERSION: napi_node_version = napi_node_version {
@@ -153,10 +164,10 @@ const NODE_VERSION: napi_node_version = napi_node_version {
 fn napi_get_node_version(
   env: *mut Env,
   result: *mut *const napi_node_version,
-) -> Result {
+) -> napi_status {
   crate::check_env!(env);
   crate::check_arg!(env, result);
 
   *result = &NODE_VERSION as *const napi_node_version;
-  Ok(())
+  napi_ok
 }

cli/napi/sym/lib.rs

@@ -25,22 +25,12 @@ pub fn napi_sym(_attr: TokenStream, item: TokenStream) -> TokenStream {
 
   let block = &func.block;
   let inputs = &func.sig.inputs;
-  let output = &func.sig.output;
   let generics = &func.sig.generics;
-  let ret_ty = match output {
-    syn::ReturnType::Default => panic!("expected a return type"),
-    syn::ReturnType::Type(_, ty) => quote! { #ty },
-  };
   TokenStream::from(quote! {
       // SAFETY: it's an NAPI function.
       #[no_mangle]
       pub unsafe extern "C" fn #name #generics (#inputs) -> napi_status {
-        let mut inner = || -> #ret_ty {
+        #block
-          #block
-        };
-        inner()
-          .map(|_| napi_ok)
-          .unwrap_or_else(|e| e.into())
       }
   })
 }

cli/napi/threadsafe_functions.rs

@@ -37,33 +37,36 @@ impl Drop for TsFn {
 }
 
 impl TsFn {
-  pub fn acquire(&mut self) -> Result {
+  pub fn acquire(&mut self) -> napi_status {
     self.thread_counter += 1;
-    Ok(())
+    napi_ok
   }
 
-  pub fn release(mut self) -> Result {
+  pub fn release(mut self) -> napi_status {
     self.thread_counter -= 1;
     if self.thread_counter == 0 {
-      self
+      if self
         .tsfn_sender
         .unbounded_send(ThreadSafeFunctionStatus::Dead)
-        .map_err(|_| Error::GenericFailure)?;
+        .is_err()
+      {
+        return napi_generic_failure;
+      }
       drop(self);
     } else {
       forget(self);
     }
-    Ok(())
+    napi_ok
   }
 
-  pub fn ref_(&mut self) -> Result {
+  pub fn ref_(&mut self) -> napi_status {
     self
       .ref_counter
       .fetch_add(1, std::sync::atomic::Ordering::SeqCst);
-    Ok(())
+    napi_ok
   }
 
-  pub fn unref(&mut self) -> Result {
+  pub fn unref(&mut self) -> napi_status {
     let _ = self.ref_counter.fetch_update(
       std::sync::atomic::Ordering::SeqCst,
       std::sync::atomic::Ordering::SeqCst,
@@ -76,7 +79,7 @@ impl TsFn {
       },
     );
 
-    Ok(())
+    napi_ok
   }
 
   pub fn call(&self, data: *mut c_void, is_blocking: bool) {
@@ -138,18 +141,22 @@ fn napi_create_threadsafe_function(
   context: *mut c_void,
   maybe_call_js_cb: Option<napi_threadsafe_function_call_js>,
   result: *mut napi_threadsafe_function,
-) -> Result {
+) -> napi_status {
-  let env_ref = env.as_mut().ok_or(Error::GenericFailure)?;
+  let Some(env_ref) = env.as_mut() else {
+    return napi_generic_failure;
+  };
   if initial_thread_count == 0 {
-    return Err(Error::InvalidArg);
+    return napi_invalid_arg;
+  }
+
+  let mut maybe_func = None;
+
+  if let Some(value) = *func {
+    let Ok(func) = v8::Local::<v8::Function>::try_from(value) else {
+      return napi_function_expected;
+    };
+    maybe_func = Some(v8::Global::new(&mut env_ref.scope(), func));
   }
-  let maybe_func = func
-    .map(|value| {
-      let func = v8::Local::<v8::Function>::try_from(value)
-        .map_err(|_| Error::FunctionExpected)?;
-      Ok(v8::Global::new(&mut env_ref.scope(), func))
-    })
-    .transpose()?;
 
   let id = TS_FN_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
 
@@ -170,35 +177,34 @@ fn napi_create_threadsafe_function(
   env_ref
     .add_threadsafe_function_ref_counter(tsfn.id, tsfn.ref_counter.clone());
 
-  env_ref
+  if env_ref
     .threadsafe_function_sender
     .unbounded_send(ThreadSafeFunctionStatus::Alive)
-    .map_err(|_| Error::GenericFailure)?;
+    .is_err()
+  {
+    return napi_generic_failure;
+  }
   *result = transmute::<Box<TsFn>, _>(Box::new(tsfn));
 
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
 fn napi_acquire_threadsafe_function(
   tsfn: napi_threadsafe_function,
   _mode: napi_threadsafe_function_release_mode,
-) -> Result {
+) -> napi_status {
   let tsfn: &mut TsFn = &mut *(tsfn as *mut TsFn);
-  tsfn.acquire()?;
+  tsfn.acquire()
-
-  Ok(())
 }
 
 #[napi_sym::napi_sym]
 fn napi_unref_threadsafe_function(
   _env: &mut Env,
   tsfn: napi_threadsafe_function,
-) -> Result {
+) -> napi_status {
   let tsfn: &mut TsFn = &mut *(tsfn as *mut TsFn);
-  tsfn.unref()?;
+  tsfn.unref()
-
-  Ok(())
 }
 
 /// Maybe called from any thread.
@@ -206,10 +212,10 @@ fn napi_unref_threadsafe_function(
 pub fn napi_get_threadsafe_function_context(
   func: napi_threadsafe_function,
   result: *mut *const c_void,
-) -> Result {
+) -> napi_status {
   let tsfn: &TsFn = &*(func as *const TsFn);
   *result = tsfn.context;
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
@@ -217,29 +223,26 @@ fn napi_call_threadsafe_function(
   func: napi_threadsafe_function,
   data: *mut c_void,
   is_blocking: napi_threadsafe_function_call_mode,
-) -> Result {
+) -> napi_status {
   let tsfn: &TsFn = &*(func as *const TsFn);
   tsfn.call(data, is_blocking != 0);
-  Ok(())
+  napi_ok
 }
 
 #[napi_sym::napi_sym]
 fn napi_ref_threadsafe_function(
   _env: &mut Env,
   func: napi_threadsafe_function,
-) -> Result {
+) -> napi_status {
   let tsfn: &mut TsFn = &mut *(func as *mut TsFn);
-  tsfn.ref_()?;
+  tsfn.ref_()
-  Ok(())
 }
 
 #[napi_sym::napi_sym]
 fn napi_release_threadsafe_function(
   tsfn: napi_threadsafe_function,
   _mode: napi_threadsafe_function_release_mode,
-) -> Result {
+) -> napi_status {
   let tsfn: Box<TsFn> = Box::from_raw(tsfn as *mut TsFn);
-  tsfn.release()?;
+  tsfn.release()
-
-  Ok(())
 }

ext/napi/lib.rs

@@ -101,95 +101,6 @@ pub struct NapiModule {
   reserved: [*mut c_void; 4],
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub enum Error {
-  InvalidArg,
-  ObjectExpected,
-  StringExpected,
-  NameExpected,
-  FunctionExpected,
-  NumberExpected,
-  BooleanExpected,
-  ArrayExpected,
-  GenericFailure,
-  PendingException,
-  Cancelled,
-  EscapeCalledTwice,
-  HandleScopeMismatch,
-  CallbackScopeMismatch,
-  QueueFull,
-  Closing,
-  BigIntExpected,
-  DateExpected,
-  ArrayBufferExpected,
-  DetachableArraybufferExpected,
-  WouldDeadlock,
-}
-
-#[allow(clippy::from_over_into)]
-impl Into<Error> for napi_status {
-  fn into(self) -> Error {
-    match self {
-      napi_invalid_arg => Error::InvalidArg,
-      napi_object_expected => Error::ObjectExpected,
-      napi_string_expected => Error::StringExpected,
-      napi_name_expected => Error::NameExpected,
-      napi_function_expected => Error::FunctionExpected,
-      napi_number_expected => Error::NumberExpected,
-      napi_boolean_expected => Error::BooleanExpected,
-      napi_array_expected => Error::ArrayExpected,
-      napi_generic_failure => Error::GenericFailure,
-      napi_pending_exception => Error::PendingException,
-      napi_cancelled => Error::Cancelled,
-      napi_escape_called_twice => Error::EscapeCalledTwice,
-      napi_handle_scope_mismatch => Error::HandleScopeMismatch,
-      napi_callback_scope_mismatch => Error::CallbackScopeMismatch,
-      napi_queue_full => Error::QueueFull,
-      napi_closing => Error::Closing,
-      napi_bigint_expected => Error::BigIntExpected,
-      napi_date_expected => Error::DateExpected,
-      napi_arraybuffer_expected => Error::ArrayBufferExpected,
-      napi_detachable_arraybuffer_expected => {
-        Error::DetachableArraybufferExpected
-      }
-      napi_would_deadlock => Error::WouldDeadlock,
-      _ => unreachable!(),
-    }
-  }
-}
-
-pub type Result = std::result::Result<(), Error>;
-
-impl From<Error> for napi_status {
-  fn from(error: Error) -> Self {
-    match error {
-      Error::InvalidArg => napi_invalid_arg,
-      Error::ObjectExpected => napi_object_expected,
-      Error::StringExpected => napi_string_expected,
-      Error::NameExpected => napi_name_expected,
-      Error::FunctionExpected => napi_function_expected,
-      Error::NumberExpected => napi_number_expected,
-      Error::BooleanExpected => napi_boolean_expected,
-      Error::ArrayExpected => napi_array_expected,
-      Error::GenericFailure => napi_generic_failure,
-      Error::PendingException => napi_pending_exception,
-      Error::Cancelled => napi_cancelled,
-      Error::EscapeCalledTwice => napi_escape_called_twice,
-      Error::HandleScopeMismatch => napi_handle_scope_mismatch,
-      Error::CallbackScopeMismatch => napi_callback_scope_mismatch,
-      Error::QueueFull => napi_queue_full,
-      Error::Closing => napi_closing,
-      Error::BigIntExpected => napi_bigint_expected,
-      Error::DateExpected => napi_date_expected,
-      Error::ArrayBufferExpected => napi_arraybuffer_expected,
-      Error::DetachableArraybufferExpected => {
-        napi_detachable_arraybuffer_expected
-      }
-      Error::WouldDeadlock => napi_would_deadlock,
-    }
-  }
-}
-
 pub type napi_valuetype = i32;
 
 pub const napi_undefined: napi_valuetype = 0;