refactor(runtime/permissions): use concrete error types (#26464)

denoland · Nov 5, 2024 · 1bdecc8 · 1bdecc8
1 parent 3bded14
commit 1bdecc8
Show file tree

Hide file tree

Showing 35 changed files with 999 additions and 830 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cli/args/flags.rs b/cli/args/flags.rs
@@ -3388,8 +3388,7 @@ fn permission_args(app: Command, requires: Option<&'static str>) -> Command {
           .value_name("IP_OR_HOSTNAME")
           .help("Allow network access. Optionally specify allowed IP addresses and host names, with ports as necessary")
           .value_parser(flags_net::validator)
-          .hide(true)
-          ;
+          .hide(true);
         if let Some(requires) = requires {
           arg = arg.requires(requires)
         }

diff --git a/cli/args/flags_net.rs b/cli/args/flags_net.rs
@@ -51,7 +51,7 @@ pub fn parse(paths: Vec<String>) -> clap::error::Result<Vec<String>> {
       }
     } else {
       NetDescriptor::parse(&host_and_port).map_err(|e| {
-        clap::Error::raw(clap::error::ErrorKind::InvalidValue, format!("{e:?}"))
+        clap::Error::raw(clap::error::ErrorKind::InvalidValue, e.to_string())
       })?;
       out.push(host_and_port)
     }

diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs
@@ -89,7 +89,7 @@ impl CliNpmResolver for CliByonmNpmResolver {
       .components()
       .any(|c| c.as_os_str().to_ascii_lowercase() == "node_modules")
     {
-      permissions.check_read_path(path)
+      permissions.check_read_path(path).map_err(Into::into)
     } else {
       Ok(Cow::Borrowed(path))
     }

diff --git a/cli/npm/managed/resolvers/common.rs b/cli/npm/managed/resolvers/common.rs
@@ -133,7 +133,7 @@ impl RegistryReadPermissionChecker {
       }
     }
 
-    permissions.check_read_path(path)
+    permissions.check_read_path(path).map_err(Into::into)
   }
 }
 

diff --git a/cli/tools/info.rs b/cli/tools/info.rs
@@ -645,10 +645,12 @@ impl<'a> GraphDisplayContext<'a> {
         let message = match err {
           HttpsChecksumIntegrity(_) => "(checksum integrity error)",
           Decode(_) => "(loading decode error)",
-          Loader(err) => match deno_core::error::get_custom_error_class(err) {
-            Some("NotCapable") => "(not capable, requires --allow-import)",
-            _ => "(loading error)",
-          },
+          Loader(err) => {
+            match deno_runtime::errors::get_error_class_name(err) {
+              Some("NotCapable") => "(not capable, requires --allow-import)",
+              _ => "(loading error)",
+            }
+          }
           Jsr(_) => "(loading error)",
           NodeUnknownBuiltinModule(_) => "(unknown node built-in error)",
           Npm(_) => "(npm loading error)",

diff --git a/ext/fetch/lib.rs b/ext/fetch/lib.rs
@@ -39,6 +39,7 @@ use deno_core::OpState;
 use deno_core::RcRef;
 use deno_core::Resource;
 use deno_core::ResourceId;
+use deno_permissions::PermissionCheckError;
 use deno_tls::rustls::RootCertStore;
 use deno_tls::Proxy;
 use deno_tls::RootCertStoreProvider;
@@ -149,7 +150,7 @@ pub enum FetchError {
   #[error(transparent)]
   Resource(deno_core::error::AnyError),
   #[error(transparent)]
-  Permission(deno_core::error::AnyError),
+  Permission(#[from] PermissionCheckError),
   #[error("NetworkError when attempting to fetch resource")]
   NetworkError,
   #[error("Fetching files only supports the GET method: received {0}")]
@@ -346,13 +347,13 @@ pub trait FetchPermissions {
     &mut self,
     url: &Url,
     api_name: &str,
-  ) -> Result<(), deno_core::error::AnyError>;
+  ) -> Result<(), PermissionCheckError>;
   #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"]
   fn check_read<'a>(
     &mut self,
     p: &'a Path,
     api_name: &str,
-  ) -> Result<Cow<'a, Path>, deno_core::error::AnyError>;
+  ) -> Result<Cow<'a, Path>, PermissionCheckError>;
 }
 
 impl FetchPermissions for deno_permissions::PermissionsContainer {
@@ -361,7 +362,7 @@ impl FetchPermissions for deno_permissions::PermissionsContainer {
     &mut self,
     url: &Url,
     api_name: &str,
-  ) -> Result<(), deno_core::error::AnyError> {
+  ) -> Result<(), PermissionCheckError> {
     deno_permissions::PermissionsContainer::check_net_url(self, url, api_name)
   }
 
@@ -370,7 +371,7 @@ impl FetchPermissions for deno_permissions::PermissionsContainer {
     &mut self,
     path: &'a Path,
     api_name: &str,
-  ) -> Result<Cow<'a, Path>, deno_core::error::AnyError> {
+  ) -> Result<Cow<'a, Path>, PermissionCheckError> {
     deno_permissions::PermissionsContainer::check_read_path(
       self,
       path,
@@ -414,9 +415,7 @@ where
     "file" => {
       let path = url.to_file_path().map_err(|_| FetchError::NetworkError)?;
       let permissions = state.borrow_mut::<FP>();
-      let path = permissions
-        .check_read(&path, "fetch()")
-        .map_err(FetchError::Permission)?;
+      let path = permissions.check_read(&path, "fetch()")?;
       let url = match path {
         Cow::Owned(path) => Url::from_file_path(path).unwrap(),
         Cow::Borrowed(_) => url,
@@ -442,9 +441,7 @@ where
     }
     "http" | "https" => {
       let permissions = state.borrow_mut::<FP>();
-      permissions
-        .check_net_url(&url, "fetch()")
-        .map_err(FetchError::Resource)?;
+      permissions.check_net_url(&url, "fetch()")?;
 
       let maybe_authority = extract_authority(&mut url);
       let uri = url
@@ -863,9 +860,7 @@ where
   if let Some(proxy) = args.proxy.clone() {
     let permissions = state.borrow_mut::<FP>();
     let url = Url::parse(&proxy.url)?;
-    permissions
-      .check_net_url(&url, "Deno.createHttpClient()")
-      .map_err(FetchError::Permission)?;
+    permissions.check_net_url(&url, "Deno.createHttpClient()")?;
   }
 
   let options = state.borrow::<Options>();

diff --git a/ext/ffi/call.rs b/ext/ffi/call.rs
@@ -32,7 +32,9 @@ pub enum CallError {
   #[error("Invalid FFI symbol name: '{0}'")]
   InvalidSymbol(String),
   #[error(transparent)]
-  Permission(deno_core::error::AnyError),
+  Permission(#[from] deno_permissions::PermissionCheckError),
+  #[error(transparent)]
+  Resource(deno_core::error::AnyError),
   #[error(transparent)]
   Callback(#[from] super::CallbackError),
 }
@@ -301,9 +303,7 @@ where
   {
     let mut state = state.borrow_mut();
     let permissions = state.borrow_mut::<FP>();
-    permissions
-      .check_partial_no_path()
-      .map_err(CallError::Permission)?;
+    permissions.check_partial_no_path()?;
   };
 
   let symbol = PtrSymbol::new(pointer, &def)?;
@@ -347,7 +347,7 @@ pub fn op_ffi_call_nonblocking(
     let resource = state
       .resource_table
       .get::<DynamicLibraryResource>(rid)
-      .map_err(CallError::Permission)?;
+      .map_err(CallError::Resource)?;
     let symbols = &resource.symbols;
     *symbols
       .get(&symbol)
@@ -401,9 +401,7 @@ where
   {
     let mut state = state.borrow_mut();
     let permissions = state.borrow_mut::<FP>();
-    permissions
-      .check_partial_no_path()
-      .map_err(CallError::Permission)?;
+    permissions.check_partial_no_path()?;
   };
 
   let symbol = PtrSymbol::new(pointer, &def)?;

diff --git a/ext/ffi/callback.rs b/ext/ffi/callback.rs
@@ -38,7 +38,7 @@ pub enum CallbackError {
   #[error(transparent)]
   Resource(deno_core::error::AnyError),
   #[error(transparent)]
-  Permission(deno_core::error::AnyError),
+  Permission(#[from] deno_permissions::PermissionCheckError),
   #[error(transparent)]
   Other(deno_core::error::AnyError),
 }
@@ -572,9 +572,7 @@ where
   FP: FfiPermissions + 'static,
 {
   let permissions = state.borrow_mut::<FP>();
-  permissions
-    .check_partial_no_path()
-    .map_err(CallbackError::Permission)?;
+  permissions.check_partial_no_path()?;
 
   let thread_id: u32 = LOCAL_THREAD_ID.with(|s| {
     let value = *s.borrow();

diff --git a/ext/ffi/dlfcn.rs b/ext/ffi/dlfcn.rs
@@ -30,7 +30,7 @@ pub enum DlfcnError {
   #[error(transparent)]
   Dlopen(#[from] dlopen2::Error),
   #[error(transparent)]
-  Permission(deno_core::error::AnyError),
+  Permission(#[from] deno_permissions::PermissionCheckError),
   #[error(transparent)]
   Other(deno_core::error::AnyError),
 }
@@ -133,9 +133,7 @@ where
   FP: FfiPermissions + 'static,
 {
   let permissions = state.borrow_mut::<FP>();
-  let path = permissions
-    .check_partial_with_path(&args.path)
-    .map_err(DlfcnError::Permission)?;
+  let path = permissions.check_partial_with_path(&args.path)?;
 
   let lib = Library::open(&path).map_err(|e| {
     dlopen2::Error::OpeningLibraryError(std::io::Error::new(

diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs
@@ -1,7 +1,5 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-use deno_core::error::AnyError;
-
 use std::mem::size_of;
 use std::os::raw::c_char;
 use std::os::raw::c_short;
@@ -31,6 +29,7 @@ use symbol::Symbol;
 
 pub use call::CallError;
 pub use callback::CallbackError;
+use deno_permissions::PermissionCheckError;
 pub use dlfcn::DlfcnError;
 pub use ir::IRError;
 pub use r#static::StaticError;
@@ -48,25 +47,25 @@ const _: () = {
 pub const UNSTABLE_FEATURE_NAME: &str = "ffi";
 
 pub trait FfiPermissions {
-  fn check_partial_no_path(&mut self) -> Result<(), AnyError>;
+  fn check_partial_no_path(&mut self) -> Result<(), PermissionCheckError>;
   #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"]
   fn check_partial_with_path(
     &mut self,
     path: &str,
-  ) -> Result<PathBuf, AnyError>;
+  ) -> Result<PathBuf, PermissionCheckError>;
 }
 
 impl FfiPermissions for deno_permissions::PermissionsContainer {
   #[inline(always)]
-  fn check_partial_no_path(&mut self) -> Result<(), AnyError> {
+  fn check_partial_no_path(&mut self) -> Result<(), PermissionCheckError> {
     deno_permissions::PermissionsContainer::check_ffi_partial_no_path(self)
   }
 
   #[inline(always)]
   fn check_partial_with_path(
     &mut self,
     path: &str,
-  ) -> Result<PathBuf, AnyError> {
+  ) -> Result<PathBuf, PermissionCheckError> {
     deno_permissions::PermissionsContainer::check_ffi_partial_with_path(
       self, path,
     )
-Original file line number
+Diff line change
@@ Expand Up / @@ -133,7 +133,7 @@ impl RegistryReadPermissionChecker { @@
           }
         }
-        permissions.check_read_path(path)
+        permissions.check_read_path(path).map_err(Into::into)
       }
     }
@@ Expand Down @@