From 0d7a79df1150c1a84c334ea15459198791f00c03 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Mon, 27 Jul 2026 16:59:02 -0700 Subject: [PATCH 1/6] feat: Initial working tiles version --- .../integration-tests/tests/mars_async.rs | 75 +++++++++++++++++++ components/ads-client/src/client/error.rs | 7 +- components/ads-client/src/ffi.rs | 20 +++-- components/ads-client/src/lib.rs | 61 ++++++++++++++- components/ads-client/src/worker.rs | 10 +++ 5 files changed, 163 insertions(+), 10 deletions(-) create mode 100644 components/ads-client/integration-tests/tests/mars_async.rs create mode 100644 components/ads-client/src/worker.rs diff --git a/components/ads-client/integration-tests/tests/mars_async.rs b/components/ads-client/integration-tests/tests/mars_async.rs new file mode 100644 index 0000000000..125455ed1b --- /dev/null +++ b/components/ads-client/integration-tests/tests/mars_async.rs @@ -0,0 +1,75 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public +* License, v. 2.0. If a copy of the MPL was not distributed with this +* file, You can obtain one at http://mozilla.org/MPL/2.0/. +*/ + +use std::{collections::HashMap, sync::{Arc, mpsc::{self, Sender}}}; + +use ads_client::{ + DispatchCommand, MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, +}; + +fn init_backend() { + viaduct_hyper::viaduct_init_backend_hyper(); +} + +fn prod_client() -> ads_client::MozAdsClient { + Arc::new(MozAdsClientBuilder::new()) + .environment(MozAdsEnvironment::Prod) + .build() +} + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_contract_tile_prod_async() { + init_backend(); + + let client = prod_client(); + + let (success_tx, success_rx) = mpsc::channel(); + let (err_tx, err_rx) = mpsc::channel(); + pub struct TileCallback { + on_ad_fn: Box, Sender>) + Send + Sync>, + on_error_fn: Box) + Send + Sync>, + success_tx: Sender>, + err_tx: Sender, + } + + impl ads_client::TileRequestCallback for TileCallback { + fn on_ad(&self,tiles: std::collections::HashMap) { + (self.on_ad_fn)(tiles, self.success_tx.clone()) + } + fn on_error(&self,err: ads_client::MozAdsClientApiError) { + (self.on_error_fn)(err, self.err_tx.clone()) + } + } + + let callback = TileCallback { + success_tx, + err_tx, + on_ad_fn: Box::new(|tiles, tx| { + tx.send(tiles).expect("Testing channels should not hang up.") + }), + on_error_fn: Box::new(|err, tx| { + tx.send(err).expect("Testing channels should not hang up.") + }) + }; + + client.dispatch( + DispatchCommand::RequestTileAd { moz_ad_requests: vec![MozAdsPlacementRequest { + iab_content: None, + placement_id: "mock_tile_1".to_string(), + }], options: None, callback: Arc::new(callback) + }).expect("Asynchronous dispatch should return Ok()"); + + loop { + if let Ok(placements) = success_rx.recv() { + assert!(placements.contains_key("mock_tile_1")); + break; + } + if let Ok(err) = err_rx.recv() { + panic!("Tile ad request failed: {:?}", err); + } + + } +} \ No newline at end of file diff --git a/components/ads-client/src/client/error.rs b/components/ads-client/src/client/error.rs index 2542939493..626eb3805f 100644 --- a/components/ads-client/src/client/error.rs +++ b/components/ads-client/src/client/error.rs @@ -3,7 +3,9 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::mars::error::{FetchAdsError, RecordClickError, RecordImpressionError, ReportAdError}; +use std::sync::mpsc::SendError; + +use crate::{DispatchCommand, mars::error::{FetchAdsError, RecordClickError, RecordImpressionError, ReportAdError}}; #[derive(Debug, thiserror::Error)] pub enum ComponentError { @@ -18,6 +20,9 @@ pub enum ComponentError { #[error("Error requesting ads: {0}")] RequestAds(#[from] RequestAdsError), + + #[error("Error dispatching an asynchronous command: {0}")] + Dispatch(SendError) } #[derive(Debug, thiserror::Error)] diff --git a/components/ads-client/src/ffi.rs b/components/ads-client/src/ffi.rs index ba50352fdc..4653d94905 100644 --- a/components/ads-client/src/ffi.rs +++ b/components/ads-client/src/ffi.rs @@ -6,7 +6,7 @@ pub mod error; pub mod telemetry; -use std::sync::Arc; +use std::sync::{Arc, mpsc}; use crate::client::config::{AdsCacheConfig, AdsClientConfig}; use crate::client::{AdsClient, ContextIdProvider}; @@ -20,7 +20,7 @@ use crate::mars::ad_response::{ }; use crate::mars::Environment; use crate::mars::ReportReason; -use crate::AdsClientUrl; +use crate::{AdsClientUrl, DispatchCommand}; use crate::MozAdsClient; use parking_lot::Mutex; use std::collections::HashMap; @@ -55,7 +55,7 @@ impl From for Box { } } -#[derive(Default, uniffi::Record)] +#[derive(Default, Clone, Debug, uniffi::Record)] pub struct MozAdsRequestOptions { pub cache_policy: Option, #[uniffi(default)] @@ -64,7 +64,7 @@ pub struct MozAdsRequestOptions { pub ohttp: bool, } -#[derive(Default, uniffi::Record)] +#[derive(Default, Debug, Clone, uniffi::Record)] pub struct MozAdsCallbackOptions { #[uniffi(default = false)] pub ohttp: bool, @@ -138,9 +138,17 @@ impl MozAdsClientBuilder { .map(MozAdsTelemetryWrapper::new) .unwrap_or_else(MozAdsTelemetryWrapper::noop), }; - let client = AdsClient::new(client_config); + let client = Arc::new(Mutex::new(AdsClient::new(client_config))); + let client_clone = client.clone(); + + let (tx, rx)= mpsc::channel::(); + let worker_thread_handle = std::thread::spawn(move || crate::worker::worker(client_clone, rx)); + MozAdsClient { - inner: Mutex::new(client), + inner: client, + _worker_thread_handle: worker_thread_handle, + command_tx: tx + } } diff --git a/components/ads-client/src/lib.rs b/components/ads-client/src/lib.rs index 2088ee16cb..501f63e02a 100644 --- a/components/ads-client/src/lib.rs +++ b/components/ads-client/src/lib.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use std::collections::HashMap; +use std::{collections::HashMap, sync::{Arc, mpsc::Sender}, thread::JoinHandle}; use client::error::ComponentError; use error_support::handle_error; @@ -19,12 +19,12 @@ mod client; mod ffi; pub mod http_cache; mod mars; +pub mod worker; pub mod telemetry; pub use ffi::*; use crate::ffi::telemetry::MozAdsTelemetryWrapper; - #[cfg(test)] mod test_utils; @@ -38,8 +38,12 @@ uniffi::custom_type!(AdsClientUrl, String, { #[derive(uniffi::Object)] pub struct MozAdsClient { - inner: Mutex>, + inner: Arc>>, + _worker_thread_handle : JoinHandle<()>, + command_tx: Sender + } +pub type MozAdsClientInner = Arc>>; #[uniffi::export] impl MozAdsClient { @@ -161,4 +165,55 @@ impl MozAdsClient { .map_err(ComponentError::RequestAds)?; Ok(response.into_iter().map(|(k, v)| (k, v.into())).collect()) } + + #[handle_error(ComponentError)] + #[uniffi::method()] + pub fn dispatch( + &self, + command : DispatchCommand + ) -> AdsClientApiResult<()> { + self.command_tx.send(command).map_err(ComponentError::Dispatch)?; + Ok(()) + } +} + +#[derive(uniffi::Enum)] + +pub enum DispatchCommand { + RequestTileAd { + moz_ad_requests: Vec, + options: Option, + callback: Arc, + } +} + +impl DispatchCommand { + + #[handle_error(ComponentError)] + pub fn run_command(self, ads_client_inner : &MozAdsClientInner) -> AdsClientApiResult<()> { + // TODO: Duplicated behavior with the sync functions- either they call this or you call those + match self { + DispatchCommand::RequestTileAd { moz_ad_requests, options, callback } => { + let inner = ads_client_inner.lock(); + let requests: Vec = moz_ad_requests.iter().map(|r| r.into()).collect(); + let options = options.unwrap_or_default(); + let flags = AdRequestFlags::from(&options); + let ohttp = options.ohttp; + let cache_policy: CachePolicy = options.into(); + let response = inner + .request_tile_ads(requests, flags, Some(cache_policy), ohttp) + .map_err(ComponentError::RequestAds)?; + let tiles = response.into_iter().map(|(k, v)| (k, v.into())).collect(); + callback.on_ad(tiles); + // TODO: error, modular version + } + } + Ok(()) + } +} + +#[uniffi::export(with_foreign)] +pub trait TileRequestCallback : Send + Sync { + fn on_ad(&self, tiles : HashMap); + fn on_error(&self, err: MozAdsClientApiError); } diff --git a/components/ads-client/src/worker.rs b/components/ads-client/src/worker.rs new file mode 100644 index 0000000000..68b20ec041 --- /dev/null +++ b/components/ads-client/src/worker.rs @@ -0,0 +1,10 @@ +use std::sync::mpsc::Receiver; +use crate::{DispatchCommand, MozAdsClientInner}; + +pub fn worker(inner_client : MozAdsClientInner, rx : Receiver) { + // TODO: This could be an async environment where we wait on buffered futures- can send many out at once, instead of FIFO. + while let Ok(task) = rx.recv() { + let task : DispatchCommand = task; + task.run_command(&inner_client).expect("TODO: handle error here. maybe retries should be here?"); + } +} From e05e1c223365c59908367c73c18f20927f009470 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Mon, 27 Jul 2026 17:38:49 -0700 Subject: [PATCH 2/6] feat: some small refactoring --- .../integration-tests/tests/mars_async.rs | 4 +- components/ads-client/src/client/error.rs | 2 +- components/ads-client/src/ffi.rs | 4 +- components/ads-client/src/lib.rs | 45 +----------------- components/ads-client/src/worker.rs | 47 ++++++++++++++++++- 5 files changed, 52 insertions(+), 50 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars_async.rs b/components/ads-client/integration-tests/tests/mars_async.rs index 125455ed1b..7e9ad2058a 100644 --- a/components/ads-client/integration-tests/tests/mars_async.rs +++ b/components/ads-client/integration-tests/tests/mars_async.rs @@ -6,7 +6,7 @@ use std::{collections::HashMap, sync::{Arc, mpsc::{self, Sender}}}; use ads_client::{ - DispatchCommand, MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, + MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, worker::{DispatchCommand, TileRequestCallback}, }; fn init_backend() { @@ -35,7 +35,7 @@ fn test_contract_tile_prod_async() { err_tx: Sender, } - impl ads_client::TileRequestCallback for TileCallback { + impl TileRequestCallback for TileCallback { fn on_ad(&self,tiles: std::collections::HashMap) { (self.on_ad_fn)(tiles, self.success_tx.clone()) } diff --git a/components/ads-client/src/client/error.rs b/components/ads-client/src/client/error.rs index 626eb3805f..71981c654d 100644 --- a/components/ads-client/src/client/error.rs +++ b/components/ads-client/src/client/error.rs @@ -5,7 +5,7 @@ use std::sync::mpsc::SendError; -use crate::{DispatchCommand, mars::error::{FetchAdsError, RecordClickError, RecordImpressionError, ReportAdError}}; +use crate::{mars::error::{FetchAdsError, RecordClickError, RecordImpressionError, ReportAdError}, worker::DispatchCommand}; #[derive(Debug, thiserror::Error)] pub enum ComponentError { diff --git a/components/ads-client/src/ffi.rs b/components/ads-client/src/ffi.rs index 4653d94905..c6d9fda7e0 100644 --- a/components/ads-client/src/ffi.rs +++ b/components/ads-client/src/ffi.rs @@ -20,8 +20,9 @@ use crate::mars::ad_response::{ }; use crate::mars::Environment; use crate::mars::ReportReason; -use crate::{AdsClientUrl, DispatchCommand}; +use crate::AdsClientUrl; use crate::MozAdsClient; +use crate::worker::DispatchCommand; use parking_lot::Mutex; use std::collections::HashMap; @@ -148,7 +149,6 @@ impl MozAdsClientBuilder { inner: client, _worker_thread_handle: worker_thread_handle, command_tx: tx - } } diff --git a/components/ads-client/src/lib.rs b/components/ads-client/src/lib.rs index 501f63e02a..fa0058cf72 100644 --- a/components/ads-client/src/lib.rs +++ b/components/ads-client/src/lib.rs @@ -24,7 +24,7 @@ pub mod telemetry; pub use ffi::*; -use crate::ffi::telemetry::MozAdsTelemetryWrapper; +use crate::{ffi::telemetry::MozAdsTelemetryWrapper, worker::DispatchCommand}; #[cfg(test)] mod test_utils; @@ -175,45 +175,4 @@ impl MozAdsClient { self.command_tx.send(command).map_err(ComponentError::Dispatch)?; Ok(()) } -} - -#[derive(uniffi::Enum)] - -pub enum DispatchCommand { - RequestTileAd { - moz_ad_requests: Vec, - options: Option, - callback: Arc, - } -} - -impl DispatchCommand { - - #[handle_error(ComponentError)] - pub fn run_command(self, ads_client_inner : &MozAdsClientInner) -> AdsClientApiResult<()> { - // TODO: Duplicated behavior with the sync functions- either they call this or you call those - match self { - DispatchCommand::RequestTileAd { moz_ad_requests, options, callback } => { - let inner = ads_client_inner.lock(); - let requests: Vec = moz_ad_requests.iter().map(|r| r.into()).collect(); - let options = options.unwrap_or_default(); - let flags = AdRequestFlags::from(&options); - let ohttp = options.ohttp; - let cache_policy: CachePolicy = options.into(); - let response = inner - .request_tile_ads(requests, flags, Some(cache_policy), ohttp) - .map_err(ComponentError::RequestAds)?; - let tiles = response.into_iter().map(|(k, v)| (k, v.into())).collect(); - callback.on_ad(tiles); - // TODO: error, modular version - } - } - Ok(()) - } -} - -#[uniffi::export(with_foreign)] -pub trait TileRequestCallback : Send + Sync { - fn on_ad(&self, tiles : HashMap); - fn on_error(&self, err: MozAdsClientApiError); -} +} \ No newline at end of file diff --git a/components/ads-client/src/worker.rs b/components/ads-client/src/worker.rs index 68b20ec041..9b81f4ce61 100644 --- a/components/ads-client/src/worker.rs +++ b/components/ads-client/src/worker.rs @@ -1,5 +1,7 @@ -use std::sync::mpsc::Receiver; -use crate::{DispatchCommand, MozAdsClientInner}; +use std::{collections::HashMap, sync::{Arc, mpsc::Receiver}}; +use error_support::handle_error; + +use crate::{AdsClientApiResult, MozAdsClientApiError, MozAdsClientInner, MozAdsPlacementRequest, MozAdsRequestOptions, MozAdsTile, client::error::ComponentError, http_cache::CachePolicy, mars::ad_request::{AdPlacementRequest, AdRequestFlags}}; pub fn worker(inner_client : MozAdsClientInner, rx : Receiver) { // TODO: This could be an async environment where we wait on buffered futures- can send many out at once, instead of FIFO. @@ -8,3 +10,44 @@ pub fn worker(inner_client : MozAdsClientInner, rx : Receiver) task.run_command(&inner_client).expect("TODO: handle error here. maybe retries should be here?"); } } + +#[derive(uniffi::Enum)] + +pub enum DispatchCommand { + RequestTileAd { + moz_ad_requests: Vec, + options: Option, + callback: Arc, + } +} + +impl DispatchCommand { + + #[handle_error(ComponentError)] + pub fn run_command(self, ads_client_inner : &MozAdsClientInner) -> AdsClientApiResult<()> { + // TODO: Duplicated behavior with the sync functions- either they call this or you call those + match self { + DispatchCommand::RequestTileAd { moz_ad_requests, options, callback } => { + let inner = ads_client_inner.lock(); + let requests: Vec = moz_ad_requests.iter().map(|r| r.into()).collect(); + let options = options.unwrap_or_default(); + let flags = AdRequestFlags::from(&options); + let ohttp = options.ohttp; + let cache_policy: CachePolicy = options.into(); + let response = inner + .request_tile_ads(requests, flags, Some(cache_policy), ohttp) + .map_err(ComponentError::RequestAds)?; + let tiles = response.into_iter().map(|(k, v)| (k, v.into())).collect(); + callback.on_ad(tiles); + // TODO: error, modular version + } + } + Ok(()) + } +} + +#[uniffi::export(with_foreign)] +pub trait TileRequestCallback : Send + Sync { + fn on_ad(&self, tiles : HashMap); + fn on_error(&self, err: MozAdsClientApiError); +} From dc9a721d6e56d7f6a2e653303277a74236c96f8a Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Mon, 27 Jul 2026 18:36:22 -0700 Subject: [PATCH 3/6] feat: Adds other commands --- .../integration-tests/tests/mars_async.rs | 230 +++++++++++++++--- components/ads-client/src/client/error.rs | 7 +- components/ads-client/src/ffi.rs | 15 +- components/ads-client/src/lib.rs | 24 +- components/ads-client/src/worker.rs | 180 ++++++++++++-- 5 files changed, 390 insertions(+), 66 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars_async.rs b/components/ads-client/integration-tests/tests/mars_async.rs index 7e9ad2058a..40606ebd9e 100644 --- a/components/ads-client/integration-tests/tests/mars_async.rs +++ b/components/ads-client/integration-tests/tests/mars_async.rs @@ -3,10 +3,21 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use std::{collections::HashMap, sync::{Arc, mpsc::{self, Sender}}}; - use ads_client::{ - MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, worker::{DispatchCommand, TileRequestCallback}, + worker::{ + DispatchCommand, ErrorOnlyRequestCallback, ImageRequestCallback, SpocRequestCallback, + TileRequestCallback, + }, + MozAdsClientApiError, MozAdsClientBuilder, MozAdsEnvironment, MozAdsIABContent, + MozAdsIABContentTaxonomy, MozAdsImage, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount, + MozAdsRequestOptions, MozAdsSpoc, MozAdsTile, +}; +use std::{ + collections::HashMap, + sync::{ + mpsc::{self, Sender}, + Arc, + }, }; fn init_backend() { @@ -19,48 +30,204 @@ fn prod_client() -> ads_client::MozAdsClient { .build() } +// TODO: explain +pub struct CallbackTestStruct { + on_ad_fn: Box) + Send + Sync>, + on_error_fn: Box) + Send + Sync>, + success_tx: Sender, + err_tx: Sender, +} + +impl CallbackTestStruct { + pub fn new(success_tx: Sender, err_tx: Sender) -> CallbackTestStruct { + CallbackTestStruct { + success_tx, + err_tx, + on_ad_fn: Box::new(|tiles, tx| { + tx.send(tiles) + .expect("Testing channels should not hang up.") + }), + on_error_fn: Box::new(|err, tx| { + tx.send(err).expect("Testing channels should not hang up.") + }), + } + } +} + +impl ErrorOnlyRequestCallback for CallbackTestStruct<(), MozAdsClientApiError> { + fn on_error(&self, err: MozAdsClientApiError) { + (self.on_error_fn)(err, self.err_tx.clone()) + } +} + +impl ImageRequestCallback + for CallbackTestStruct, MozAdsClientApiError> +{ + fn on_ad(&self, ads: HashMap) { + (self.on_ad_fn)(ads, self.success_tx.clone()) + } + fn on_error(&self, err: MozAdsClientApiError) { + (self.on_error_fn)(err, self.err_tx.clone()) + } +} + +impl SpocRequestCallback + for CallbackTestStruct>, MozAdsClientApiError> +{ + fn on_ad(&self, ads: HashMap>) { + (self.on_ad_fn)(ads, self.success_tx.clone()) + } + fn on_error(&self, err: MozAdsClientApiError) { + (self.on_error_fn)(err, self.err_tx.clone()) + } +} + +impl TileRequestCallback for CallbackTestStruct, MozAdsClientApiError> { + fn on_ad(&self, tiles: HashMap) { + (self.on_ad_fn)(tiles, self.success_tx.clone()) + } + fn on_error(&self, err: MozAdsClientApiError) { + (self.on_error_fn)(err, self.err_tx.clone()) + } +} + #[test] #[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_tile_prod_async() { +fn test_contract_image_prod_async() { init_backend(); - let client = prod_client(); let (success_tx, success_rx) = mpsc::channel(); let (err_tx, err_rx) = mpsc::channel(); - pub struct TileCallback { - on_ad_fn: Box, Sender>) + Send + Sync>, - on_error_fn: Box) + Send + Sync>, - success_tx: Sender>, - err_tx: Sender, + let callback = CallbackTestStruct::, MozAdsClientApiError>::new( + success_tx, err_tx, + ); + + client + .dispatch(DispatchCommand::RequestImageAds { + moz_ad_requests: vec![MozAdsPlacementRequest { + iab_content: None, + placement_id: "mock_billboard_1".to_string(), + }], + options: None, + callback: Arc::new(callback), + }) + .expect("Asynchronous dispatch should return Ok()"); + + loop { + if let Ok(placements) = success_rx.recv() { + assert!(placements.contains_key("mock_billboard_1")); + break; + } + if let Ok(err) = err_rx.recv() { + panic!("Image ad request failed: {:?}", err); + } } +} + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_contract_image_with_categories_prod_async() { + init_backend(); + let client = prod_client(); - impl TileRequestCallback for TileCallback { - fn on_ad(&self,tiles: std::collections::HashMap) { - (self.on_ad_fn)(tiles, self.success_tx.clone()) + let (success_tx, success_rx) = mpsc::channel(); + let (err_tx, err_rx) = mpsc::channel(); + let callback = CallbackTestStruct::, MozAdsClientApiError>::new( + success_tx, err_tx, + ); + + client + .dispatch(DispatchCommand::RequestImageAds { + moz_ad_requests: vec![MozAdsPlacementRequest { + iab_content: Some(MozAdsIABContent { + category_ids: vec!["338".to_string()], + taxonomy: MozAdsIABContentTaxonomy::IAB3_0, + }), + placement_id: "mock_billboard_1".to_string(), + }], + options: Some(MozAdsRequestOptions { + flags: std::collections::HashMap::from([( + "contextual_placement".to_string(), + true, + )]), + ..Default::default() + }), + callback: Arc::new(callback), + }) + .expect("Asynchronous dispatch should return Ok()"); + + loop { + if let Ok(placements) = success_rx.recv() { + assert!(placements.contains_key("mock_billboard_1")); + break; } - fn on_error(&self,err: ads_client::MozAdsClientApiError) { - (self.on_error_fn)(err, self.err_tx.clone()) + if let Ok(err) = err_rx.recv() { + panic!("Image ad request failed: {:?}", err); } } +} - let callback = TileCallback { - success_tx, - err_tx, - on_ad_fn: Box::new(|tiles, tx| { - tx.send(tiles).expect("Testing channels should not hang up.") - }), - on_error_fn: Box::new(|err, tx| { - tx.send(err).expect("Testing channels should not hang up.") +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_contract_spoc_prod_async() { + init_backend(); + let client = prod_client(); + + let (success_tx, success_rx) = mpsc::channel(); + let (err_tx, err_rx) = mpsc::channel(); + let callback = + CallbackTestStruct::>, MozAdsClientApiError>::new( + success_tx, err_tx, + ); + + client + .dispatch(DispatchCommand::RequestSpocAds { + moz_ad_requests: vec![MozAdsPlacementRequestWithCount { + count: 3, + iab_content: None, + placement_id: "mock_spoc_1".to_string(), + }], + options: None, + callback: Arc::new(callback), }) - }; + .expect("Asynchronous dispatch should return Ok()"); + + loop { + if let Ok(placements) = success_rx.recv() { + assert!(placements.contains_key("mock_spoc_1")); + assert!(placements.get("mock_spoc_1").unwrap().len() == 3); + + break; + } + if let Ok(err) = err_rx.recv() { + panic!("Spoc ad request failed: {:?}", err); + } + } +} + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_contract_tile_prod_async() { + init_backend(); + let client = prod_client(); + + let (success_tx, success_rx) = mpsc::channel(); + let (err_tx, err_rx) = mpsc::channel(); + let callback = CallbackTestStruct::, MozAdsClientApiError>::new( + success_tx, err_tx, + ); - client.dispatch( - DispatchCommand::RequestTileAd { moz_ad_requests: vec![MozAdsPlacementRequest { - iab_content: None, - placement_id: "mock_tile_1".to_string(), - }], options: None, callback: Arc::new(callback) - }).expect("Asynchronous dispatch should return Ok()"); + client + .dispatch(DispatchCommand::RequestTileAd { + moz_ad_requests: vec![MozAdsPlacementRequest { + iab_content: None, + placement_id: "mock_tile_1".to_string(), + }], + options: None, + callback: Arc::new(callback), + }) + .expect("Asynchronous dispatch should return Ok()"); loop { if let Ok(placements) = success_rx.recv() { @@ -70,6 +237,5 @@ fn test_contract_tile_prod_async() { if let Ok(err) = err_rx.recv() { panic!("Tile ad request failed: {:?}", err); } - } -} \ No newline at end of file +} diff --git a/components/ads-client/src/client/error.rs b/components/ads-client/src/client/error.rs index 71981c654d..25823ae31b 100644 --- a/components/ads-client/src/client/error.rs +++ b/components/ads-client/src/client/error.rs @@ -5,7 +5,10 @@ use std::sync::mpsc::SendError; -use crate::{mars::error::{FetchAdsError, RecordClickError, RecordImpressionError, ReportAdError}, worker::DispatchCommand}; +use crate::{ + mars::error::{FetchAdsError, RecordClickError, RecordImpressionError, ReportAdError}, + worker::DispatchCommand, +}; #[derive(Debug, thiserror::Error)] pub enum ComponentError { @@ -22,7 +25,7 @@ pub enum ComponentError { RequestAds(#[from] RequestAdsError), #[error("Error dispatching an asynchronous command: {0}")] - Dispatch(SendError) + Dispatch(SendError), } #[derive(Debug, thiserror::Error)] diff --git a/components/ads-client/src/ffi.rs b/components/ads-client/src/ffi.rs index c6d9fda7e0..a4fcd3d206 100644 --- a/components/ads-client/src/ffi.rs +++ b/components/ads-client/src/ffi.rs @@ -6,7 +6,7 @@ pub mod error; pub mod telemetry; -use std::sync::{Arc, mpsc}; +use std::sync::{mpsc, Arc}; use crate::client::config::{AdsCacheConfig, AdsClientConfig}; use crate::client::{AdsClient, ContextIdProvider}; @@ -20,9 +20,9 @@ use crate::mars::ad_response::{ }; use crate::mars::Environment; use crate::mars::ReportReason; +use crate::worker::DispatchCommand; use crate::AdsClientUrl; use crate::MozAdsClient; -use crate::worker::DispatchCommand; use parking_lot::Mutex; use std::collections::HashMap; @@ -56,7 +56,7 @@ impl From for Box { } } -#[derive(Default, Clone, Debug, uniffi::Record)] +#[derive(Default, Clone, Debug, uniffi::Record)] pub struct MozAdsRequestOptions { pub cache_policy: Option, #[uniffi(default)] @@ -141,14 +141,15 @@ impl MozAdsClientBuilder { }; let client = Arc::new(Mutex::new(AdsClient::new(client_config))); let client_clone = client.clone(); - - let (tx, rx)= mpsc::channel::(); - let worker_thread_handle = std::thread::spawn(move || crate::worker::worker(client_clone, rx)); + + let (tx, rx) = mpsc::channel::(); + let worker_thread_handle = + std::thread::spawn(move || crate::worker::worker(client_clone, rx)); MozAdsClient { inner: client, _worker_thread_handle: worker_thread_handle, - command_tx: tx + command_tx: tx, } } diff --git a/components/ads-client/src/lib.rs b/components/ads-client/src/lib.rs index fa0058cf72..dfc6265e4b 100644 --- a/components/ads-client/src/lib.rs +++ b/components/ads-client/src/lib.rs @@ -3,7 +3,11 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use std::{collections::HashMap, sync::{Arc, mpsc::Sender}, thread::JoinHandle}; +use std::{ + collections::HashMap, + sync::{mpsc::Sender, Arc}, + thread::JoinHandle, +}; use client::error::ComponentError; use error_support::handle_error; @@ -19,8 +23,8 @@ mod client; mod ffi; pub mod http_cache; mod mars; -pub mod worker; pub mod telemetry; +pub mod worker; pub use ffi::*; @@ -39,9 +43,8 @@ uniffi::custom_type!(AdsClientUrl, String, { #[derive(uniffi::Object)] pub struct MozAdsClient { inner: Arc>>, - _worker_thread_handle : JoinHandle<()>, - command_tx: Sender - + _worker_thread_handle: JoinHandle<()>, + command_tx: Sender, } pub type MozAdsClientInner = Arc>>; @@ -168,11 +171,10 @@ impl MozAdsClient { #[handle_error(ComponentError)] #[uniffi::method()] - pub fn dispatch( - &self, - command : DispatchCommand - ) -> AdsClientApiResult<()> { - self.command_tx.send(command).map_err(ComponentError::Dispatch)?; + pub fn dispatch(&self, command: DispatchCommand) -> AdsClientApiResult<()> { + self.command_tx + .send(command) + .map_err(ComponentError::Dispatch)?; Ok(()) } -} \ No newline at end of file +} diff --git a/components/ads-client/src/worker.rs b/components/ads-client/src/worker.rs index 9b81f4ce61..dd19faaf20 100644 --- a/components/ads-client/src/worker.rs +++ b/components/ads-client/src/worker.rs @@ -1,35 +1,169 @@ -use std::{collections::HashMap, sync::{Arc, mpsc::Receiver}}; use error_support::handle_error; +use std::{ + collections::HashMap, + sync::{mpsc::Receiver, Arc}, +}; +use url::Url as AdsClientUrl; -use crate::{AdsClientApiResult, MozAdsClientApiError, MozAdsClientInner, MozAdsPlacementRequest, MozAdsRequestOptions, MozAdsTile, client::error::ComponentError, http_cache::CachePolicy, mars::ad_request::{AdPlacementRequest, AdRequestFlags}}; +use crate::{ + client::error::ComponentError, + http_cache::CachePolicy, + mars::{ + ad_request::{AdPlacementRequest, AdRequestFlags}, + ad_response::AdSpoc, + error::CallbackRequestError, + }, + AdsClientApiResult, MozAdsCallbackOptions, MozAdsClientApiError, MozAdsClientInner, + MozAdsImage, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount, MozAdsReportReason, + MozAdsRequestOptions, MozAdsSpoc, MozAdsTile, +}; -pub fn worker(inner_client : MozAdsClientInner, rx : Receiver) { - // TODO: This could be an async environment where we wait on buffered futures- can send many out at once, instead of FIFO. +pub fn worker(inner_client: MozAdsClientInner, rx: Receiver) { while let Ok(task) = rx.recv() { - let task : DispatchCommand = task; - task.run_command(&inner_client).expect("TODO: handle error here. maybe retries should be here?"); + let task: DispatchCommand = task; + task.run_command(&inner_client) + .expect("TODO: handle error here. maybe retries should be here?"); } } #[derive(uniffi::Enum)] - pub enum DispatchCommand { + RecordClick { + click_url: String, + options: Option, + callback: Arc, + }, + RecordImpression { + impression_url: String, + options: Option, + callback: Arc, + }, + ReportAd { + report_url: String, + reason: MozAdsReportReason, + options: Option, + callback: Arc, + }, + RequestImageAds { + moz_ad_requests: Vec, + options: Option, + callback: Arc, + }, + RequestSpocAds { + moz_ad_requests: Vec, + options: Option, + callback: Arc, + }, RequestTileAd { moz_ad_requests: Vec, options: Option, callback: Arc, - } + }, } impl DispatchCommand { - #[handle_error(ComponentError)] - pub fn run_command(self, ads_client_inner : &MozAdsClientInner) -> AdsClientApiResult<()> { + pub fn run_command(self, ads_client_inner: &MozAdsClientInner) -> AdsClientApiResult<()> { // TODO: Duplicated behavior with the sync functions- either they call this or you call those match self { - DispatchCommand::RequestTileAd { moz_ad_requests, options, callback } => { + DispatchCommand::RecordClick { + click_url, + options, + callback, + } => { + let url = AdsClientUrl::parse(&click_url).map_err(|e| { + ComponentError::RecordClick(CallbackRequestError::InvalidUrl(e).into()) + })?; + let ohttp = options.map(|o| o.ohttp).unwrap_or(false); + let inner = ads_client_inner.lock(); + inner + .record_click(url, ohttp) + .map_err(ComponentError::RecordClick); + // TODO: error, modular version + } + DispatchCommand::RecordImpression { + impression_url, + options, + callback, + } => { + let url = AdsClientUrl::parse(&impression_url).map_err(|e| { + ComponentError::RecordImpression(CallbackRequestError::InvalidUrl(e).into()) + })?; + let ohttp = options.map(|o| o.ohttp).unwrap_or(false); + let inner = ads_client_inner.lock(); + inner + .record_impression(url, ohttp) + .map_err(ComponentError::RecordImpression); + // TODO: error, modular version + } + DispatchCommand::ReportAd { + report_url, + reason, + options, + callback, + } => { + let url = AdsClientUrl::parse(&report_url).map_err(|e| { + ComponentError::ReportAd(CallbackRequestError::InvalidUrl(e).into()) + })?; + let ohttp = options.map(|o| o.ohttp).unwrap_or(false); let inner = ads_client_inner.lock(); - let requests: Vec = moz_ad_requests.iter().map(|r| r.into()).collect(); + inner + .report_ad(url, reason.into(), ohttp) + .map_err(ComponentError::ReportAd); + // TODO: error, modular version + } + DispatchCommand::RequestImageAds { + moz_ad_requests, + options, + callback, + } => { + let inner = ads_client_inner.lock(); + let requests: Vec = + moz_ad_requests.iter().map(|r| r.into()).collect(); + let options = options.unwrap_or_default(); + let flags = AdRequestFlags::from(&options); + let ohttp = options.ohttp; + let cache_policy: CachePolicy = options.into(); + let response = inner + .request_image_ads(requests, flags, Some(cache_policy), ohttp) + .map_err(ComponentError::RequestAds)?; + let ads = response.into_iter().map(|(k, v)| (k, v.into())).collect(); + + callback.on_ad(ads); + // TODO: error, modular version + } + + DispatchCommand::RequestSpocAds { + moz_ad_requests, + options, + callback, + } => { + let inner = ads_client_inner.lock(); + let requests: Vec = + moz_ad_requests.iter().map(|r| r.into()).collect(); + let options = options.unwrap_or_default(); + let flags = AdRequestFlags::from(&options); + let ohttp = options.ohttp; + let cache_policy: CachePolicy = options.into(); + let response: HashMap> = inner + .request_spoc_ads(requests, flags, Some(cache_policy), ohttp) + .map_err(ComponentError::RequestAds)?; + let ads = response + .into_iter() + .map(|(k, v)| (k, v.into_iter().map(|spoc| spoc.into()).collect())) + .collect(); + callback.on_ad(ads); + // TODO: error, modular version + } + + DispatchCommand::RequestTileAd { + moz_ad_requests, + options, + callback, + } => { + let inner = ads_client_inner.lock(); + let requests: Vec = + moz_ad_requests.iter().map(|r| r.into()).collect(); let options = options.unwrap_or_default(); let flags = AdRequestFlags::from(&options); let ohttp = options.ohttp; @@ -46,8 +180,26 @@ impl DispatchCommand { } } +// TODO: Uniffi does not currently support generics here or direct function callbacks, so we use a few distinct interfaces. +#[uniffi::export(with_foreign)] +pub trait ErrorOnlyRequestCallback: Send + Sync { + fn on_error(&self, err: MozAdsClientApiError); +} + +#[uniffi::export(with_foreign)] +pub trait ImageRequestCallback: Send + Sync { + fn on_ad(&self, ads: HashMap); + fn on_error(&self, err: MozAdsClientApiError); +} + +#[uniffi::export(with_foreign)] +pub trait SpocRequestCallback: Send + Sync { + fn on_ad(&self, ads: HashMap>); + fn on_error(&self, err: MozAdsClientApiError); +} + #[uniffi::export(with_foreign)] -pub trait TileRequestCallback : Send + Sync { - fn on_ad(&self, tiles : HashMap); +pub trait TileRequestCallback: Send + Sync { + fn on_ad(&self, tiles: HashMap); fn on_error(&self, err: MozAdsClientApiError); } From 6ad7f5a39b22031ac1f4022bb87d70cfc346b116 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Mon, 27 Jul 2026 19:35:43 -0700 Subject: [PATCH 4/6] fix: refactor to allow sending errors --- components/ads-client/src/ffi/error.rs | 10 +- components/ads-client/src/worker.rs | 185 +++++++++++++++---------- 2 files changed, 117 insertions(+), 78 deletions(-) diff --git a/components/ads-client/src/ffi/error.rs b/components/ads-client/src/ffi/error.rs index e2da7471c2..cbe367ae2e 100644 --- a/components/ads-client/src/ffi/error.rs +++ b/components/ads-client/src/ffi/error.rs @@ -8,7 +8,7 @@ use error_support::{ErrorHandling, GetErrorHandling}; pub type AdsClientApiResult = std::result::Result; -#[derive(Debug, thiserror::Error, uniffi::Error)] +#[derive(Debug, Clone, thiserror::Error, uniffi::Error)] pub enum MozAdsClientApiError { #[error("Something unexpected occurred.")] Other { reason: String }, @@ -31,3 +31,11 @@ impl GetErrorHandling for ComponentError { }) } } + +impl From for MozAdsClientApiError { + fn from(value: ComponentError) -> Self { + MozAdsClientApiError::Other { + reason: value.to_string(), + } + } +} diff --git a/components/ads-client/src/worker.rs b/components/ads-client/src/worker.rs index dd19faaf20..21bba1a0d4 100644 --- a/components/ads-client/src/worker.rs +++ b/components/ads-client/src/worker.rs @@ -1,10 +1,3 @@ -use error_support::handle_error; -use std::{ - collections::HashMap, - sync::{mpsc::Receiver, Arc}, -}; -use url::Url as AdsClientUrl; - use crate::{ client::error::ComponentError, http_cache::CachePolicy, @@ -17,6 +10,12 @@ use crate::{ MozAdsImage, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount, MozAdsReportReason, MozAdsRequestOptions, MozAdsSpoc, MozAdsTile, }; +use error_support::handle_error; +use std::{ + collections::HashMap, + sync::{mpsc::Receiver, Arc}, +}; +use url::Url as AdsClientUrl; pub fn worker(inner_client: MozAdsClientInner, rx: Receiver) { while let Ok(task) = rx.recv() { @@ -71,30 +70,42 @@ impl DispatchCommand { options, callback, } => { - let url = AdsClientUrl::parse(&click_url).map_err(|e| { - ComponentError::RecordClick(CallbackRequestError::InvalidUrl(e).into()) - })?; - let ohttp = options.map(|o| o.ohttp).unwrap_or(false); - let inner = ads_client_inner.lock(); - inner - .record_click(url, ohttp) - .map_err(ComponentError::RecordClick); - // TODO: error, modular version + let resp = (|| { + let url = AdsClientUrl::parse(&click_url).map_err(|e| { + ComponentError::RecordClick(CallbackRequestError::InvalidUrl(e).into()) + })?; + let ohttp = options.map(|o| o.ohttp).unwrap_or(false); + let inner = ads_client_inner.lock(); + inner + .record_click(url, ohttp) + .map_err(ComponentError::RecordClick) + })(); + + match resp { + Ok(()) => (), + Err(e) => callback.on_error(e.into()), + } } DispatchCommand::RecordImpression { impression_url, options, callback, } => { - let url = AdsClientUrl::parse(&impression_url).map_err(|e| { - ComponentError::RecordImpression(CallbackRequestError::InvalidUrl(e).into()) - })?; - let ohttp = options.map(|o| o.ohttp).unwrap_or(false); - let inner = ads_client_inner.lock(); - inner - .record_impression(url, ohttp) - .map_err(ComponentError::RecordImpression); - // TODO: error, modular version + let resp = (|| { + let url = AdsClientUrl::parse(&impression_url).map_err(|e| { + ComponentError::RecordImpression(CallbackRequestError::InvalidUrl(e).into()) + })?; + let ohttp = options.map(|o| o.ohttp).unwrap_or(false); + let inner = ads_client_inner.lock(); + inner + .record_impression(url, ohttp) + .map_err(ComponentError::RecordImpression) + })(); + + match resp { + Ok(()) => (), + Err(e) => callback.on_error(e.into()), + } } DispatchCommand::ReportAd { report_url, @@ -102,35 +113,44 @@ impl DispatchCommand { options, callback, } => { - let url = AdsClientUrl::parse(&report_url).map_err(|e| { - ComponentError::ReportAd(CallbackRequestError::InvalidUrl(e).into()) - })?; - let ohttp = options.map(|o| o.ohttp).unwrap_or(false); - let inner = ads_client_inner.lock(); - inner - .report_ad(url, reason.into(), ohttp) - .map_err(ComponentError::ReportAd); - // TODO: error, modular version + let resp = (|| { + let url = AdsClientUrl::parse(&report_url).map_err(|e| { + ComponentError::ReportAd(CallbackRequestError::InvalidUrl(e).into()) + })?; + let ohttp = options.map(|o| o.ohttp).unwrap_or(false); + let inner = ads_client_inner.lock(); + inner + .report_ad(url, reason.into(), ohttp) + .map_err(ComponentError::ReportAd) + })(); + + match resp { + Ok(()) => (), + Err(e) => callback.on_error(e.into()), + } } DispatchCommand::RequestImageAds { moz_ad_requests, options, callback, } => { - let inner = ads_client_inner.lock(); - let requests: Vec = - moz_ad_requests.iter().map(|r| r.into()).collect(); - let options = options.unwrap_or_default(); - let flags = AdRequestFlags::from(&options); - let ohttp = options.ohttp; - let cache_policy: CachePolicy = options.into(); - let response = inner - .request_image_ads(requests, flags, Some(cache_policy), ohttp) - .map_err(ComponentError::RequestAds)?; - let ads = response.into_iter().map(|(k, v)| (k, v.into())).collect(); - - callback.on_ad(ads); - // TODO: error, modular version + let resp = (|| { + let inner = ads_client_inner.lock(); + let requests: Vec = + moz_ad_requests.iter().map(|r| r.into()).collect(); + let options = options.unwrap_or_default(); + let flags = AdRequestFlags::from(&options); + let ohttp = options.ohttp; + let cache_policy: CachePolicy = options.into(); + let response = inner + .request_image_ads(requests, flags, Some(cache_policy), ohttp) + .map_err(ComponentError::RequestAds)?; + Ok(response.into_iter().map(|(k, v)| (k, v.into())).collect()) + })(); + match resp { + Ok(ads) => callback.on_ad(ads), + Err(e) => callback.on_error(e), + } } DispatchCommand::RequestSpocAds { @@ -138,22 +158,28 @@ impl DispatchCommand { options, callback, } => { - let inner = ads_client_inner.lock(); - let requests: Vec = - moz_ad_requests.iter().map(|r| r.into()).collect(); - let options = options.unwrap_or_default(); - let flags = AdRequestFlags::from(&options); - let ohttp = options.ohttp; - let cache_policy: CachePolicy = options.into(); - let response: HashMap> = inner - .request_spoc_ads(requests, flags, Some(cache_policy), ohttp) - .map_err(ComponentError::RequestAds)?; - let ads = response - .into_iter() - .map(|(k, v)| (k, v.into_iter().map(|spoc| spoc.into()).collect())) - .collect(); - callback.on_ad(ads); - // TODO: error, modular version + let resp = (|| { + let inner = ads_client_inner.lock(); + let requests: Vec = + moz_ad_requests.iter().map(|r| r.into()).collect(); + let options = options.unwrap_or_default(); + let flags = AdRequestFlags::from(&options); + let ohttp = options.ohttp; + let cache_policy: CachePolicy = options.into(); + let response: HashMap> = inner + .request_spoc_ads(requests, flags, Some(cache_policy), ohttp) + .map_err(ComponentError::RequestAds)?; + Ok::<_, ComponentError>( + response + .into_iter() + .map(|(k, v)| (k, v.into_iter().map(|spoc| spoc.into()).collect())) + .collect(), + ) + })(); + match resp { + Ok(ads) => callback.on_ad(ads), + Err(e) => callback.on_error(e.into()), + } } DispatchCommand::RequestTileAd { @@ -161,23 +187,28 @@ impl DispatchCommand { options, callback, } => { - let inner = ads_client_inner.lock(); - let requests: Vec = - moz_ad_requests.iter().map(|r| r.into()).collect(); - let options = options.unwrap_or_default(); - let flags = AdRequestFlags::from(&options); - let ohttp = options.ohttp; - let cache_policy: CachePolicy = options.into(); - let response = inner - .request_tile_ads(requests, flags, Some(cache_policy), ohttp) - .map_err(ComponentError::RequestAds)?; - let tiles = response.into_iter().map(|(k, v)| (k, v.into())).collect(); - callback.on_ad(tiles); - // TODO: error, modular version + let resp = (|| { + let inner = ads_client_inner.lock(); + let requests: Vec = + moz_ad_requests.iter().map(|r| r.into()).collect(); + let options = options.unwrap_or_default(); + let flags = AdRequestFlags::from(&options); + let ohttp = options.ohttp; + let cache_policy: CachePolicy = options.into(); + let response = inner + .request_tile_ads(requests, flags, Some(cache_policy), ohttp) + .map_err(ComponentError::RequestAds)?; + Ok(response.into_iter().map(|(k, v)| (k, v.into())).collect()) + })(); + match resp { + Ok(tiles) => callback.on_ad(tiles), + Err(e) => callback.on_error(e), + } } } Ok(()) } + } // TODO: Uniffi does not currently support generics here or direct function callbacks, so we use a few distinct interfaces. From dc8b8f61393108684077b0e43649b84752718c83 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Tue, 28 Jul 2026 17:33:50 -0700 Subject: [PATCH 5/6] feat: Revamped to use Box --- .../integration-tests/tests/mars_async.rs | 227 +++++++++++++++++- components/ads-client/src/client/error.rs | 4 +- components/ads-client/src/ffi.rs | 11 +- components/ads-client/src/lib.rs | 115 ++++++++- components/ads-client/src/worker.rs | 40 +-- 5 files changed, 357 insertions(+), 40 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars_async.rs b/components/ads-client/integration-tests/tests/mars_async.rs index 40606ebd9e..430ec8b003 100644 --- a/components/ads-client/integration-tests/tests/mars_async.rs +++ b/components/ads-client/integration-tests/tests/mars_async.rs @@ -8,9 +8,9 @@ use ads_client::{ DispatchCommand, ErrorOnlyRequestCallback, ImageRequestCallback, SpocRequestCallback, TileRequestCallback, }, - MozAdsClientApiError, MozAdsClientBuilder, MozAdsEnvironment, MozAdsIABContent, + MozAdsClient, MozAdsClientApiError, MozAdsClientBuilder, MozAdsEnvironment, MozAdsIABContent, MozAdsIABContentTaxonomy, MozAdsImage, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount, - MozAdsRequestOptions, MozAdsSpoc, MozAdsTile, + MozAdsReportReason, MozAdsRequestOptions, MozAdsSpoc, MozAdsTile, }; use std::{ collections::HashMap, @@ -55,6 +55,9 @@ impl CallbackTestStruct { } impl ErrorOnlyRequestCallback for CallbackTestStruct<(), MozAdsClientApiError> { + fn on_success(&self) { + (self.on_ad_fn)((), self.success_tx.clone()) + } fn on_error(&self, err: MozAdsClientApiError) { (self.on_error_fn)(err, self.err_tx.clone()) } @@ -91,9 +94,45 @@ impl TileRequestCallback for CallbackTestStruct, Moz } } +// Helper function that runs the tile contract test and produces the result (for tests that require this in setup) +// Callback setup in rust can be weighty for tests, so reuse makes a bit more concise +// This should match the logic of `test_contract_tile_prod_async` +fn generate_ad_test(client: &MozAdsClient) -> HashMap { + // Create ad + let (success_tx, success_rx) = mpsc::channel(); + let (err_tx, err_rx) = mpsc::channel(); + let callback = CallbackTestStruct::, MozAdsClientApiError>::new( + success_tx, err_tx, + ); + + client + .dispatch(DispatchCommand::RequestTileAd { + moz_ad_requests: vec![MozAdsPlacementRequest { + placement_id: "mock_tile_1".to_string(), + iab_content: None, + }], + options: None, + callback: Box::new(callback), + }) + .expect("Asynchronous dispatch should return Ok()"); + + let placements; + loop { + if let Ok(placements_res) = success_rx.recv() { + placements = placements_res; + break; + } + if let Ok(err) = err_rx.recv() { + panic!("Tile ad request failed: {:?}", err); + } + } + + placements +} + #[test] #[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_image_prod_async() { +fn test_contract_image_prod_callback() { init_backend(); let client = prod_client(); @@ -110,7 +149,7 @@ fn test_contract_image_prod_async() { placement_id: "mock_billboard_1".to_string(), }], options: None, - callback: Arc::new(callback), + callback: Box::new(callback), }) .expect("Asynchronous dispatch should return Ok()"); @@ -127,7 +166,7 @@ fn test_contract_image_prod_async() { #[test] #[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_image_with_categories_prod_async() { +fn test_contract_image_with_categories_prod_callback() { init_backend(); let client = prod_client(); @@ -153,7 +192,7 @@ fn test_contract_image_with_categories_prod_async() { )]), ..Default::default() }), - callback: Arc::new(callback), + callback: Box::new(callback), }) .expect("Asynchronous dispatch should return Ok()"); @@ -170,7 +209,7 @@ fn test_contract_image_with_categories_prod_async() { #[test] #[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_spoc_prod_async() { +fn test_contract_spoc_prod_callback() { init_backend(); let client = prod_client(); @@ -189,7 +228,7 @@ fn test_contract_spoc_prod_async() { placement_id: "mock_spoc_1".to_string(), }], options: None, - callback: Arc::new(callback), + callback: Box::new(callback), }) .expect("Asynchronous dispatch should return Ok()"); @@ -208,7 +247,7 @@ fn test_contract_spoc_prod_async() { #[test] #[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_tile_prod_async() { +fn test_contract_tile_prod_callback() { init_backend(); let client = prod_client(); @@ -225,7 +264,7 @@ fn test_contract_tile_prod_async() { placement_id: "mock_tile_1".to_string(), }], options: None, - callback: Arc::new(callback), + callback: Box::new(callback), }) .expect("Asynchronous dispatch should return Ok()"); @@ -239,3 +278,171 @@ fn test_contract_tile_prod_async() { } } } + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_record_impression_callback() { + init_backend(); + let client = prod_client(); + + // Generate tiles + let placements = generate_ad_test(&client); + let ad = placements + .get("mock_tile_1") + .clone() + .expect("mock_tile_1 placement should be present"); + + // Record an impression + let (success_tx, success_rx) = mpsc::channel(); + let (err_tx, err_rx) = mpsc::channel(); + let callback = CallbackTestStruct::<(), MozAdsClientApiError>::new(success_tx, err_tx); + + client + .dispatch(DispatchCommand::RecordImpression { + impression_url: ad.callbacks.impression.to_string(), + options: None, + callback: Box::new(callback), + }) + .expect("Asynchronous dispatch should return Ok()"); + + loop { + if let Ok(_) = success_rx.recv() { + break; + } + if let Ok(err) = err_rx.recv() { + panic!("record_impression failed: {:?}", err); + } + } +} + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_record_click_callback() { + init_backend(); + let client = prod_client(); + + // Generate tiles + let placements = generate_ad_test(&client); + let ad = placements + .get("mock_tile_1") + .clone() + .expect("mock_tile_1 placement should be present"); + + // Record a click + let (success_tx, success_rx) = mpsc::channel(); + let (err_tx, err_rx) = mpsc::channel(); + let callback = CallbackTestStruct::<(), MozAdsClientApiError>::new(success_tx, err_tx); + + client + .dispatch(DispatchCommand::RecordClick { + click_url: ad.callbacks.click.to_string(), + options: None, + callback: Box::new(callback), + }) + .expect("Asynchronous dispatch should return Ok()"); + + loop { + if let Ok(_) = success_rx.recv() { + break; + } + if let Ok(err) = err_rx.recv() { + panic!("record_click failed: {:?}", err); + } + } +} + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_report_ad_callback() { + init_backend(); + let client = prod_client(); + + // Generate tiles + let placements = generate_ad_test(&client); + let ad = placements + .get("mock_tile_1") + .clone() + .expect("mock_tile_1 placement should be present"); + + // Assertions on the ad + let report_url = ad + .callbacks + .report + .as_ref() + .expect("mock_tile_1 should have a report URL"); + let pairs: Vec<(_, _)> = report_url.query_pairs().collect(); + let placement_id_count = pairs.iter().filter(|(k, _)| k == "placement_id").count(); + let position_count = pairs.iter().filter(|(k, _)| k == "position").count(); + assert_eq!(placement_id_count, 1, "expected exactly one placement_id"); + assert_eq!(position_count, 1, "expected exactly one position"); + + // Report the ad + let (success_tx, success_rx) = mpsc::channel(); + let (err_tx, err_rx) = mpsc::channel(); + let callback = CallbackTestStruct::<(), MozAdsClientApiError>::new(success_tx, err_tx); + + client + .dispatch(DispatchCommand::ReportAd { + report_url: report_url.to_string(), + reason: MozAdsReportReason::NotInterested, + options: None, + callback: Box::new(callback), + }) + .expect("Asynchronous dispatch should return Ok()"); + + loop { + if let Ok(_) = success_rx.recv() { + break; + } + if let Ok(err) = err_rx.recv() { + panic!("report_ad failed: {:?}", err); + } + } +} + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_contract_tile_ohttp_prod_callback() { + init_backend(); + viaduct::ohttp::configure_ohttp_channel( + "ads-client".to_string(), + viaduct::ohttp::OhttpConfig { + relay_url: "https://mozilla-ohttp.fastly-edge.com/".to_string(), + gateway_host: "prod.ohttp-gateway.prod.webservices.mozgcp.net".to_string(), + }, + ) + .expect("OHTTP channel configuration should succeed"); + + let client = prod_client(); + + let (success_tx, success_rx) = mpsc::channel(); + let (err_tx, err_rx) = mpsc::channel(); + let callback = CallbackTestStruct::, MozAdsClientApiError>::new( + success_tx, err_tx, + ); + + client + .dispatch(DispatchCommand::RequestTileAd { + moz_ad_requests: vec![MozAdsPlacementRequest { + placement_id: "mock_tile_1".to_string(), + iab_content: None, + }], + options: None, + callback: Box::new(callback), + }) + .expect("Asynchronous dispatch should return Ok()"); + + loop { + if let Ok(placements) = success_rx.recv() { + assert!( + placements.contains_key("mock_tile_1"), + "OHTTP response should contain mock_tile_1" + ); + + break; + } + if let Ok(err) = err_rx.recv() { + panic!("Tile ad request over OHTTP should succeed: {:?}", err); + } + } +} diff --git a/components/ads-client/src/client/error.rs b/components/ads-client/src/client/error.rs index 25823ae31b..372ff9436b 100644 --- a/components/ads-client/src/client/error.rs +++ b/components/ads-client/src/client/error.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use std::sync::mpsc::SendError; +use std::sync::mpsc::TrySendError; use crate::{ mars::error::{FetchAdsError, RecordClickError, RecordImpressionError, ReportAdError}, @@ -25,7 +25,7 @@ pub enum ComponentError { RequestAds(#[from] RequestAdsError), #[error("Error dispatching an asynchronous command: {0}")] - Dispatch(SendError), + Dispatch(TrySendError), } #[derive(Debug, thiserror::Error)] diff --git a/components/ads-client/src/ffi.rs b/components/ads-client/src/ffi.rs index a4fcd3d206..5fddb239c4 100644 --- a/components/ads-client/src/ffi.rs +++ b/components/ads-client/src/ffi.rs @@ -20,7 +20,7 @@ use crate::mars::ad_response::{ }; use crate::mars::Environment; use crate::mars::ReportReason; -use crate::worker::DispatchCommand; +use crate::worker::{DispatchCommand, ADS_CLIENT_WORKER_CHANNEL_BUFFER_SIZE}; use crate::AdsClientUrl; use crate::MozAdsClient; use parking_lot::Mutex; @@ -142,9 +142,12 @@ impl MozAdsClientBuilder { let client = Arc::new(Mutex::new(AdsClient::new(client_config))); let client_clone = client.clone(); - let (tx, rx) = mpsc::channel::(); - let worker_thread_handle = - std::thread::spawn(move || crate::worker::worker(client_clone, rx)); + let (tx, rx) = mpsc::sync_channel::(ADS_CLIENT_WORKER_CHANNEL_BUFFER_SIZE); + + let worker_thread_handle = std::thread::Builder::new() + .name("ads-client.worker".to_string()) + .spawn(move || crate::worker::worker(client_clone, rx)) + .expect("Failed to create an OS thread."); // TODO: Maybe this sets it to None and no background thread is used? MozAdsClient { inner: client, diff --git a/components/ads-client/src/lib.rs b/components/ads-client/src/lib.rs index dfc6265e4b..7c164a6566 100644 --- a/components/ads-client/src/lib.rs +++ b/components/ads-client/src/lib.rs @@ -5,7 +5,10 @@ use std::{ collections::HashMap, - sync::{mpsc::Sender, Arc}, + sync::{ + mpsc::SyncSender, + Arc, + }, thread::JoinHandle, }; @@ -28,7 +31,13 @@ pub mod worker; pub use ffi::*; -use crate::{ffi::telemetry::MozAdsTelemetryWrapper, worker::DispatchCommand}; +use crate::{ + ffi::telemetry::MozAdsTelemetryWrapper, + worker::{ + DispatchCommand, ErrorOnlyRequestCallback, ImageRequestCallback, SpocRequestCallback, + TileRequestCallback, + }, +}; #[cfg(test)] mod test_utils; @@ -44,7 +53,7 @@ uniffi::custom_type!(AdsClientUrl, String, { pub struct MozAdsClient { inner: Arc>>, _worker_thread_handle: JoinHandle<()>, - command_tx: Sender, + command_tx: SyncSender, } pub type MozAdsClientInner = Arc>>; @@ -169,11 +178,107 @@ impl MozAdsClient { Ok(response.into_iter().map(|(k, v)| (k, v.into())).collect()) } + // TODO: Remove the following functions when uniffi = 0.32 lands, and we can properly expose the available enum directly with `dispatch(command)`. #[handle_error(ComponentError)] #[uniffi::method()] - pub fn dispatch(&self, command: DispatchCommand) -> AdsClientApiResult<()> { + pub fn dispatch_record_click( + &self, + click_url: String, + options: Option, + callback: Box, + ) -> AdsClientApiResult<()> { + self.dispatch(DispatchCommand::RecordClick { + click_url, + options, + callback, + }) + } + + #[handle_error(ComponentError)] + #[uniffi::method(default(options = None))] + pub fn dispatch_record_impression( + &self, + impression_url: String, + options: Option, + callback: Box, + ) -> AdsClientApiResult<()> { + self.dispatch(DispatchCommand::RecordImpression { + impression_url, + options, + callback, + }) + } + + #[handle_error(ComponentError)] + #[uniffi::method(default(options = None))] + pub fn dispatch_report_ad( + &self, + report_url: String, + reason: MozAdsReportReason, + options: Option, + callback: Box, + ) -> AdsClientApiResult<()> { + self.dispatch(DispatchCommand::ReportAd { + report_url, + reason, + options, + callback, + }) + } + + #[handle_error(ComponentError)] + #[uniffi::method(default(options = None))] + pub fn dispatch_request_image_ads( + &self, + moz_ad_requests: Vec, + options: Option, + callback: Box, + ) -> AdsClientApiResult<()> { + self.dispatch(DispatchCommand::RequestImageAds { + moz_ad_requests, + options, + callback, + }) + } + + #[handle_error(ComponentError)] + #[uniffi::method(default(options = None))] + pub fn dispatch_request_spoc_ads( + &self, + moz_ad_requests: Vec, + options: Option, + callback: Box, + ) -> AdsClientApiResult<()> { + self.dispatch(DispatchCommand::RequestSpocAds { + moz_ad_requests, + options, + callback, + }) + } + + #[handle_error(ComponentError)] + #[uniffi::method(default(options = None))] + pub fn dispatch_request_tile_ads( + &self, + moz_ad_requests: Vec, + options: Option, + callback: Box, + ) -> AdsClientApiResult<()> { + self.dispatch(DispatchCommand::RequestTileAd { + moz_ad_requests, + options, + callback, + }) + } +} + +impl MozAdsClient { + // TODO: The following + pub fn dispatch(&self, command: DispatchCommand) -> Result<(), ComponentError> { + // try_send provides an error on a disconnection or a SyncChannel full buffer + // TODO: drop oldest on error, not newest self.command_tx - .send(command) + .try_send(command) .map_err(ComponentError::Dispatch)?; Ok(()) } diff --git a/components/ads-client/src/worker.rs b/components/ads-client/src/worker.rs index 21bba1a0d4..730bcf1193 100644 --- a/components/ads-client/src/worker.rs +++ b/components/ads-client/src/worker.rs @@ -11,12 +11,11 @@ use crate::{ MozAdsRequestOptions, MozAdsSpoc, MozAdsTile, }; use error_support::handle_error; -use std::{ - collections::HashMap, - sync::{mpsc::Receiver, Arc}, -}; +use std::{collections::HashMap, sync::mpsc::Receiver}; use url::Url as AdsClientUrl; +pub const ADS_CLIENT_WORKER_CHANNEL_BUFFER_SIZE: usize = 1000; + pub fn worker(inner_client: MozAdsClientInner, rx: Receiver) { while let Ok(task) = rx.recv() { let task: DispatchCommand = task; @@ -25,38 +24,37 @@ pub fn worker(inner_client: MozAdsClientInner, rx: Receiver) { } } -#[derive(uniffi::Enum)] pub enum DispatchCommand { RecordClick { click_url: String, options: Option, - callback: Arc, + callback: Box, }, RecordImpression { impression_url: String, options: Option, - callback: Arc, + callback: Box, }, ReportAd { report_url: String, reason: MozAdsReportReason, options: Option, - callback: Arc, + callback: Box, }, RequestImageAds { moz_ad_requests: Vec, options: Option, - callback: Arc, + callback: Box, }, RequestSpocAds { moz_ad_requests: Vec, options: Option, - callback: Arc, + callback: Box, }, RequestTileAd { moz_ad_requests: Vec, options: Option, - callback: Arc, + callback: Box, }, } @@ -82,7 +80,7 @@ impl DispatchCommand { })(); match resp { - Ok(()) => (), + Ok(_) => callback.on_success(), Err(e) => callback.on_error(e.into()), } } @@ -103,7 +101,7 @@ impl DispatchCommand { })(); match resp { - Ok(()) => (), + Ok(_) => callback.on_success(), Err(e) => callback.on_error(e.into()), } } @@ -125,7 +123,7 @@ impl DispatchCommand { })(); match resp { - Ok(()) => (), + Ok(_) => callback.on_success(), Err(e) => callback.on_error(e.into()), } } @@ -208,28 +206,32 @@ impl DispatchCommand { } Ok(()) } - } // TODO: Uniffi does not currently support generics here or direct function callbacks, so we use a few distinct interfaces. -#[uniffi::export(with_foreign)] +// TODO: We use #[uniffi::export(callback_interface)] rather than #[uniffi::export(with_foreign)] for reasons discussed here: +// - https://github.com/mozilla/application-services/pull/7443 +// In the future, this should be changed to uniffi::export(impl = "foreign") alongside the enum change, when uniffi = 0.32 +#[uniffi::export(callback_interface)] pub trait ErrorOnlyRequestCallback: Send + Sync { + // TODO: We don't need this, but is useful for tests + fn on_success(&self); fn on_error(&self, err: MozAdsClientApiError); } -#[uniffi::export(with_foreign)] +#[uniffi::export(callback_interface)] pub trait ImageRequestCallback: Send + Sync { fn on_ad(&self, ads: HashMap); fn on_error(&self, err: MozAdsClientApiError); } -#[uniffi::export(with_foreign)] +#[uniffi::export(callback_interface)] pub trait SpocRequestCallback: Send + Sync { fn on_ad(&self, ads: HashMap>); fn on_error(&self, err: MozAdsClientApiError); } -#[uniffi::export(with_foreign)] +#[uniffi::export(callback_interface)] pub trait TileRequestCallback: Send + Sync { fn on_ad(&self, tiles: HashMap); fn on_error(&self, err: MozAdsClientApiError); From 54baa07d4ecf9206de33445df8d6eb7910a97788 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Tue, 28 Jul 2026 18:17:07 -0700 Subject: [PATCH 6/6] feat: Resolves some todos --- .../integration-tests/tests/mars_async.rs | 3 ++- components/ads-client/src/client/error.rs | 1 - components/ads-client/src/ffi.rs | 9 +++----- components/ads-client/src/lib.rs | 8 ++++--- components/ads-client/src/worker.rs | 21 +++++++++++++++---- 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars_async.rs b/components/ads-client/integration-tests/tests/mars_async.rs index 430ec8b003..20ad8e1bdf 100644 --- a/components/ads-client/integration-tests/tests/mars_async.rs +++ b/components/ads-client/integration-tests/tests/mars_async.rs @@ -30,7 +30,8 @@ fn prod_client() -> ads_client::MozAdsClient { .build() } -// TODO: explain +// Reusable test structure that implements the varying callback traits with generics. +// We can use a generic here, unlike in MAC, because it's not going through uniffi. pub struct CallbackTestStruct { on_ad_fn: Box) + Send + Sync>, on_error_fn: Box) + Send + Sync>, diff --git a/components/ads-client/src/client/error.rs b/components/ads-client/src/client/error.rs index 372ff9436b..4d0181f33b 100644 --- a/components/ads-client/src/client/error.rs +++ b/components/ads-client/src/client/error.rs @@ -4,7 +4,6 @@ */ use std::sync::mpsc::TrySendError; - use crate::{ mars::error::{FetchAdsError, RecordClickError, RecordImpressionError, ReportAdError}, worker::DispatchCommand, diff --git a/components/ads-client/src/ffi.rs b/components/ads-client/src/ffi.rs index 5fddb239c4..eb2eac18b4 100644 --- a/components/ads-client/src/ffi.rs +++ b/components/ads-client/src/ffi.rs @@ -20,7 +20,7 @@ use crate::mars::ad_response::{ }; use crate::mars::Environment; use crate::mars::ReportReason; -use crate::worker::{DispatchCommand, ADS_CLIENT_WORKER_CHANNEL_BUFFER_SIZE}; +use crate::worker::{ADS_CLIENT_WORKER_CHANNEL_BUFFER_SIZE, DispatchCommand, worker_thread}; use crate::AdsClientUrl; use crate::MozAdsClient; use parking_lot::Mutex; @@ -144,11 +144,8 @@ impl MozAdsClientBuilder { let (tx, rx) = mpsc::sync_channel::(ADS_CLIENT_WORKER_CHANNEL_BUFFER_SIZE); - let worker_thread_handle = std::thread::Builder::new() - .name("ads-client.worker".to_string()) - .spawn(move || crate::worker::worker(client_clone, rx)) - .expect("Failed to create an OS thread."); // TODO: Maybe this sets it to None and no background thread is used? - + let worker_thread_handle = worker_thread(client_clone, rx); + MozAdsClient { inner: client, _worker_thread_handle: worker_thread_handle, diff --git a/components/ads-client/src/lib.rs b/components/ads-client/src/lib.rs index 7c164a6566..90b5e431cb 100644 --- a/components/ads-client/src/lib.rs +++ b/components/ads-client/src/lib.rs @@ -52,7 +52,7 @@ uniffi::custom_type!(AdsClientUrl, String, { #[derive(uniffi::Object)] pub struct MozAdsClient { inner: Arc>>, - _worker_thread_handle: JoinHandle<()>, + _worker_thread_handle: Option>, command_tx: SyncSender, } pub type MozAdsClientInner = Arc>>; @@ -178,7 +178,6 @@ impl MozAdsClient { Ok(response.into_iter().map(|(k, v)| (k, v.into())).collect()) } - // TODO: Remove the following functions when uniffi = 0.32 lands, and we can properly expose the available enum directly with `dispatch(command)`. #[handle_error(ComponentError)] #[uniffi::method()] pub fn dispatch_record_click( @@ -273,10 +272,13 @@ impl MozAdsClient { } impl MozAdsClient { - // TODO: The following + // TODO: The following is the intended surface for dispatching commands with callbacks, but passing an enum with uniffi does not work yet in 0.31. + // When upgraded to 0.32, we can change the myriad #[uniffi::export(callback_interface)] usage to #[uniffi::export(impl = "foreign")], + // and attempt to expose this enum for direct use. pub fn dispatch(&self, command: DispatchCommand) -> Result<(), ComponentError> { // try_send provides an error on a disconnection or a SyncChannel full buffer // TODO: drop oldest on error, not newest + // TODO: This currently simply drops messages if the thread is killed for whatever reason- should we fall through to sync? self.command_tx .try_send(command) .map_err(ComponentError::Dispatch)?; diff --git a/components/ads-client/src/worker.rs b/components/ads-client/src/worker.rs index 730bcf1193..8db4c5ce0e 100644 --- a/components/ads-client/src/worker.rs +++ b/components/ads-client/src/worker.rs @@ -11,13 +11,27 @@ use crate::{ MozAdsRequestOptions, MozAdsSpoc, MozAdsTile, }; use error_support::handle_error; -use std::{collections::HashMap, sync::mpsc::Receiver}; +use std::{collections::HashMap, sync::mpsc::Receiver, thread::JoinHandle}; use url::Url as AdsClientUrl; pub const ADS_CLIENT_WORKER_CHANNEL_BUFFER_SIZE: usize = 1000; +pub const ADS_CLIENT_WORKER_THREAD_NAME : &'static str = "ads-client.worker"; -pub fn worker(inner_client: MozAdsClientInner, rx: Receiver) { +// Spawn worker thread with reference to client and dispatch command receiver. +// Returns None if thread fails to build. +pub fn worker_thread(inner_client: MozAdsClientInner, rx: Receiver) -> Option> { + let worker_thread_handle = std::thread::Builder::new() + .name(ADS_CLIENT_WORKER_THREAD_NAME.to_string()) + .spawn(move || crate::worker::worker(inner_client, rx)).inspect_err(|err| { + error_support::error!("Failed to create ads-client worker thread `{ADS_CLIENT_WORKER_THREAD_NAME}` with: {err}") + }).ok()?; + Some(worker_thread_handle) +} + +fn worker(inner_client: MozAdsClientInner, rx: Receiver) { while let Ok(task) = rx.recv() { + + // Synchronously run tasks in the order they are passed in this separate channel. let task: DispatchCommand = task; task.run_command(&inner_client) .expect("TODO: handle error here. maybe retries should be here?"); @@ -61,7 +75,7 @@ pub enum DispatchCommand { impl DispatchCommand { #[handle_error(ComponentError)] pub fn run_command(self, ads_client_inner: &MozAdsClientInner) -> AdsClientApiResult<()> { - // TODO: Duplicated behavior with the sync functions- either they call this or you call those + // TODO: Duplicated behavior with the sync functions. Behavior is pretty simple though- probably OK to duplicate. match self { DispatchCommand::RecordClick { click_url, @@ -214,7 +228,6 @@ impl DispatchCommand { // In the future, this should be changed to uniffi::export(impl = "foreign") alongside the enum change, when uniffi = 0.32 #[uniffi::export(callback_interface)] pub trait ErrorOnlyRequestCallback: Send + Sync { - // TODO: We don't need this, but is useful for tests fn on_success(&self); fn on_error(&self, err: MozAdsClientApiError); }