fix(ext/fetch): include URL and error details on fetch failures (#24910)

This commit improves error messages that `fetch` generates on failure.

Fixes #24835

3 files changed, 64 insertions, 14 deletions

diff --git a/ext/fetch/26_fetch.js b/ext/fetch/26_fetch.js
index 674d99709..e2809187b 100644
--- a/ext/fetch/26_fetch.js
+++ b/ext/fetch/26_fetch.js
@@ -65,7 +65,7 @@ const REQUEST_BODY_HEADER_NAMES = [
 
 /**
  * @param {number} rid
- * @returns {Promise<{ status: number, statusText: string, headers: [string, string][], url: string, responseRid: number, error: string? }>}
+ * @returns {Promise<{ status: number, statusText: string, headers: [string, string][], url: string, responseRid: number, error: [string, string]? }>}
  */
 function opFetchSend(rid) {
   return op_fetch_send(rid);
@@ -177,8 +177,9 @@ async function mainFetch(req, recursive, terminator) {
     }
   }
   // Re-throw any body errors
-  if (resp.error) {
-    throw new TypeError("body failed", { cause: new Error(resp.error) });
+  if (resp.error !== null) {
+    const { 0: message, 1: cause } = resp.error;
+    throw new TypeError(message, { cause: new Error(cause) });
   }
   if (terminator.aborted) return abortedNetworkError();
 
diff --git a/ext/fetch/Cargo.toml b/ext/fetch/Cargo.toml
index f0e06c87f..75a358b2d 100644
--- a/ext/fetch/Cargo.toml
+++ b/ext/fetch/Cargo.toml
@@ -21,6 +21,7 @@ deno_core.workspace = true
 deno_permissions.workspace = true
 deno_tls.workspace = true
 dyn-clone = "1"
+error_reporter = "1"
 http.workspace = true
 http-body-util.workspace = true
 hyper.workspace = true
diff --git a/ext/fetch/lib.rs b/ext/fetch/lib.rs
index 7c717ccec..798acad0b 100644
--- a/ext/fetch/lib.rs
+++ b/ext/fetch/lib.rs
@@ -26,6 +26,7 @@ use deno_core::futures::Future;
 use deno_core::futures::FutureExt;
 use deno_core::futures::Stream;
 use deno_core::futures::StreamExt;
+use deno_core::futures::TryFutureExt;
 use deno_core::op2;
 use deno_core::unsync::spawn;
 use deno_core::url::Url;
@@ -429,7 +430,7 @@ where
 
       let mut request = http::Request::new(body);
       *request.method_mut() = method.clone();
-      *request.uri_mut() = uri;
+      *request.uri_mut() = uri.clone();
 
       if let Some((username, password)) = maybe_authority {
         request.headers_mut().insert(
@@ -469,8 +470,15 @@ where
       let cancel_handle = CancelHandle::new_rc();
       let cancel_handle_ = cancel_handle.clone();
 
-      let fut =
-        async move { client.send(request).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 {
         future: Box::pin(fut),
@@ -532,7 +540,11 @@ pub struct FetchResponse {
   pub content_length: Option<u64>,
   pub remote_addr_ip: Option<String>,
   pub remote_addr_port: Option<u16>,
-  pub error: Option<String>,
+  /// This field is populated if some error occurred which needs to be
+  /// reconstructed in the JS side to set the error _cause_.
+  /// In the tuple, the first element is an error message and the second one is
+  /// an error cause.
+  pub error: Option<(String, String)>,
 }
 
 #[op2(async)]
@@ -558,16 +570,16 @@ pub async fn op_fetch_send(
       // 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) = std::error::Error::source(err_ref) {
-        if let Some(err) = err.downcast_ref::<hyper::Error>() {
-          if let Some(err) = std::error::Error::source(err) {
+      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()),
+              error: Some((err.to_string(), err_src.to_string())),
               ..Default::default()
             });
           }
         }
-        err_ref = err;
+        err_ref = err_src;
       }
 
       return Err(type_error(err.to_string()));
@@ -1082,11 +1094,41 @@ type Connector = proxy::ProxyConnector<HttpConnector>;
 #[allow(clippy::declare_interior_mutable_const)]
 const STAR_STAR: HeaderValue = HeaderValue::from_static("*/*");
 
+#[derive(Debug)]
+pub struct ClientSendError {
+  uri: Uri,
+  source: hyper_util::client::legacy::Error,
+}
+
+impl ClientSendError {
+  pub fn is_connect_error(&self) -> bool {
+    self.source.is_connect()
+  }
+}
+
+impl std::fmt::Display for ClientSendError {
+  fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+    write!(
+      f,
+      "error sending request for url ({uri}): {source}",
+      uri = self.uri,
+      // NOTE: we can use `std::error::Report` instead once it's stabilized.
+      source = error_reporter::Report::new(&self.source),
+    )
+  }
+}
+
+impl std::error::Error for ClientSendError {
+  fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+    Some(&self.source)
+  }
+}
+
 impl Client {
   pub async fn send(
     self,
     mut req: http::Request<ReqBody>,
-  ) -> Result<http::Response<ResBody>, AnyError> {
+  ) -> Result<http::Response<ResBody>, ClientSendError> {
     req
       .headers_mut()
       .entry(USER_AGENT)
@@ -1098,7 +1140,13 @@ impl Client {
       req.headers_mut().insert(PROXY_AUTHORIZATION, auth.clone());
     }
 
-    let resp = self.inner.oneshot(req).await?;
+    let uri = req.uri().clone();
+
+    let resp = self
+      .inner
+      .oneshot(req)
+      .await
+      .map_err(|e| ClientSendError { uri, source: e })?;
     Ok(resp.map(|b| b.map_err(|e| anyhow!(e)).boxed()))
   }
 }