Viaduct dependency fixes (#7106)

Removed what I hope are the last remaining `tokio`/`hyper` dependencies
in our components:
 - Updated `ads-client` to use `viaduct-dev` for its integration tests.
 - Removed the unused `backend-dev` feature from `viaduct`.

 Also improved the error handling on `viaduct-dev`.  If you remove the
 `#[ignore]` attribute from the ads-client integration tests, they now
 correctly report that they're failing because they're trying to hit
 HTTPS endpoints.  I think you can re-enable these tests by configuring
 them to hit `http://ads.allizom.org` instead of `https`.

Author: bendk

Cargo.lock

@@ -33,7 +33,6 @@ sql-support = { path = "../support/sql" }
 mockall = "0.12"
 mockito = { version = "0.31", default-features = false }
 viaduct-dev = { path = "../support/viaduct-dev" }
-viaduct-reqwest = { path = "../support/viaduct-reqwest" }
 
 [build-dependencies]
 uniffi = { version = "0.29.0", features = ["build"] }

components/ads-client/Cargo.toml

@@ -15,7 +15,7 @@ use viaduct::Request;
 #[test]
 #[ignore]
 fn test_mock_pocket_billboard_1_placement() {
-    viaduct_reqwest::use_reqwest_backend();
+    viaduct_dev::init_backend_dev();
 
     let client = MozAdsClient::new(None);
 
@@ -25,7 +25,6 @@ fn test_mock_pocket_billboard_1_placement() {
     };
 
     let result = client.request_image_ads(vec![placement_request], None);
-    println!("result: {:?}", result);
 
     assert!(result.is_ok(), "Failed to request ads: {:?}", result.err());
 
@@ -44,7 +43,7 @@ fn test_mock_pocket_billboard_1_placement() {
 #[test]
 #[ignore]
 fn test_newtab_spocs_placement() {
-    viaduct_reqwest::use_reqwest_backend();
+    viaduct_dev::init_backend_dev();
 
     let client = MozAdsClient::new(None);
 
@@ -56,7 +55,6 @@ fn test_newtab_spocs_placement() {
     };
 
     let result = client.request_spoc_ads(vec![placement_request], None);
-    println!("result: {:?}", result);
 
     assert!(result.is_ok(), "Failed to request ads: {:?}", result.err());
 
@@ -81,7 +79,7 @@ fn test_newtab_spocs_placement() {
 #[test]
 #[ignore]
 fn test_newtab_tile_1_placement() {
-    viaduct_reqwest::use_reqwest_backend();
+    viaduct_dev::init_backend_dev();
 
     let client = MozAdsClient::new(None);
 
@@ -91,7 +89,6 @@ fn test_newtab_tile_1_placement() {
     };
 
     let result = client.request_tile_ads(vec![placement_request], None);
-    println!("result: {:?}", result);
 
     assert!(result.is_ok(), "Failed to request ads: {:?}", result.err());
 
@@ -110,7 +107,7 @@ fn test_newtab_tile_1_placement() {
 #[test]
 #[ignore]
 fn test_cache_works_using_real_timeouts() {
-    viaduct_reqwest::use_reqwest_backend();
+    viaduct_dev::init_backend_dev();
 
     let cache = HttpCache::builder("integration_tests.db")
         .default_ttl(Duration::from_secs(60))

components/ads-client/tests/integration_test.rs

@@ -29,7 +29,7 @@ enum Event {
     SendRequest {
         request: Request,
         settings: ClientSettings,
-        channel: oneshot::Sender<Response>,
+        channel: oneshot::Sender<Result<Response>>,
     },
     Quit,
 }
@@ -49,9 +49,10 @@ fn worker_thread(channel: mpsc::Receiver<Event>) {
                 settings,
                 channel,
             }) => {
-                if let Err(e) = send_request(request, settings, channel) {
-                    error!("Error sending request: {e}");
-                }
+                let result = send_request(request, settings);
+                channel
+                    .send(result)
+                    .expect("Error sending to oneshot channel");
             }
             Ok(Event::Quit) => {
                 info!("Saw Quit event, exiting");
@@ -62,11 +63,7 @@ fn worker_thread(channel: mpsc::Receiver<Event>) {
 }
 
 /// Handle `Event::SendRequest`
-fn send_request(
-    request: Request,
-    settings: ClientSettings,
-    channel: oneshot::Sender<Response>,
-) -> Result<()> {
+fn send_request(request: Request, settings: ClientSettings) -> Result<Response> {
     let method = match request.method {
         Method::Get => minreq::Method::Get,
         Method::Head => minreq::Method::Head,
@@ -89,18 +86,15 @@ fn send_request(
         .with_timeout(settings.timeout.div_ceil(1000) as u64)
         .with_body(request.body.unwrap_or_default());
     let mut resp = req.send().map_backend_error()?;
-    channel
-        .send(Response {
-            request_method: request.method,
-            url: Url::parse(&resp.url)?,
-            // Use `take` to take all headers, but not partially deconstruct the `Response`.
-            // This lets us use `into_bytes()` below.
-            headers: Headers::try_from_hashmap(std::mem::take(&mut resp.headers))?,
-            status: resp.status_code as u16,
-            body: resp.into_bytes(),
-        })
-        .map_backend_error()?;
-    Ok(())
+    Ok(Response {
+        request_method: request.method,
+        url: Url::parse(&resp.url)?,
+        // Use `take` to take all headers, but not partially deconstruct the `Response`.
+        // This lets us use `into_bytes()` below.
+        headers: Headers::try_from_hashmap(std::mem::take(&mut resp.headers))?,
+        status: resp.status_code as u16,
+        body: resp.into_bytes(),
+    })
 }
 
 /// Initialize the `dev` backend.
@@ -155,6 +149,6 @@ impl Backend for DevBackend {
             })
             .map_backend_error()?;
         // Await the response from the worker thread.
-        oneshot_rx.await.map_backend_error()
+        oneshot_rx.await.expect("Error awaiting oneshot channel")
     }
 }

components/support/viaduct-dev/src/lib.rs

@@ -22,12 +22,9 @@ prost = "0.13"
 ffi-support = "0.4"
 thiserror = "2"
 uniffi = { version = "0.29.0" }
-tokio = { version = "1", features = ["rt-multi-thread"], optional = true }
-hyper = { version = "0.14", features = ["client", "http1", "http2", "tcp"], optional = true }
 bhttp = { git = "https://github.com/martinthomson/ohttp.git", rev = "bf6a983845cc0b540effb3a615e92d914dfcfd0b", optional = true }
 ohttp = { git = "https://github.com/martinthomson/ohttp.git", rev = "bf6a983845cc0b540effb3a615e92d914dfcfd0b", features = ["client", "server", "app-svc", "external-sqlite"], default-features = false, optional = true }
 
 [features]
 default = []
-backend-dev = ["dep:tokio", "dep:hyper"]
 ohttp = ["dep:bhttp", "dep:ohttp"]