Fix a bug where HTTP errors where being reported incorrectly (#18828)

Mikayla Maki and Marshall Bowers created 1 year ago

Release Notes:

- N/A

---------

Co-authored-by: Marshall Bowers <[e-mail]>

Change summary

crates/gpui/src/app.rs                      |  4 +++
crates/http_client/src/http_client.rs       | 27 ++++++++++++++++++++++
crates/reqwest_client/src/reqwest_client.rs |  6 ++++
crates/ureq_client/src/ureq_client.rs       | 15 +++++++++++
4 files changed, 50 insertions(+), 2 deletions(-)

Detailed changes

crates/gpui/src/app.rs

@@ -1533,4 +1533,8 @@ impl HttpClient for NullHttpClient {
     fn proxy(&self) -> Option<&http_client::Uri> {
         None
     }
+
+    fn type_name(&self) -> &'static str {
+        type_name::<Self>()
+    }
 }

crates/http_client/src/http_client.rs

@@ -11,6 +11,7 @@ use http::request::Builder;
 #[cfg(feature = "test-support")]
 use std::fmt;
 use std::{
+    any::type_name,
     sync::{Arc, LazyLock, Mutex},
     time::Duration,
 };
@@ -72,6 +73,8 @@ impl HttpRequestExt for http::request::Builder {
 }
 
 pub trait HttpClient: 'static + Send + Sync {
+    fn type_name(&self) -> &'static str;
+
     fn send(
         &self,
         req: http::Request<AsyncBody>,
@@ -154,6 +157,10 @@ impl HttpClient for HttpClientWithProxy {
     fn proxy(&self) -> Option<&Uri> {
         self.proxy.as_ref()
     }
+
+    fn type_name(&self) -> &'static str {
+        self.client.type_name()
+    }
 }
 
 impl HttpClient for Arc<HttpClientWithProxy> {
@@ -167,6 +174,10 @@ impl HttpClient for Arc<HttpClientWithProxy> {
     fn proxy(&self) -> Option<&Uri> {
         self.proxy.as_ref()
     }
+
+    fn type_name(&self) -> &'static str {
+        self.client.type_name()
+    }
 }
 
 /// An [`HttpClient`] that has a base URL.
@@ -278,6 +289,10 @@ impl HttpClient for Arc<HttpClientWithUrl> {
     fn proxy(&self) -> Option<&Uri> {
         self.client.proxy.as_ref()
     }
+
+    fn type_name(&self) -> &'static str {
+        self.client.type_name()
+    }
 }
 
 impl HttpClient for HttpClientWithUrl {
@@ -291,6 +306,10 @@ impl HttpClient for HttpClientWithUrl {
     fn proxy(&self) -> Option<&Uri> {
         self.client.proxy.as_ref()
     }
+
+    fn type_name(&self) -> &'static str {
+        self.client.type_name()
+    }
 }
 
 pub fn read_proxy_from_env() -> Option<Uri> {
@@ -331,6 +350,10 @@ impl HttpClient for BlockedHttpClient {
     fn proxy(&self) -> Option<&Uri> {
         None
     }
+
+    fn type_name(&self) -> &'static str {
+        type_name::<Self>()
+    }
 }
 
 #[cfg(feature = "test-support")]
@@ -403,4 +426,8 @@ impl HttpClient for FakeHttpClient {
     fn proxy(&self) -> Option<&Uri> {
         None
     }
+
+    fn type_name(&self) -> &'static str {
+        type_name::<Self>()
+    }
 }

crates/reqwest_client/src/reqwest_client.rs

@@ -1,4 +1,4 @@
-use std::{borrow::Cow, io::Read, pin::Pin, task::Poll};
+use std::{any::type_name, borrow::Cow, io::Read, pin::Pin, task::Poll};
 
 use anyhow::anyhow;
 use bytes::{BufMut, Bytes, BytesMut};
@@ -183,6 +183,10 @@ impl http_client::HttpClient for ReqwestClient {
         None
     }
 
+    fn type_name(&self) -> &'static str {
+        type_name::<Self>()
+    }
+
     fn send(
         &self,
         req: http::Request<http_client::AsyncBody>,

crates/ureq_client/src/ureq_client.rs

@@ -1,3 +1,4 @@
+use std::any::type_name;
 use std::collections::HashMap;
 use std::io::Read;
 use std::sync::Arc;
@@ -74,6 +75,10 @@ impl HttpClient for UreqClient {
         self.proxy_url.as_ref()
     }
 
+    fn type_name(&self) -> &'static str {
+        type_name::<Self>()
+    }
+
     fn send(
         &self,
         request: http::Request<AsyncBody>,
@@ -100,7 +105,15 @@ impl HttpClient for UreqClient {
 
         self.background_executor
             .spawn(async move {
-                let response = req.send(body)?;
+                let response = match req.send(body) {
+                    Ok(response) => response,
+                    Err(e) => match e {
+                        ureq::Error::Status(_, response) => response,
+                        ureq::Error::Transport(transport) => {
+                            anyhow::bail!(transport)
+                        }
+                    },
+                };
 
                 let mut builder = http::Response::builder()
                     .status(response.status())