refactor(ext/fetch): use concrete error types (#26220)

3 files changed, 156 insertions, 94 deletions

diff --git a/ext/fetch/Cargo.toml b/ext/fetch/Cargo.toml
index ddb58a3f3..afffd3ffb 100644
--- a/ext/fetch/Cargo.toml
+++ b/ext/fetch/Cargo.toml
@@ -32,6 +32,7 @@ percent-encoding.workspace = true
 rustls-webpki.workspace = true
 serde.workspace = true
 serde_json.workspace = true
+thiserror.workspace = true
 tokio.workspace = true
 tokio-rustls.workspace = true
 tokio-socks.workspace = true
diff --git a/ext/fetch/fs_fetch_handler.rs b/ext/fetch/fs_fetch_handler.rs
index 4c2b81f35..c236dd9c6 100644
--- a/ext/fetch/fs_fetch_handler.rs
+++ b/ext/fetch/fs_fetch_handler.rs
@@ -4,7 +4,6 @@ use crate::CancelHandle;
 use crate::CancelableResponseFuture;
 use crate::FetchHandler;
 
-use deno_core::error::type_error;
 use deno_core::futures::FutureExt;
 use deno_core::futures::TryFutureExt;
 use deno_core::futures::TryStreamExt;
@@ -42,9 +41,7 @@ impl FetchHandler for FsFetchHandler {
         .map_err(|_| ())?;
       Ok::<_, ()>(response)
     }
-    .map_err(move |_| {
-      type_error("NetworkError when attempting to fetch resource")
-    })
+    .map_err(move |_| super::FetchError::NetworkError)
     .or_cancel(&cancel_handle)
     .boxed_local();
 
diff --git a/ext/fetch/lib.rs b/ext/fetch/lib.rs
index 88f303852..4df8dc3d7 100644
--- a/ext/fetch/lib.rs
+++ b/ext/fetch/lib.rs
@@ -17,10 +17,6 @@ use std::sync::Arc;
 use std::task::Context;
 use std::task::Poll;
 
-use deno_core::anyhow::anyhow;
-use deno_core::anyhow::Error;
-use deno_core::error::type_error;
-use deno_core::error::AnyError;
 use deno_core::futures::stream::Peekable;
 use deno_core::futures::Future;
 use deno_core::futures::FutureExt;
@@ -28,6 +24,7 @@ use deno_core::futures::Stream;
 use deno_core::futures::StreamExt;
 use deno_core::futures::TryFutureExt;
 use deno_core::op2;
+use deno_core::url;
 use deno_core::url::Url;
 use deno_core::AsyncRefCell;
 use deno_core::AsyncResult;
@@ -87,15 +84,18 @@ pub struct Options {
   pub root_cert_store_provider: Option<Arc<dyn RootCertStoreProvider>>,
   pub proxy: Option<Proxy>,
   #[allow(clippy::type_complexity)]
-  pub request_builder_hook:
-    Option<fn(&mut http::Request<ReqBody>) -> Result<(), AnyError>>,
+  pub request_builder_hook: Option<
+    fn(&mut http::Request<ReqBody>) -> Result<(), deno_core::error::AnyError>,
+  >,
   pub unsafely_ignore_certificate_errors: Option<Vec<String>>,
   pub client_cert_chain_and_key: TlsKeys,
   pub file_fetch_handler: Rc<dyn FetchHandler>,
 }
 
 impl Options {
-  pub fn root_cert_store(&self) -> Result<Option<RootCertStore>, AnyError> {
+  pub fn root_cert_store(
+    &self,
+  ) -> Result<Option<RootCertStore>, deno_core::error::AnyError> {
     Ok(match &self.root_cert_store_provider {
       Some(provider) => Some(provider.get_or_try_init()?.clone()),
       None => None,
@@ -144,6 +144,51 @@ deno_core::extension!(deno_fetch,
   },
 );
 
+#[derive(Debug, thiserror::Error)]
+pub enum FetchError {
+  #[error(transparent)]
+  Resource(deno_core::error::AnyError),
+  #[error(transparent)]
+  Permission(deno_core::error::AnyError),
+  #[error("NetworkError when attempting to fetch resource")]
+  NetworkError,
+  #[error("Fetching files only supports the GET method: received {0}")]
+  FsNotGet(Method),
+  #[error("Invalid URL {0}")]
+  InvalidUrl(Url),
+  #[error(transparent)]
+  InvalidHeaderName(#[from] http::header::InvalidHeaderName),
+  #[error(transparent)]
+  InvalidHeaderValue(#[from] http::header::InvalidHeaderValue),
+  #[error("{0:?}")]
+  DataUrl(data_url::DataUrlError),
+  #[error("{0:?}")]
+  Base64(data_url::forgiving_base64::InvalidBase64),
+  #[error("Blob for the given URL not found.")]
+  BlobNotFound,
+  #[error("Url scheme '{0}' not supported")]
+  SchemeNotSupported(String),
+  #[error("Request was cancelled")]
+  RequestCanceled,
+  #[error(transparent)]
+  Http(#[from] http::Error),
+  #[error(transparent)]
+  ClientCreate(#[from] HttpClientCreateError),
+  #[error(transparent)]
+  Url(#[from] url::ParseError),
+  #[error(transparent)]
+  Method(#[from] http::method::InvalidMethod),
+  #[error(transparent)]
+  ClientSend(#[from] ClientSendError),
+  #[error(transparent)]
+  RequestBuilderHook(deno_core::error::AnyError),
+  #[error(transparent)]
+  Io(#[from] std::io::Error),
+  // Only used for node upgrade
+  #[error(transparent)]
+  Hyper(#[from] hyper::Error),
+}
+
 pub type CancelableResponseFuture =
   Pin<Box<dyn Future<Output = CancelableResponseResult>>>;
 
@@ -170,11 +215,7 @@ impl FetchHandler for DefaultFileFetchHandler {
     _state: &mut OpState,
     _url: &Url,
   ) -> (CancelableResponseFuture, Option<Rc<CancelHandle>>) {
-    let fut = async move {
-      Ok(Err(type_error(
-        "NetworkError when attempting to fetch resource",
-      )))
-    };
+    let fut = async move { Ok(Err(FetchError::NetworkError)) };
     (Box::pin(fut), None)
   }
 }
@@ -191,7 +232,7 @@ pub struct FetchReturn {
 
 pub fn get_or_create_client_from_state(
   state: &mut OpState,
-) -> Result<Client, AnyError> {
+) -> Result<Client, HttpClientCreateError> {
   if let Some(client) = state.try_borrow::<Client>() {
     Ok(client.clone())
   } else {
@@ -204,11 +245,13 @@ pub fn get_or_create_client_from_state(
 
 pub fn create_client_from_options(
   options: &Options,
-) -> Result<Client, AnyError> {
+) -> Result<Client, HttpClientCreateError> {
   create_http_client(
     &options.user_agent,
     CreateHttpClientOptions {
-      root_cert_store: options.root_cert_store()?,
+      root_cert_store: options
+        .root_cert_store()
+        .map_err(HttpClientCreateError::RootCertStore)?,
       ca_certs: vec![],
       proxy: options.proxy.clone(),
       unsafely_ignore_certificate_errors: options
@@ -230,7 +273,9 @@ pub fn create_client_from_options(
 #[allow(clippy::type_complexity)]
 pub struct ResourceToBodyAdapter(
   Rc<dyn Resource>,
-  Option<Pin<Box<dyn Future<Output = Result<BufView, Error>>>>>,
+  Option<
+    Pin<Box<dyn Future<Output = Result<BufView, deno_core::error::AnyError>>>>,
+  >,
 );
 
 impl ResourceToBodyAdapter {
@@ -246,7 +291,7 @@ unsafe impl Send for ResourceToBodyAdapter {}
 unsafe impl Sync for ResourceToBodyAdapter {}
 
 impl Stream for ResourceToBodyAdapter {
-  type Item = Result<Bytes, Error>;
+  type Item = Result<Bytes, deno_core::error::AnyError>;
 
   fn poll_next(
     self: Pin<&mut Self>,
@@ -276,7 +321,7 @@ impl Stream for ResourceToBodyAdapter {
 
 impl hyper::body::Body for ResourceToBodyAdapter {
   type Data = Bytes;
-  type Error = Error;
+  type Error = deno_core::error::AnyError;
 
   fn poll_frame(
     self: Pin<&mut Self>,
@@ -301,13 +346,13 @@ pub trait FetchPermissions {
     &mut self,
     url: &Url,
     api_name: &str,
-  ) -> Result<(), AnyError>;
+  ) -> Result<(), deno_core::error::AnyError>;
   #[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>, AnyError>;
+  ) -> Result<Cow<'a, Path>, deno_core::error::AnyError>;
 }
 
 impl FetchPermissions for deno_permissions::PermissionsContainer {
@@ -316,7 +361,7 @@ impl FetchPermissions for deno_permissions::PermissionsContainer {
     &mut self,
     url: &Url,
     api_name: &str,
-  ) -> Result<(), AnyError> {
+  ) -> Result<(), deno_core::error::AnyError> {
     deno_permissions::PermissionsContainer::check_net_url(self, url, api_name)
   }
 
@@ -325,7 +370,7 @@ impl FetchPermissions for deno_permissions::PermissionsContainer {
     &mut self,
     path: &'a Path,
     api_name: &str,
-  ) -> Result<Cow<'a, Path>, AnyError> {
+  ) -> Result<Cow<'a, Path>, deno_core::error::AnyError> {
     deno_permissions::PermissionsContainer::check_read_path(
       self,
       path,
@@ -346,12 +391,15 @@ pub fn op_fetch<FP>(
   has_body: bool,
   #[buffer] data: Option<JsBuffer>,
   #[smi] resource: Option<ResourceId>,
-) -> Result<FetchReturn, AnyError>
+) -> Result<FetchReturn, FetchError>
 where
   FP: FetchPermissions + 'static,
 {
   let (client, allow_host) = if let Some(rid) = client_rid {
-    let r = state.resource_table.get::<HttpClientResource>(rid)?;
+    let r = state
+      .resource_table
+      .get::<HttpClientResource>(rid)
+      .map_err(FetchError::Resource)?;
     (r.client.clone(), r.allow_host)
   } else {
     (get_or_create_client_from_state(state)?, false)
@@ -364,20 +412,18 @@ where
   let scheme = url.scheme();
   let (request_rid, cancel_handle_rid) = match scheme {
     "file" => {
-      let path = url.to_file_path().map_err(|_| {
-        type_error("NetworkError when attempting to fetch resource")
-      })?;
+      let path = url.to_file_path().map_err(|_| FetchError::NetworkError)?;
       let permissions = state.borrow_mut::<FP>();
-      let path = permissions.check_read(&path, "fetch()")?;
+      let path = permissions
+        .check_read(&path, "fetch()")
+        .map_err(FetchError::Permission)?;
       let url = match path {
         Cow::Owned(path) => Url::from_file_path(path).unwrap(),
         Cow::Borrowed(_) => url,
       };
 
       if method != Method::GET {
-        return Err(type_error(format!(
-          "Fetching files only supports the GET method: received {method}"
-        )));
+        return Err(FetchError::FsNotGet(method));
       }
 
       let Options {
@@ -396,13 +442,15 @@ where
     }
     "http" | "https" => {
       let permissions = state.borrow_mut::<FP>();
-      permissions.check_net_url(&url, "fetch()")?;
+      permissions
+        .check_net_url(&url, "fetch()")
+        .map_err(FetchError::Resource)?;
 
       let maybe_authority = extract_authority(&mut url);
       let uri = url
         .as_str()
         .parse::<Uri>()
-        .map_err(|_| type_error(format!("Invalid URL {url}")))?;
+        .map_err(|_| FetchError::InvalidUrl(url.clone()))?;
 
       let mut con_len = None;
       let body = if has_body {
@@ -416,7 +464,10 @@ where
               .boxed()
           }
           (_, Some(resource)) => {
-            let resource = state.resource_table.take_any(resource)?;
+            let resource = state
+              .resource_table
+              .take_any(resource)
+              .map_err(FetchError::Resource)?;
             match resource.size_hint() {
               (body_size, Some(n)) if body_size == n && body_size > 0 => {
                 con_len = Some(body_size);
@@ -453,10 +504,8 @@ where
       }
 
       for (key, value) in headers {
-        let name = HeaderName::from_bytes(&key)
-          .map_err(|err| type_error(err.to_string()))?;
-        let v = HeaderValue::from_bytes(&value)
-          .map_err(|err| type_error(err.to_string()))?;
+        let name = HeaderName::from_bytes(&key)?;
+        let v = HeaderValue::from_bytes(&value)?;
 
         if (name != HOST || allow_host) && name != CONTENT_LENGTH {
           request.headers_mut().append(name, v);
@@ -474,20 +523,18 @@ where
       let options = state.borrow::<Options>();
       if let Some(request_builder_hook) = options.request_builder_hook {
         request_builder_hook(&mut request)
-          .map_err(|err| type_error(err.to_string()))?;
+          .map_err(FetchError::RequestBuilderHook)?;
       }
 
       let cancel_handle = CancelHandle::new_rc();
       let cancel_handle_ = cancel_handle.clone();
 
-      let fut = {
-        async move {
-          client
-            .send(request)
-            .map_err(Into::into)
-            .or_cancel(cancel_handle_)
-            .await
-        }
+      let fut = async move {
+        client
+          .send(request)
+          .map_err(Into::into)
+          .or_cancel(cancel_handle_)
+          .await
       };
 
       let request_rid = state.resource_table.add(FetchRequestResource {
@@ -501,12 +548,10 @@ where
       (request_rid, Some(cancel_handle_rid))
     }
     "data" => {
-      let data_url = DataUrl::process(url.as_str())
-        .map_err(|e| type_error(format!("{e:?}")))?;
+      let data_url =
+        DataUrl::process(url.as_str()).map_err(FetchError::DataUrl)?;
 
-      let (body, _) = data_url
-        .decode_to_vec()
-        .map_err(|e| type_error(format!("{e:?}")))?;
+      let (body, _) = data_url.decode_to_vec().map_err(FetchError::Base64)?;
       let body = http_body_util::Full::new(body.into())
         .map_err(|never| match never {})
         .boxed();
@@ -528,11 +573,9 @@ where
     "blob" => {
       // Blob URL resolution happens in the JS side of fetch. If we got here is
       // because the URL isn't an object URL.
-      return Err(type_error("Blob for the given URL not found."));
-    }
-    _ => {
-      return Err(type_error(format!("Url scheme '{scheme}' not supported")))
+      return Err(FetchError::BlobNotFound);
     }
+    _ => return Err(FetchError::SchemeNotSupported(scheme.to_string())),
   };
 
   Ok(FetchReturn {
@@ -564,11 +607,12 @@ pub struct FetchResponse {
 pub async fn op_fetch_send(
   state: Rc<RefCell<OpState>>,
   #[smi] rid: ResourceId,
-) -> Result<FetchResponse, AnyError> {
+) -> Result<FetchResponse, FetchError> {
   let request = state
     .borrow_mut()
     .resource_table
-    .take::<FetchRequestResource>(rid)?;
+    .take::<FetchRequestResource>(rid)
+    .map_err(FetchError::Resource)?;
 
   let request = Rc::try_unwrap(request)
     .ok()
@@ -581,22 +625,23 @@ pub async fn op_fetch_send(
       // If any error in the chain is a hyper body error, return that as a special result we can use to
       // reconstruct an error chain (eg: `new TypeError(..., { cause: new Error(...) })`).
       // TODO(mmastrac): it would be a lot easier if we just passed a v8::Global through here instead
-      let mut err_ref: &dyn std::error::Error = err.as_ref();
-      while let Some(err_src) = std::error::Error::source(err_ref) {
-        if let Some(err_src) = err_src.downcast_ref::<hyper::Error>() {
-          if let Some(err_src) = std::error::Error::source(err_src) {
-            return Ok(FetchResponse {
-              error: Some((err.to_string(), err_src.to_string())),
-              ..Default::default()
-            });
+
+      if let FetchError::ClientSend(err_src) = &err {
+        if let Some(client_err) = std::error::Error::source(&err_src.source) {
+          if let Some(err_src) = client_err.downcast_ref::<hyper::Error>() {
+            if let Some(err_src) = std::error::Error::source(err_src) {
+              return Ok(FetchResponse {
+                error: Some((err.to_string(), err_src.to_string())),
+                ..Default::default()
+              });
+            }
           }
         }
-        err_ref = err_src;
       }
 
-      return Err(type_error(err.to_string()));
+      return Err(err);
     }
-    Err(_) => return Err(type_error("Request was cancelled")),
+    Err(_) => return Err(FetchError::RequestCanceled),
   };
 
   let status = res.status();
@@ -636,7 +681,7 @@ pub async fn op_fetch_send(
 }
 
 type CancelableResponseResult =
-  Result<Result<http::Response<ResBody>, AnyError>, Canceled>;
+  Result<Result<http::Response<ResBody>, FetchError>, Canceled>;
 
 pub struct FetchRequestResource {
   pub future: Pin<Box<dyn Future<Output = CancelableResponseResult>>>,
@@ -691,7 +736,7 @@ impl FetchResponseResource {
     }
   }
 
-  pub async fn upgrade(self) -> Result<hyper::upgrade::Upgraded, AnyError> {
+  pub async fn upgrade(self) -> Result<hyper::upgrade::Upgraded, hyper::Error> {
     let reader = self.response_reader.into_inner();
     match reader {
       FetchResponseReader::Start(resp) => Ok(hyper::upgrade::on(resp).await?),
@@ -746,7 +791,9 @@ impl Resource for FetchResponseResource {
             // safely call `await` on it without creating a race condition.
             Some(_) => match reader.as_mut().next().await.unwrap() {
               Ok(chunk) => assert!(chunk.is_empty()),
-              Err(err) => break Err(type_error(err.to_string())),
+              Err(err) => {
+                break Err(deno_core::error::type_error(err.to_string()))
+              }
             },
             None => break Ok(BufView::empty()),
           }
@@ -809,14 +856,16 @@ pub fn op_fetch_custom_client<FP>(
   state: &mut OpState,
   #[serde] args: CreateHttpClientArgs,
   #[cppgc] tls_keys: &TlsKeysHolder,
-) -> Result<ResourceId, AnyError>
+) -> Result<ResourceId, FetchError>
 where
   FP: FetchPermissions + 'static,
 {
   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()")?;
+    permissions
+      .check_net_url(&url, "Deno.createHttpClient()")
+      .map_err(FetchError::Permission)?;
   }
 
   let options = state.borrow::<Options>();
@@ -829,7 +878,9 @@ where
   let client = create_http_client(
     &options.user_agent,
     CreateHttpClientOptions {
-      root_cert_store: options.root_cert_store()?,
+      root_cert_store: options
+        .root_cert_store()
+        .map_err(HttpClientCreateError::RootCertStore)?,
       ca_certs,
       proxy: args.proxy,
       unsafely_ignore_certificate_errors: options
@@ -887,19 +938,34 @@ impl Default for CreateHttpClientOptions {
   }
 }
 
+#[derive(Debug, thiserror::Error)]
+pub enum HttpClientCreateError {
+  #[error(transparent)]
+  Tls(deno_tls::TlsError),
+  #[error("Illegal characters in User-Agent: received {0}")]
+  InvalidUserAgent(String),
+  #[error("invalid proxy url")]
+  InvalidProxyUrl,
+  #[error("Cannot create Http Client: either `http1` or `http2` needs to be set to true")]
+  HttpVersionSelectionInvalid,
+  #[error(transparent)]
+  RootCertStore(deno_core::error::AnyError),
+}
+
 /// Create new instance of async Client. This client supports
 /// proxies and doesn't follow redirects.
 pub fn create_http_client(
   user_agent: &str,
   options: CreateHttpClientOptions,
-) -> Result<Client, AnyError> {
+) -> Result<Client, HttpClientCreateError> {
   let mut tls_config = deno_tls::create_client_config(
     options.root_cert_store,
     options.ca_certs,
     options.unsafely_ignore_certificate_errors,
     options.client_cert_chain_and_key.into(),
     deno_tls::SocketUse::Http,
-  )?;
+  )
+  .map_err(HttpClientCreateError::Tls)?;
 
   // Proxy TLS should not send ALPN
   tls_config.alpn_protocols.clear();
@@ -919,9 +985,7 @@ pub fn create_http_client(
   http_connector.enforce_http(false);
 
   let user_agent = user_agent.parse::<HeaderValue>().map_err(|_| {
-    type_error(format!(
-      "Illegal characters in User-Agent: received {user_agent}"
-    ))
+    HttpClientCreateError::InvalidUserAgent(user_agent.to_string())
   })?;
 
   let mut builder =
@@ -932,7 +996,7 @@ pub fn create_http_client(
   let mut proxies = proxy::from_env();
   if let Some(proxy) = options.proxy {
     let mut intercept = proxy::Intercept::all(&proxy.url)
-      .ok_or_else(|| type_error("invalid proxy url"))?;
+      .ok_or_else(|| HttpClientCreateError::InvalidProxyUrl)?;
     if let Some(basic_auth) = &proxy.basic_auth {
       intercept.set_auth(&basic_auth.username, &basic_auth.password);
     }
@@ -964,7 +1028,7 @@ pub fn create_http_client(
     }
     (true, true) => {}
     (false, false) => {
-      return Err(type_error("Cannot create Http Client: either `http1` or `http2` needs to be set to true"))
+      return Err(HttpClientCreateError::HttpVersionSelectionInvalid)
     }
   }
 
@@ -980,10 +1044,8 @@ pub fn create_http_client(
 
 #[op2]
 #[serde]
-pub fn op_utf8_to_byte_string(
-  #[string] input: String,
-) -> Result<ByteString, AnyError> {
-  Ok(input.into())
+pub fn op_utf8_to_byte_string(#[string] input: String) -> ByteString {
+  input.into()
 }
 
 #[derive(Clone, Debug)]
@@ -1003,7 +1065,7 @@ const STAR_STAR: HeaderValue = HeaderValue::from_static("*/*");
 #[derive(Debug)]
 pub struct ClientSendError {
   uri: Uri,
-  source: hyper_util::client::legacy::Error,
+  pub source: hyper_util::client::legacy::Error,
 }
 
 impl ClientSendError {
@@ -1075,12 +1137,14 @@ impl Client {
       .oneshot(req)
       .await
       .map_err(|e| ClientSendError { uri, source: e })?;
-    Ok(resp.map(|b| b.map_err(|e| anyhow!(e)).boxed()))
+    Ok(resp.map(|b| b.map_err(|e| deno_core::anyhow::anyhow!(e)).boxed()))
   }
 }
 
-pub type ReqBody = http_body_util::combinators::BoxBody<Bytes, Error>;
-pub type ResBody = http_body_util::combinators::BoxBody<Bytes, Error>;
+pub type ReqBody =
+  http_body_util::combinators::BoxBody<Bytes, deno_core::error::AnyError>;
+pub type ResBody =
+  http_body_util::combinators::BoxBody<Bytes, deno_core::error::AnyError>;
 
 /// Copied from https://github.com/seanmonstar/reqwest/blob/b9d62a0323d96f11672a61a17bf8849baec00275/src/async_impl/request.rs#L572
 /// Check the request URL for a "username:password" type authority, and if