diff --git a/crates/bitwarden-uniffi/src/error.rs b/crates/bitwarden-uniffi/src/error.rs index 23fe3835d..4d92c3c62 100644 --- a/crates/bitwarden-uniffi/src/error.rs +++ b/crates/bitwarden-uniffi/src/error.rs @@ -93,7 +93,16 @@ pub enum BitwardenError { SshGeneration(#[from] bitwarden_ssh::error::KeyGenerationError), #[error(transparent)] SshImport(#[from] bitwarden_ssh::error::SshKeyImportError), + #[error("Callback invocation failed")] + CallbackError, #[error("A conversion error occurred: {0}")] Conversion(String), } +/// Required From implementation for UNIFFI callback error handling +/// Converts unexpected mobile exceptions into BitwardenError +impl From for BitwardenError { + fn from(_: uniffi::UnexpectedUniFFICallbackError) -> Self { + Self::CallbackError + } +} diff --git a/crates/bitwarden-uniffi/src/lib.rs b/crates/bitwarden-uniffi/src/lib.rs index f16325763..6098d490f 100644 --- a/crates/bitwarden-uniffi/src/lib.rs +++ b/crates/bitwarden-uniffi/src/lib.rs @@ -11,7 +11,8 @@ use bitwarden_core::{ClientSettings, client::internal::ClientManagedTokens}; pub mod auth; #[allow(missing_docs)] pub mod crypto; -mod error; +pub mod error; +mod log_callback; #[allow(missing_docs)] pub mod platform; #[allow(missing_docs)] @@ -24,6 +25,7 @@ pub mod vault; mod android_support; use crypto::CryptoClient; +pub use log_callback::LogCallback; use error::{Error, Result}; use platform::PlatformClient; use tool::{ExporterClient, GeneratorClients, SendClient, SshClient}; @@ -40,8 +42,9 @@ impl Client { pub fn new( token_provider: Arc, settings: Option, + log_callback: Option>, ) -> Self { - init_logger(); + init_logger(log_callback); setup_error_converter(); #[cfg(target_os = "android")] @@ -113,7 +116,7 @@ impl Client { static INIT: Once = Once::new(); -fn init_logger() { +fn init_logger(callback: Option>) { use tracing_subscriber::{EnvFilter, layer::SubscriberExt as _, util::SubscriberInitExt as _}; INIT.call_once(|| { @@ -137,13 +140,21 @@ fn init_logger() { .with_target(true) .pretty(); + // Build base registry once instead of duplicating per-platform + let registry = tracing_subscriber::registry() + .with(fmtlayer) + .with(filter); + + // Conditionally add callback layer if provided + // Use Option to avoid type incompatibility between Some/None branches + let callback_layer = callback.map(log_callback::CallbackLayer::new); + let registry = registry.with(callback_layer); + + // Platform-specific layers now compose with base registry #[cfg(target_os = "ios")] { const TAG: &str = "com.8bit.bitwarden"; - - tracing_subscriber::registry() - .with(fmtlayer) - .with(filter) + registry .with(tracing_oslog::OsLogger::new(TAG, "default")) .init(); } @@ -151,10 +162,7 @@ fn init_logger() { #[cfg(target_os = "android")] { const TAG: &str = "com.bitwarden.sdk"; - - tracing_subscriber::registry() - .with(fmtlayer) - .with(filter) + registry .with( tracing_android::layer(TAG) .expect("initialization of android logcat tracing layer"), @@ -164,10 +172,7 @@ fn init_logger() { #[cfg(not(any(target_os = "android", target_os = "ios")))] { - tracing_subscriber::registry() - .with(fmtlayer) - .with(filter) - .init(); + registry.init(); } }); } @@ -179,3 +184,56 @@ fn setup_error_converter() { crate::error::BitwardenError::Conversion(e.to_string()).into() }); } +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex; + // Mock token provider for testing + #[derive(Debug)] + struct MockTokenProvider; + + #[async_trait::async_trait] + impl ClientManagedTokens for MockTokenProvider { + async fn get_access_token(&self) -> Option { + Some("mock_token".to_string()) + } + } + /// Mock LogCallback implementation for testing + struct TestLogCallback { + logs: Arc>>, + } + impl LogCallback for TestLogCallback { + fn on_log(&self, level: String, target: String, message: String) + -> Result<()> + { + self.logs.lock().unwrap().push((level, target, message)); + Ok(()) + } + } + + // Log callback unit tests only test happy path because running this with + // Once means we get one registered callback per test run. There are + // other tests written as integration tests in the /tests/ folder that + // assert more specific details. + #[test] + fn test_callback_receives_logs() { + let logs = Arc::new(Mutex::new(Vec::new())); + let callback = Arc::new(TestLogCallback { logs: logs.clone() }); + + // Create client with callback + let _client = Client::new( + Arc::new(MockTokenProvider), + None, + Some(callback), + ); + + // Trigger a log + tracing::info!("test message from SDK"); + + // Verify callback received it + let captured = logs.lock().unwrap(); + assert!(!captured.is_empty(), "Callback should receive logs"); + assert_eq!(captured[0].0, "INFO"); + assert!(captured[0].2.contains("test message")); + } +} diff --git a/crates/bitwarden-uniffi/src/log_callback.rs b/crates/bitwarden-uniffi/src/log_callback.rs new file mode 100644 index 000000000..40a67b881 --- /dev/null +++ b/crates/bitwarden-uniffi/src/log_callback.rs @@ -0,0 +1,107 @@ +use std::sync::Arc; +use tracing_subscriber::{layer::Context, Layer}; +/// Callback interface for receiving SDK log events +/// Mobile implementations forward these to Flight Recorder +#[uniffi::export(with_foreign)] +pub trait LogCallback: Send + Sync { + /// Called when SDK emits a log entry + /// + /// # Parameters + /// - level: Log level ("TRACE", "DEBUG", "INFO", "WARN", "ERROR") + /// - target: Module that emitted log (e.g., "bitwarden_core::auth") + /// - message: The log message text + /// + /// # Returns + /// Result<(), BitwardenError> - mobile implementations should catch exceptions + /// and return errors rather than panicking + fn on_log(&self, level: String, target: String, message: String) -> crate::Result<()>; +} + +/// Custom tracing Layer that forwards events to UNIFFI callback +pub(crate) struct CallbackLayer { + callback: Arc, +} +impl CallbackLayer { + pub(crate) fn new(callback: Arc) -> Self { + Self { callback } + } +} +impl Layer for CallbackLayer +where + S: tracing::Subscriber, +{ + fn on_event(&self, event: &tracing::Event<'_>, _ctx: Context<'_, S>) { + let metadata = event.metadata(); + // CRITICAL: Filter out our own error messages to prevent infinite callback loop + if metadata.target() == "bitwarden_uniffi::log_callback" { + return; // Platform loggers still receive this for debugging + } + let level = metadata.level().to_string(); + let target = metadata.target().to_string(); + // Format event message + let mut visitor = MessageVisitor::default(); + event.record(&mut visitor); + let message = visitor.message; + // Forward to UNIFFI callback with error handling + if let Err(e) = self.callback.on_log(level, target, message) { + // Error logged with explicit target for filtering above + // Platform loggers receive this for mobile team debugging + tracing::error!(target: "bitwarden_uniffi::log_callback", "Logging callback failed: {:?}", e); + } + } +} +/// Visitor to extract message from tracing event +/// +/// **Why only record_debug is implemented:** +/// +/// MessageVisitor implements only record_debug because it's the ONLY required method. +/// The tracing::field::Visit trait provides default implementations for all other +/// record methods (record_str, record_i64, record_u64, record_bool, etc.) that +/// forward to record_debug. The SDK uses % and ? format specifiers which route +/// through record_debug via tracing's default implementations. +#[derive(Default)] +struct MessageVisitor { + message: String, +} +impl tracing::field::Visit for MessageVisitor { + fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) { + if field.name() == "message" { + self.message = format!("{:?}", value); + } + } +} +#[cfg(test)] +mod tests { + use super::*; + use std::sync::{Arc, Mutex}; + + struct TestLogCallback { + logs: Arc>>, + } + + impl LogCallback for TestLogCallback { + fn on_log(&self, level: String, target: String, message: String) -> crate::Result<()> { + self.logs.lock().unwrap().push((level, target, message)); + Ok(()) + } + } + + #[test] + fn test_trait_can_be_implemented() { + let _callback: Arc = Arc::new(TestLogCallback { + logs: Arc::new(Mutex::new(Vec::new())), + }); + } + + #[test] + fn test_callback_layer_forwards_events() { + // Verify CallbackLayer correctly extracts and forwards log data + let logs = Arc::new(Mutex::new(Vec::new())); + let callback = Arc::new(TestLogCallback { logs: logs.clone() }); + let _layer = CallbackLayer::new(callback); + + // Test that layer compiles and can be created + // Full integration test will happen after Client::new() modification + assert!(logs.lock().unwrap().is_empty()); + } +} diff --git a/crates/bitwarden-uniffi/tests/callback_error_handling.rs b/crates/bitwarden-uniffi/tests/callback_error_handling.rs new file mode 100644 index 000000000..091a9d4de --- /dev/null +++ b/crates/bitwarden-uniffi/tests/callback_error_handling.rs @@ -0,0 +1,53 @@ +use bitwarden_uniffi::*; +use std::sync::Arc; + +// Type alias to match trait definition +type Result = std::result::Result; + +/// Mock token provider for testing +#[derive(Debug)] +struct MockTokenProvider; + +#[async_trait::async_trait] +impl bitwarden_core::client::internal::ClientManagedTokens for MockTokenProvider { + async fn get_access_token(&self) -> Option { + Some("mock_token".to_string()) + } +} + +/// Failing callback that always returns errors +struct FailingCallback; + +impl LogCallback for FailingCallback { + fn on_log(&self, _level: String, _target: String, _message: String) -> Result<()> { + // Simulate mobile callback exception + // Use a simple error that will be converted at FFI boundary + Err(bitwarden_uniffi::error::BitwardenError::Conversion("Simulated mobile callback failure".to_string())) + } +} + +#[test] +fn test_callback_error_does_not_crash_sdk() { + // TC-005: Verify SDK continues operating when callback returns errors + + // Create client with failing callback + let client = Client::new( + Arc::new(MockTokenProvider), + None, + Some(Arc::new(FailingCallback)), + ); + + // SDK should work before triggering callback + assert_eq!(client.echo("test".into()), "test"); + + // Trigger logs that invoke failing callback + tracing::info!("This log triggers failing callback"); + tracing::warn!("Another log that fails"); + tracing::error!("Yet another failing log"); + + // Verify SDK still operational after multiple callback errors + assert_eq!(client.echo("still works".into()), "still works"); + + // SDK operations continue normally despite callback failures + assert_eq!(client.echo("definitely still working".into()), "definitely still working"); +} diff --git a/crates/bitwarden-uniffi/tests/callback_field_coverage.rs b/crates/bitwarden-uniffi/tests/callback_field_coverage.rs new file mode 100644 index 000000000..de7ff4650 --- /dev/null +++ b/crates/bitwarden-uniffi/tests/callback_field_coverage.rs @@ -0,0 +1,64 @@ +use bitwarden_uniffi::*; +use std::sync::{Arc, Mutex}; + +// Type alias to match trait definition +type Result = std::result::Result; + +/// Mock token provider for testing +#[derive(Debug)] +struct MockTokenProvider; + +#[async_trait::async_trait] +impl bitwarden_core::client::internal::ClientManagedTokens for MockTokenProvider { + async fn get_access_token(&self) -> Option { + Some("mock_token".to_string()) + } +} + +/// Test callback that captures logs +struct TestCallback { + logs: Arc>>, +} + +impl LogCallback for TestCallback { + fn on_log(&self, level: String, target: String, message: String) -> Result<()> { + self.logs.lock().unwrap().push((level, target, message)); + Ok(()) + } +} + +#[test] +fn test_message_visitor_captures_message_field() { + // TC-007: Validate MessageVisitor captures the message field from trace events + // Note: Structured fields (user_id, valid, etc.) are NOT captured in v1 - this is + // a known limitation documented in the technical breakdown. The visitor only + // extracts the "message" field, not additional structured metadata. + let logs = Arc::new(Mutex::new(Vec::new())); + let callback = Arc::new(TestCallback { logs: logs.clone() }); + + let _client = Client::new( + Arc::new(MockTokenProvider), + None, + Some(callback), + ); + + // Emit logs at different levels with message text + tracing::info!("info message"); + tracing::warn!("warn message"); + tracing::error!("error message"); + + let captured = logs.lock().unwrap(); + + // Verify all messages captured + assert_eq!(captured.len(), 3, "All log entries should be captured"); + + // Validate message field extraction + assert!(captured[0].2.contains("info message"), "INFO message should be captured, got: {}", captured[0].2); + assert!(captured[1].2.contains("warn message"), "WARN message should be captured, got: {}", captured[1].2); + assert!(captured[2].2.contains("error message"), "ERROR message should be captured, got: {}", captured[2].2); + + // Validate levels + assert_eq!(captured[0].0, "INFO"); + assert_eq!(captured[1].0, "WARN"); + assert_eq!(captured[2].0, "ERROR"); +} diff --git a/crates/bitwarden-uniffi/tests/callback_happy_path.rs b/crates/bitwarden-uniffi/tests/callback_happy_path.rs new file mode 100644 index 000000000..3cc8af17f --- /dev/null +++ b/crates/bitwarden-uniffi/tests/callback_happy_path.rs @@ -0,0 +1,55 @@ +use bitwarden_uniffi::*; +use std::sync::{Arc, Mutex}; + +// Type alias to match trait definition +type Result = std::result::Result; + +/// Mock token provider for testing +#[derive(Debug)] +struct MockTokenProvider; + +#[async_trait::async_trait] +impl bitwarden_core::client::internal::ClientManagedTokens for MockTokenProvider { + async fn get_access_token(&self) -> Option { + Some("mock_token".to_string()) + } +} + +/// Test callback implementation that captures logs +struct TestCallback { + logs: Arc>>, +} + +impl LogCallback for TestCallback { + fn on_log(&self, level: String, target: String, message: String) -> Result<()> { + self.logs.lock().unwrap().push((level, target, message)); + Ok(()) + } +} + +#[test] +fn test_callback_happy_path() { + // TC-001: Verify callback receives logs with correct data + let logs = Arc::new(Mutex::new(Vec::new())); + let callback = Arc::new(TestCallback { logs: logs.clone() }); + + // Create client with callback + let _client = Client::new( + Arc::new(MockTokenProvider), + None, + Some(callback), + ); + + // Trigger SDK logging + tracing::info!("integration test message"); + + // Verify callback received the log + let captured = logs.lock().unwrap(); + assert!(!captured.is_empty(), "Callback should receive logs"); + + // Validate log data structure + let (level, target, message) = &captured[0]; + assert_eq!(level, "INFO", "Log level should be INFO"); + assert!(!target.is_empty(), "Target should not be empty"); + assert!(message.contains("integration test message"), "Message should contain logged text"); +} diff --git a/crates/bitwarden-uniffi/tests/callback_multiple_levels.rs b/crates/bitwarden-uniffi/tests/callback_multiple_levels.rs new file mode 100644 index 000000000..5a520b932 --- /dev/null +++ b/crates/bitwarden-uniffi/tests/callback_multiple_levels.rs @@ -0,0 +1,60 @@ +use bitwarden_uniffi::*; +use std::sync::{Arc, Mutex}; + +// Type alias to match trait definition +type Result = std::result::Result; + +/// Mock token provider for testing +#[derive(Debug)] +struct MockTokenProvider; + +#[async_trait::async_trait] +impl bitwarden_core::client::internal::ClientManagedTokens for MockTokenProvider { + async fn get_access_token(&self) -> Option { + Some("mock_token".to_string()) + } +} + +/// Test callback that captures logs +struct TestCallback { + logs: Arc>>, +} + +impl LogCallback for TestCallback { + fn on_log(&self, level: String, target: String, message: String) -> Result<()> { + self.logs.lock().unwrap().push((level, target, message)); + Ok(()) + } +} + +#[test] +fn test_callback_receives_multiple_log_levels() { + // TC-002: Verify callback receives events at different log levels + let logs = Arc::new(Mutex::new(Vec::new())); + let callback = Arc::new(TestCallback { logs: logs.clone() }); + + let _client = Client::new( + Arc::new(MockTokenProvider), + None, + Some(callback), + ); + + // Emit logs at multiple levels + tracing::info!("info message"); + tracing::warn!("warn message"); + tracing::error!("error message"); + + // Verify all levels captured + let captured = logs.lock().unwrap(); + assert_eq!(captured.len(), 3, "Should capture all 3 log levels"); + + // Validate each level + assert_eq!(captured[0].0, "INFO"); + assert!(captured[0].2.contains("info message")); + + assert_eq!(captured[1].0, "WARN"); + assert!(captured[1].2.contains("warn message")); + + assert_eq!(captured[2].0, "ERROR"); + assert!(captured[2].2.contains("error message")); +} diff --git a/crates/bitwarden-uniffi/tests/callback_thread_safety.rs b/crates/bitwarden-uniffi/tests/callback_thread_safety.rs new file mode 100644 index 000000000..740fbb864 --- /dev/null +++ b/crates/bitwarden-uniffi/tests/callback_thread_safety.rs @@ -0,0 +1,67 @@ +use bitwarden_uniffi::*; +use std::sync::{Arc, Mutex}; +use std::thread; + +// Type alias to match trait definition +type Result = std::result::Result; + +/// Mock token provider for testing +#[derive(Debug)] +struct MockTokenProvider; + +#[async_trait::async_trait] +impl bitwarden_core::client::internal::ClientManagedTokens for MockTokenProvider { + async fn get_access_token(&self) -> Option { + Some("mock_token".to_string()) + } +} + +/// Thread-safe test callback +struct TestCallback { + logs: Arc>>, +} + +impl LogCallback for TestCallback { + fn on_log(&self, level: String, target: String, message: String) -> Result<()> { + self.logs.lock().unwrap().push((level, target, message)); + Ok(()) + } +} + +#[test] +fn test_callback_thread_safety() { + // TC-004: Verify callback handles concurrent invocations safely + let logs = Arc::new(Mutex::new(Vec::new())); + let callback = Arc::new(TestCallback { logs: logs.clone() }); + + let _client = Client::new( + Arc::new(MockTokenProvider), + None, + Some(callback), + ); + + // Spawn multiple threads logging simultaneously + let handles: Vec<_> = (0..10) + .map(|i| { + thread::spawn(move || { + tracing::info!("thread {} message", i); + }) + }) + .collect(); + + // Wait for all threads to complete + for handle in handles { + handle.join().unwrap(); + } + + // Verify all logs captured without data races + let captured = logs.lock().unwrap(); + assert_eq!(captured.len(), 10, "All threaded logs should be captured"); + + // Verify no corrupted entries (all should be INFO level) + for (level, _target, message) in captured.iter() { + assert_eq!(level, "INFO"); + assert!(message.contains("thread")); + assert!(message.contains("message")); + } +}